[PATCH] D35791: [COFF, ARM64] Fix symbol offsets in ADRP/ADD/LDR/STR relocations
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 25 10:26:10 PDT 2017
I think this is fine. Just make sure there is test coverage for both
IsResolved and !IsResolved cases.
LGTM from the MC side, but please have someone that actually knows COFF
to LGTM this too :-)
Cheers,
Rafael
Martin Storsjö via Phabricator <reviews at reviews.llvm.org> writes:
> mstorsjo updated this revision to Diff 108046.
> mstorsjo added a comment.
>
> Changed the previous check for !IsResolved into an assert, and added checks for it in the other fixups (where it is needed).
>
>
> https://reviews.llvm.org/D35791
>
> Files:
> lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
> test/MC/AArch64/coff-relocations.s
>
> Index: test/MC/AArch64/coff-relocations.s
> ===================================================================
> --- test/MC/AArch64/coff-relocations.s
> +++ test/MC/AArch64/coff-relocations.s
> @@ -1,5 +1,6 @@
> -; RUN: llvm-mc -triple aarch64-windows -filetype obj -o - %s | \
> -; RUN: llvm-readobj -r - | FileCheck %s
> +; RUN: llvm-mc -triple aarch64-windows -filetype obj -o %t.obj %s
> +; RUN: llvm-readobj -r %t.obj | FileCheck %s
> +; RUN: llvm-objdump -d %t.obj | FileCheck %s -check-prefix DISASM
>
> ; IMAGE_REL_ARM64_ADDR32
> .Linfo_foo:
> @@ -37,6 +38,13 @@
> ; IMAGE_REL_ARM64_SECTION
> .secidx func
>
> +.align 2
> +adrp x0, baz + 0x12345
> +baz:
> +add x0, x0, :lo12:foo + 0x12345
> +ldrb w0, [x0, :lo12:foo + 0x12345]
> +ldr x0, [x0, :lo12:foo + 0x12348]
> +
> ; CHECK: Format: COFF-ARM64
> ; CHECK: Arch: aarch64
> ; CHECK: AddressSize: 64bit
> @@ -52,5 +60,14 @@
> ; CHECK: 0x24 IMAGE_REL_ARM64_PAGEBASE_REL21 bar
> ; CHECK: 0x28 IMAGE_REL_ARM64_SECREL .text
> ; CHECK: 0x2C IMAGE_REL_ARM64_SECTION func
> +; CHECK: 0x30 IMAGE_REL_ARM64_PAGEBASE_REL21 baz
> +; CHECK: 0x34 IMAGE_REL_ARM64_PAGEOFFSET_12A foo
> +; CHECK: 0x38 IMAGE_REL_ARM64_PAGEOFFSET_12L foo
> +; CHECK: 0x3C IMAGE_REL_ARM64_PAGEOFFSET_12L foo
> ; CHECK: }
> ; CHECK: ]
> +
> +; DISASM: 30: 20 1a 09 b0 adrp x0, #305418240
> +; DISASM: 34: 00 14 0d 91 add x0, x0, #837
> +; DISASM: 38: 00 14 4d 39 ldrb w0, [x0, #837]
> +; DISASM: 3c: 00 a4 41 f9 ldr x0, [x0, #840]
> Index: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
> ===================================================================
> --- lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
> +++ lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
> @@ -30,12 +30,14 @@
> class AArch64AsmBackend : public MCAsmBackend {
> static const unsigned PCRelFlagVal =
> MCFixupKindInfo::FKF_IsAlignedDownTo32Bits | MCFixupKindInfo::FKF_IsPCRel;
> + Triple TheTriple;
> +
> public:
> bool IsLittleEndian;
>
> public:
> - AArch64AsmBackend(const Target &T, bool IsLittleEndian)
> - : MCAsmBackend(), IsLittleEndian(IsLittleEndian) {}
> + AArch64AsmBackend(const Target &T, const Triple &TT, bool IsLittleEndian)
> + : MCAsmBackend(), TheTriple(TT), IsLittleEndian(IsLittleEndian) {}
>
> unsigned getNumFixupKinds() const override {
> return AArch64::NumTargetFixupKinds;
> @@ -143,7 +145,8 @@
> }
>
> static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
> - MCContext &Ctx) {
> + MCContext &Ctx, const Triple &TheTriple,
> + bool IsResolved) {
> unsigned Kind = Fixup.getKind();
> int64_t SignedValue = static_cast<int64_t>(Value);
> switch (Kind) {
> @@ -154,6 +157,9 @@
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> return AdrImmBits(Value & 0x1fffffULL);
> case AArch64::fixup_aarch64_pcrel_adrp_imm21:
> + assert(!IsResolved);
> + if (TheTriple.isOSBinFormatCOFF())
> + return AdrImmBits(Value & 0x1fffffULL);
> return AdrImmBits((Value & 0x1fffff000ULL) >> 12);
> case AArch64::fixup_aarch64_ldr_pcrel_imm19:
> case AArch64::fixup_aarch64_pcrel_branch19:
> @@ -166,32 +172,42 @@
> return (Value >> 2) & 0x7ffff;
> case AArch64::fixup_aarch64_add_imm12:
> case AArch64::fixup_aarch64_ldst_imm12_scale1:
> + if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
> + Value &= 0xfff;
> // Unsigned 12-bit immediate
> if (Value >= 0x1000)
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> return Value;
> case AArch64::fixup_aarch64_ldst_imm12_scale2:
> + if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
> + Value &= 0xfff;
> // Unsigned 12-bit immediate which gets multiplied by 2
> if (Value >= 0x2000)
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> if (Value & 0x1)
> Ctx.reportError(Fixup.getLoc(), "fixup must be 2-byte aligned");
> return Value >> 1;
> case AArch64::fixup_aarch64_ldst_imm12_scale4:
> + if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
> + Value &= 0xfff;
> // Unsigned 12-bit immediate which gets multiplied by 4
> if (Value >= 0x4000)
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> if (Value & 0x3)
> Ctx.reportError(Fixup.getLoc(), "fixup must be 4-byte aligned");
> return Value >> 2;
> case AArch64::fixup_aarch64_ldst_imm12_scale8:
> + if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
> + Value &= 0xfff;
> // Unsigned 12-bit immediate which gets multiplied by 8
> if (Value >= 0x8000)
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> if (Value & 0x7)
> Ctx.reportError(Fixup.getLoc(), "fixup must be 8-byte aligned");
> return Value >> 3;
> case AArch64::fixup_aarch64_ldst_imm12_scale16:
> + if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
> + Value &= 0xfff;
> // Unsigned 12-bit immediate which gets multiplied by 16
> if (Value >= 0x10000)
> Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
> @@ -278,7 +294,7 @@
> MCFixupKindInfo Info = getFixupKindInfo(Fixup.getKind());
> MCContext &Ctx = Asm.getContext();
> // Apply any target-specific value adjustments.
> - Value = adjustFixupValue(Fixup, Value, Ctx);
> + Value = adjustFixupValue(Fixup, Value, Ctx, TheTriple, IsResolved);
>
> // Shift the value into position.
> Value <<= Info.TargetOffset;
> @@ -412,8 +428,9 @@
> }
>
> public:
> - DarwinAArch64AsmBackend(const Target &T, const MCRegisterInfo &MRI)
> - : AArch64AsmBackend(T, /*IsLittleEndian*/true), MRI(MRI) {}
> + DarwinAArch64AsmBackend(const Target &T, const Triple &TT,
> + const MCRegisterInfo &MRI)
> + : AArch64AsmBackend(T, TT, /*IsLittleEndian*/ true), MRI(MRI) {}
>
> MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
> return createAArch64MachObjectWriter(OS, MachO::CPU_TYPE_ARM64,
> @@ -560,9 +577,10 @@
> uint8_t OSABI;
> bool IsILP32;
>
> - ELFAArch64AsmBackend(const Target &T, uint8_t OSABI, bool IsLittleEndian,
> - bool IsILP32)
> - : AArch64AsmBackend(T, IsLittleEndian), OSABI(OSABI), IsILP32(IsILP32) {}
> + ELFAArch64AsmBackend(const Target &T, const Triple &TT, uint8_t OSABI,
> + bool IsLittleEndian, bool IsILP32)
> + : AArch64AsmBackend(T, TT, IsLittleEndian), OSABI(OSABI),
> + IsILP32(IsILP32) {}
>
> MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
> return createAArch64ELFObjectWriter(OS, OSABI, IsLittleEndian, IsILP32);
> @@ -575,7 +593,7 @@
> class COFFAArch64AsmBackend : public AArch64AsmBackend {
> public:
> COFFAArch64AsmBackend(const Target &T, const Triple &TheTriple)
> - : AArch64AsmBackend(T, /*IsLittleEndian*/true) {}
> + : AArch64AsmBackend(T, TheTriple, /*IsLittleEndian*/ true) {}
>
> MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
> return createAArch64WinCOFFObjectWriter(OS);
> @@ -589,16 +607,17 @@
> StringRef CPU,
> const MCTargetOptions &Options) {
> if (TheTriple.isOSBinFormatMachO())
> - return new DarwinAArch64AsmBackend(T, MRI);
> + return new DarwinAArch64AsmBackend(T, TheTriple, MRI);
>
> if (TheTriple.isOSBinFormatCOFF())
> return new COFFAArch64AsmBackend(T, TheTriple);
>
> assert(TheTriple.isOSBinFormatELF() && "Invalid target");
>
> uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(TheTriple.getOS());
> bool IsILP32 = Options.getABIName() == "ilp32";
> - return new ELFAArch64AsmBackend(T, OSABI, /*IsLittleEndian=*/true, IsILP32);
> + return new ELFAArch64AsmBackend(T, TheTriple, OSABI, /*IsLittleEndian=*/true,
> + IsILP32);
> }
>
> MCAsmBackend *llvm::createAArch64beAsmBackend(const Target &T,
> @@ -610,5 +629,6 @@
> "Big endian is only supported for ELF targets!");
> uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(TheTriple.getOS());
> bool IsILP32 = Options.getABIName() == "ilp32";
> - return new ELFAArch64AsmBackend(T, OSABI, /*IsLittleEndian=*/false, IsILP32);
> + return new ELFAArch64AsmBackend(T, TheTriple, OSABI, /*IsLittleEndian=*/false,
> + IsILP32);
> }
More information about the llvm-commits
mailing list