[PATCH] D43287: [LLD] [COFF] Add support for ARM64 secrel relocations for add/load instructions

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Chunks.cpp:218-222
+  if (!OS) {
+    if (Sec->isCodeView())
+      return;
+    fatal("SECREL relocation cannot be applied to absolute symbols");
+  }
----------------
ruiu wrote:
> You should factor out this part of code.
Ok, will do


================
Comment at: COFF/Chunks.cpp:225
+  SecRel >>= Shift;
+  if (Shift > 0 && SecRel > 0xfff) {
+    error("overflow in SECREL_HIGH12A relocation in section: " +
----------------
ruiu wrote:
> Why do you need to check for Shift?
When using these relocations, you'd normally use them in a pair with SECREL_HIGH12A plus SECREL_LOW12A/L. It's very much expected and ok that the actual section relative offset is larger than 12 bits, and only the truncated lower 12 bits go with the LOW12A/L relocations. For the HIGH12A relocation, there's no corresponding larger relocation that could be used to handle the rest, so if the code uses that relocation, and there's overflow when writing the high part of it, we have an issue.

Another way of writing the same would be to always check if the section relative offset is over 24 bits, regardless of which of these relocations are used.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43287