[PATCH] D70406: Ignore R_MIPS_JALR relocations against non-function symbols

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan added inline comments.


================
Comment at: lld/ELF/Arch/Mips.cpp:90
+    // relocation and emit a warning instead.
+    if (!s.isFunc()) {
+      warn(getErrorLocation(loc) +
----------------
arichardson wrote:
> atanasyan wrote:
> > This change breaks `mips-jalr.s` test case. Now we have to explicitly provide `.type @function` directive. What do you think about this condition `if (s.isObject() || s.isTls())`?
> I think `if (!s.isFunc() && s.type != STT_NOTYPE)` might be better since that catches all other non-function types.
> 
> However, this is purely a missed optimization when not being applied, but an error when applied erroneously, so I'd lean towards only allowing function symbols.
> Do you think that other than the testcase in LLD, there exists any code that would use R_MIPS_JALR against a STT_NOTYPE symbol that could not be marked with `.type @function`? I feel like in that case the programmer might as well write the bal explicitly in the assembly?
> 
> What do you think?
`if (!s.isFunc() && s.type != STT_NOTYPE)` looks better.

I do not know are there any "real" assembler code use R_MIPS_JALR  relocations and symbols without `.type` directive. But it's possible.

`jalr` can be replaced by `bal` right in a `.s` file if a distance to the target is known. But it's not so if the target and the relocation are in different object files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70406