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

Mon Aug 7 23:17:23 PDT 2023

efriedma added a comment.

> Thinking about this a bit further, I think that it might be better to conform to the Microsoft behaviour here. If we look at what MASM does, there is no concept of an "assembler temporary" hidden symbols. We should instead always emit all symbols into the symbol table.

I don't think that we can directly compare what we do to masm; we support an assembler dialect which is not masm, and that includes branching where the destination is an offset from a symbol.  Given that, I think we need this logic in WinCOFFObjectWriter, whether or not we emit all temporary symbols.  (The change in AArch64AsmBackend.cpp is necessary either way.)

Also, emitting all temporary symbols significantly bloats object files.  (I don't have numbers at hand, but I recall it being significant.)

================
Comment at: llvm/lib/MC/WinCOFFObjectWriter.cpp:944
+      Reloc.Symb = S;
+      FixedValue = 0;  // Zero offset
+    } else {
----------------
hjyamauchi wrote:
> efriedma wrote:
> > I don't understand why it's safe to throw away the "FixedValue" here; there could be an explicit offset.
> Done.
This still looks wrong in the case where `SymbolMap[&A]` is non-null; even if the initial symbol lookup succeeds, we still need to preserve the offset.

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

https://reviews.llvm.org/D155732