Skip to content

Conversation

MikailBag
Copy link

@MikailBag MikailBag commented Apr 29, 2025

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.

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.
@MikailBag MikailBag marked this pull request as ready for review April 29, 2025 21:12
Copy link
Author

@MikailBag MikailBag left a 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*);
Copy link
Author

@MikailBag MikailBag Apr 29, 2025

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;
Copy link
Author

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,
Copy link
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant