[PATCH] D65857: [MC][AArch64] Restrict use of signed relocation operators on MOV[NZK]

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 06:18:06 PDT 2019


peter.smith marked an inline comment as done.
peter.smith added a comment.

In D65857#1619366 <https://reviews.llvm.org/D65857#1619366>, @pcc wrote:

> MHO: The assembler is a low enough level component that the user can be presumed to know what they're doing, regardless of linker limitations. So I would prefer not to do this. If we do anything about this, we should document the limitations of the GNU linkers somewhere.


It is a tricky balance, we don't want to rule out a reasonable use case, but ideally we want to detect problems as soon as possible. I think what I have here may be overly strict as if you happen to know that the result of a signed operation is positive then this can work, or if you happen to know you are linking with LLD.

One possible compromise is some kind of strict mode, something like --strict-movw-relocs that people could enable to restrict the relocations to a GNU compatible subset for those that need it. The command line option could also act as a kind of documentation that is a bit more visible. Any thoughts?



================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:965
+  bool isMovKSymbolG3() const {
+    return isMovWSymbol({AArch64MCExpr::VK_ABS_G3});
+  }
----------------
pcc wrote:
> We produce MOVK with PREL_G3 when the tagged-globals target feature is enabled, so we should allow this combination at least.
I hadn't realised that this was actively being used. Are you doing something like:
```
movz x0, :prel_g0_nc: foo
movk x0, :prel_g1_nc: foo
movk x0, :prel_g2_nc: foo
movk x0, :prel_g3: foo
```

This will work in BFD and gold if the result is positive as bit 30 is already 1 in a MOVK so orring with 1 won't change it, however if the result is negative then bfd/gold will clear bit 30, which will result in an OPC of 0 1 which is MOV <register> , this will likely result in a silent failure. Similarly as VK_PREL_G* is not present in fixMOVZ it won't get altered to a MOVN so the non-checking g0 won't alter it.

If you are happy with those restrictions, for example the offset from code to data in LD.bfd with the default linker script is always going to be positive, or are willing to only support the feature on LLD, then we should support it. I'm a bit nervous about fixMOVZ though.



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

https://reviews.llvm.org/D65857





More information about the llvm-commits mailing list