[PATCH] D35791: [COFF, ARM64] Fix symbol offsets in ADRP/ADD/LDR/STR relocations
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 25 12:50:21 PDT 2017
On Tue, 25 Jul 2017, Rafael Avila de Espindola wrote:
> I think this is fine. Just make sure there is test coverage for both
> IsResolved and !IsResolved cases.
Thanks - I'll update the patch with a case of running
test/MC/AArch64/fixup-out-of-range.s (which normally assembles for
aarch64--none-eabi) for aarch64-windows as well; that should test to make
sure the out of range errors are kept as they should.
> LGTM from the MC side, but please have someone that actually knows COFF
> to LGTM this too :-)
I got an LGTM from Mandeep earlier as well, so it should be good to go.
I'll update the diff and push it in a little while then.
// Martin
> 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