Skip to content
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

Linking: only rename file-local symbols #8167

Merged

Conversation

tautschnig
Copy link
Collaborator

Rather than renaming whatever is to be added to an existing symbol table, always rename file-local symbols. This will make sure that we do not rename a symbol that a later translation unit wants to refer to.

Fixes: #7720

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Rather than renaming whatever is to be added to an existing symbol
table, always rename file-local symbols. This will make sure that we do
not rename a symbol that a later translation unit wants to refer to.

Fixes: diffblue#7720
@kroening
Copy link
Member

I am a bit uneasy to rename proactively, but this is likely the easiest approach.

@kroening kroening assigned tautschnig and unassigned kroening Jan 24, 2024
@tautschnig
Copy link
Collaborator Author

I am a bit uneasy to rename proactively, but this is likely the easiest approach.

Hmm, no, I think I managed to avoid this. When we have translation units A, B (seen in that order) with a file-local symbol X in A and a global-scoped symbol X in B we will now do:

  1. We will have "X" in the symbol table after compiling and type-checking A.
  2. When linking in B, we notice that there is a conflict on "X".
  3. With the prior approach, we would have added B's X as X$link1. With the new approach, we now (that we are adding B) will instead rename the existing X to X$link1 and will add B's X as X.

@tautschnig tautschnig merged commit 5bce1c3 into diffblue:develop Jan 24, 2024
36 of 37 checks passed
@tautschnig tautschnig deleted the bugfixes/linking-rename-local branch January 24, 2024 20:06
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.

Linker does not identify the correct function with external linkage
2 participants