Skip to content

linker: non-recursive version of get_symbols #2894

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

Merged
merged 1 commit into from
Sep 4, 2018
Merged

Conversation

kroening
Copy link
Member

@kroening kroening commented Sep 4, 2018

This prevents a stack overflow in cases of very long dependency chains.

const namespacet &ns,
const symbolt &symbol,
find_symbols_sett &dest)
{
dest.insert(symbol.name);
std::vector<const symbolt *> working_set;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have used a forward_list, but it's probably ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the density of the vector, and the fact that it won't actually do memory allocation once the working set has reached some size.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick experiment: 14.7s vs. 16.2s, for total goto-cc time on a larger .c file, in favour of the vector.

@tautschnig tautschnig merged commit 3f5847c into develop Sep 4, 2018
@tautschnig tautschnig deleted the linker-get-symbols branch September 4, 2018 16:17
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR failed Diffblue compatibility checks (cbmc commit: 0a255fd).
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

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

Successfully merging this pull request may close these issues.

3 participants