[PATCH] D100835: [WIP][LLD][RISCV] Linker Relaxation

[Chih-Mao Chen via Phabricator via llvm-commits]

PkmX added inline comments.


================
Comment at: lld/ELF/Arch/RISCV.cpp:686
+      for (auto &rel : is->relocations) {
+        if (rel.type == R_RISCV_ALIGN && rel.addend > 0) {
+          uint64_t pc = is->getVA(rel.offset) - bytesDeleted;
----------------
jrtc27 wrote:
> When would the addend ever be 0? That seems like invalid input.
Yes, valid addends should only be 2^n-2 for n >= 2. With that said I don't think lld is really checking this anywhere, so malicious input could cause lld to crash.


================
Comment at: lld/ELF/Relocations.cpp:135
+template <RelExpr... Exprs> struct RelExprMaskTester {
+  static inline bool test(RelExpr expr) { return false; }
 };
----------------
PkmX wrote:
> jrtc27 wrote:
> > (a) this is unrelated to the diff
> > (b) this completely removes the whole point of oneof
> As noted in the summary adding more `RelExpr` makes it unable to fit in a 64-bit mask, so this is temporary change to make this patch work for now.
> 
> There were already 64 `RelExpr`s before this patch so there is no room for growing and I don't think it is workable in the long run. The only solution I can think of is split them into target specific `RelExpr` with some common exprs like `R_ABS` being shared, so as long as the target doesn't define more than ~20 `RelExpr` it should be fine. However this probably should belong to another patch.
Alternatively I think we can use two 64-bit masks, so this should work up to 128 `RelExpr`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100835