[ELF] Respect orders of symbol assignments and DEFINED #65866
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM. Would it be better to add a link to the GNU-ld's AVR default linker script, in your commit message ? https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/scripttempl/avr.sc;hb=HEAD |
benshi001
approved these changes
Sep 11, 2023
smithp35
approved these changes
Sep 11, 2023
MaskRay
force-pushed
the
lld-defined
branch
from
b249b87 to
36dc233
Compare
|
Thanks for the review. Improved comments. |
MaskRay
force-pushed
the
lld-defined
branch
from
36dc233 to
591aa46
Compare
MaskRay
force-pushed
the
lld-defined
branch
from
591aa46 to
764e66b
Compare
Fix llvm#64600: the currently implementation is minimal (see https://reviews.llvm.org/D83758), and an assignment like `__TEXT_REGION_ORIGIN__ = DEFINED(__TEXT_REGION_ORIGIN__) ? __TEXT_REGION_ORIGIN__ : 0;` (used by avr-ld[1]) leads to a value of zero (default value in `declareSymbol`), which is unexpected. Assign orders to symbol assignments and references so that for a script-defined symbol, the `DEFINED` results match users' expectation. I am unclear about GNU ld's exact behavior, but this hopefully matches its behavior in the majority of cases. [1]: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/scripttempl/avr.sc
MaskRay
force-pushed
the
lld-defined
branch
from
764e66b to
65a15a5
Compare
ZijunZhaoCCK
pushed a commit
to ZijunZhaoCCK/llvm-project
that referenced
this pull request
Sep 19, 2023
Fix llvm#64600: the currently implementation is minimal (see https://reviews.llvm.org/D83758), and an assignment like `__TEXT_REGION_ORIGIN__ = DEFINED(__TEXT_REGION_ORIGIN__) ? __TEXT_REGION_ORIGIN__ : 0;` (used by avr-ld[1]) leads to a value of zero (default value in `declareSymbol`), which is unexpected. Assign orders to symbol assignments and references so that for a script-defined symbol, the `DEFINED` results match users' expectation. I am unclear about GNU ld's exact behavior, but this hopefully matches its behavior in the majority of cases. [1]: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/scripttempl/avr.sc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #64600: the currently implementation is minimal (see
https://reviews.llvm.org/D83758), and an assignment like
__TEXT_REGION_ORIGIN__ = DEFINED(__TEXT_REGION_ORIGIN__) ? __TEXT_REGION_ORIGIN__ : 0;(used by avr-ld1) leads to a value of zero (default value in
declareSymbol),which is unexpected.
Assign orders to symbol assignments and references so that
for a script-defined symbol, the
DEFINEDresults match users'expectation. I am unclear about GNU ld's exact behavior, but this hopefully
matches its behavior in the majority of cases.