-
Notifications
You must be signed in to change notification settings - Fork 255
Avoid recursion in resolve_unit_addrs_overlap #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This patch replaces recursion with explicit stack of enclosing ranges. This fixes stack overflow on certain binaries when initialization happens inside a fiber. See ianlancetaylor#155 for original discussion.
5921018
to
bf7888f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some questions I've got while writing this patch.
&& ((struct unit_addrs**)enclosing->base)[enclosing_count-1]->high <= old_addrs[from].low) | ||
{ | ||
enclosing_count--; | ||
enclosing->alc += sizeof (struct unit_addrs*); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I want to do here is enclosing.pop_back()
. I have found backtrace_vector_grow
, but didn't find inverse function that shrinks used part of the allocation.
*addrs_vec = new_vec; | ||
backtrace_vector_free (state, &enclosing, error_callback, data); | ||
if (walk_ok) | ||
*addrs_vec = new_vec; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously return code from walk
subroutine was ignored here. Was that intended?
resolve_unit_addrs_overlap. */ | ||
|
||
static int | ||
resolve_unit_addrs_overlap_walk (struct backtrace_state *state, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this code path is covered by existing tests. Should I add a unit test?
This patch replaces recursion with explicit stack of enclosing ranges. This fixes stack overflow on certain binaries when initialization happens inside a fiber.
See #155 for original discussion.