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

[Hiroshi Yamauchi via Phabricator via llvm-commits]

hjyamauchi marked 2 inline comments as done.
hjyamauchi added inline comments.


================
Comment at: llvm/lib/MC/WinCOFFObjectWriter.cpp:961
+      // instruction to symbol A and drop the relocation.  Set to the
+      // correct relative offset.
+      FixedValue = Layout.getSymbolOffset(A) - Reloc.Data.VirtualAddress;
----------------
MaskRay wrote:
> efriedma wrote:
> > We shouldn't end up in recordRelocation for branches where the source and destination are in the same section, I think.
> Ideally we should reuse the temporary symbol, not creating a new one.
> 
> The ELF approach is to omit temporary symbols only when unused by relocations (see `ELFWriter::isInSymtab`).
> 
> COFF D14975 may give some insights as well.
Done. Reused the temporary symbol.


================
Comment at: llvm/lib/MC/WinCOFFObjectWriter.cpp:961
+      // instruction to symbol A and drop the relocation.  Set to the
+      // correct relative offset.
+      FixedValue = Layout.getSymbolOffset(A) - Reloc.Data.VirtualAddress;
----------------
hjyamauchi wrote:
> MaskRay wrote:
> > efriedma wrote:
> > > We shouldn't end up in recordRelocation for branches where the source and destination are in the same section, I think.
> > Ideally we should reuse the temporary symbol, not creating a new one.
> > 
> > The ELF approach is to omit temporary symbols only when unused by relocations (see `ELFWriter::isInSymtab`).
> > 
> > COFF D14975 may give some insights as well.
> Done. Reused the temporary symbol.
(Please see the other comment discussion)


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

https://reviews.llvm.org/D155732