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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

In D155732#4521021 <https://reviews.llvm.org/D155732#4521021>, @hjyamauchi wrote:

>> We should teach adjustFixupValue() in AArch64AsmBackend.cpp to print an error if an unresolved branch with an offset gets there somehow.
>
> If you mean like the following, that would unintentionally trigger when we drop the relocation in the newly-added code (in the `if (MCSec == &A.getSection())` case) because `IsResolved` is false.
>
>   case AArch64::fixup_aarch64_pcrel_branch26:
>   case AArch64::fixup_aarch64_pcrel_call26:
>     if (TheTriple.isOSBinFormatCOFF() && !IsResolved && SignedValue != 0) {
>       // MSVC link.exe and lld do not support this relocation type with a non-zero offset
>       Ctx.reportError(Fixup.getLoc(), "fixup value not supported by the linkers");
>     }

I still don't understand why the `MCSec == &A.getSection()` case is relevant; if the offset is constant, it should be resolved, since "resolved" means "doesn't need a relocation".

In D155732#4521029 <https://reviews.llvm.org/D155732#4521029>, @hjyamauchi wrote:

>> You can't create symbols at arbitrary offsets from the start of a section; you need to check that the symbol actually points somewhere within the section.
>
> Can you clarify? Does it mean that we need to check `A.isInSection() && Layout.getSymbolOffset(A) < getSectionAddressSize(&A.getSection())` before we create a new symbol at where symbol A is? What should happen if that's not the case?

Something like that, yes.  If it fails, we need to print an error, because we don't have any way to encode it.  At least, I don't think we have a way to encode it?  Not sure off the top of my head what offsets the linker would tolerate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155732