[PATCH] D155732: [MC][COFF][AArch64] Avoid incorrect IMAGE_REL_ARM64_BRANCH26 relocations.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

I agree that we can't emit all labels with the `.L` prefix into the object file symbol table. That would be prohibitively expensive.

We can, however, probably treat all `private` linkage symbols as `internal` if we don't already, so we get symbols for them in the symbol table, if this issue is still reachable from the IR level. We can probably also "upgrade" private symbols that the assembler reads to add them to the symbol table if they are used with this kind of fixup. Either way, I think we need the error handling added in the existing patch.



================
Comment at: llvm/lib/MC/WinCOFFObjectWriter.cpp:935
+              Offset < Layout.getSectionAddressSize(&A.getSection()))) {
+          Asm.getContext().reportError(
+              Fixup.getLoc(),
----------------
Please add a test exercising this error case as well.


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:320
+      // with a non-zero offset
+      Ctx.reportError(Fixup.getLoc(), "fixup value unsupported by linkers");
+    }
----------------
Please add a test exercising the error case, presumably using something like the `b .Lsym + 4` pattern.

Also, I think you can make the diagnostic message clearer. Something like, "cannot perform a PC-relative fixup with a non-zero symbol offset" (emphasize that the offset from a symbol must be zero).


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

https://reviews.llvm.org/D155732