[PATCH] D150940: [lld][RISCV][test] Add test for relaxation combined with --wrap

Wed May 31 01:54:33 PDT 2023

jobnoorman abandoned this revision.
jobnoorman added a comment.

In D150940#4382913 <https://reviews.llvm.org/D150940#4382913>, @MaskRay wrote:

> In D150940#4359968 <https://reviews.llvm.org/D150940#4359968>, @jobnoorman wrote:
>
>> In D150940#4356721 <https://reviews.llvm.org/D150940#4356721>, @kishore-rivos wrote:
>>
>>> To clarify, it isn't necessary for both foo and _wrap_foo to be defined in the same object file.  This bug was also triggered when they were defined in different object files.
>>
>> Thanks for mentioning this! I just noticed that situation is worse: it's still broken when `bar` is in a different object file but for a different reason than before.
>>
>> The following test is broken because the value of `bar` does not get adjusted:
>>
>>   #--- main.s
>>   .globl _start
>>   _start:
>>     call bar
>>   .globl bar
>>   bar:
>>   ## Note that this can be any instruction that gets relaxed. The goal is to force
>>   ## the value of __wrap_bar to change.
>>     call __wrap_bar
>>   
>>   #--- wrap.s
>>   .globl __wrap_bar
>>   __wrap_bar:
>>     call __real_bar
>>
>> This is caused by this loop <https://github.com/llvm/llvm-project/blob/bf497c0be2301c77a18d37ef104fcf6758095956/lld/ELF/Arch/RISCV.cpp#L553-L564>: for some reason, `bar` is part of the symbols returned by `getSymbols()` on `wrap.o` while its `file` points to `main.o`. So it never gets added to `anchors` and hence its value never gets adjusted.
>>
>> @MaskRay is this something that should be fixed in the relaxation implementation or is there an underlying issue with `--wrap` that should be addressed?
>
> Sorry for my belated response. I sent D151768 <https://reviews.llvm.org/D151768> to fix this case and add a test for the bug fixed by D149735 <https://reviews.llvm.org/D149735>.

Thanks! I'll close this one now since your patch includes the necessary tests.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150940/new/

https://reviews.llvm.org/D150940