[PATCH] D34138: [LLD][ELF] make default for get{ARM, AArch64}UndefinedRelativeWeakVA unreachable

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I'll update the comments and move the llvm_unreachable.



================
Comment at: ELF/InputSection.cpp:405-406
   switch (Type) {
+  // Unresolved branch relocations to weak references resolve to next
+  // instruction.
   case R_ARM_THM_JUMP11:
----------------
ruiu wrote:
> What does this comment explain?
In the ARM ABI there is a general rule for handling pc-relative relocations. There is a further special case for the subset of pc-relative instructions that are also branch relocations. I wanted to explain why the branch relocations resolved to the address of the next instruction, but the other relocations did not.

I'll update the comments to make this more explicit.


================
Comment at: ELF/InputSection.cpp:444
     return P + 4 + A;
-  default:
+  // Unresolved relative relocations to weak references resolve to the place
+  case R_AARCH64_PREL16:
----------------
ruiu wrote:
> to the place ...?
I'm using the long form for P in the PC-relative relocation calculation (S + A) - P. The place is commonly used in ELF processor supplements [*], but I'll make it clearer.

For example in the AMD64 ABI: 
> P Represents the place (section offset or address) of the storage unit being relocated (computed using r_offset).




https://reviews.llvm.org/D34138