[llvm][MC][ARM][Assembly] Emit relocations for ADRs and big-endian targets #73834
Conversation
eleanor-arm
commented
Nov 29, 2023
•
eleanor-arm
commented
|
@llvm/pr-subscribers-backend-arm Author: Eleanor Bonnici (eleanor-arm) ChangesFollow-up on #72873 When ADR instructions reference a label in a different section, the This patch ensures relocations are generated for ADR instructions, Tests for big-endian targets will follow. Full diff: https://github.com/llvm/llvm-project/pull/73834.diff 6 Files Affected:
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index ca3b77e4a356535..b3e90ac7e7cb4ec 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -88,10 +88,12 @@ const MCFixupKindInfo &ARMAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
{"fixup_arm_ldst_abs_12", 0, 32, 0},
{"fixup_thumb_adr_pcrel_10", 0, 8,
- IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
- {"fixup_arm_adr_pcrel_12", 0, 32, IsPCRelConstant},
+ MCFixupKindInfo::FKF_IsPCRel |
+ MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+ {"fixup_arm_adr_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_t2_adr_pcrel_12", 0, 32,
- IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+ MCFixupKindInfo::FKF_IsPCRel |
+ MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
{"fixup_arm_condbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_arm_uncondbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_t2_condbranch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
index 985097fc3281051..44695a86c4e36c7 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
@@ -164,6 +164,12 @@ unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
return ELF::R_ARM_LDRS_PC_G0;
case ARM::fixup_t2_ldst_pcrel_12:
return ELF::R_ARM_THM_PC12;
+ case ARM::fixup_arm_adr_pcrel_12:
+ return ELF::R_ARM_ALU_PC_G0;
+ case ARM::fixup_thumb_adr_pcrel_10:
+ return ELF::R_ARM_THM_PC8;
+ case ARM::fixup_t2_adr_pcrel_12:
+ return ELF::R_ARM_THM_ALU_PREL_11_0;
case ARM::fixup_bf_target:
return ELF::R_ARM_THM_BF16;
case ARM::fixup_bfc_target:
diff --git a/llvm/test/MC/ARM/pcrel-adr16-relocs.s b/llvm/test/MC/ARM/pcrel-adr16-relocs.s
new file mode 100644
index 000000000000000..dd73c9d28a6822b
--- /dev/null
+++ b/llvm/test/MC/ARM/pcrel-adr16-relocs.s
@@ -0,0 +1,23 @@
+@ RUN: llvm-mc -filetype=obj --triple=thumbv6m-none-eabi %s -o %t
+@ RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELOC
+@ RUN: llvm-objdump -d --triple=thumbv6m-none-eabi %t | FileCheck %s --check-prefix=ADDEND
+
+ .section .text._func1, "ax"
+
+ .balign 4
+ .global _func1
+ .type _func1, %function
+_func1:
+ adr r0, _func2
+@ RELOC: R_ARM_THM_PC8
+ bx lr
+
+// Checking the encoding only, as the disassembly is not quite correct here.
+
+// Thumb16 encoding supports only adding of the encoded immediate (not
+// subtracting, see [Arm ARM]), therefore sign change is required if the pcrel
+// offset is negative. This makes the calculation of the addend for
+// R_ARM_THM_PC8 more complex, for details see [ELF for the Arm 32-bit
+// architecture].
+@ ADDEND: a0ff
+
diff --git a/llvm/test/MC/ARM/pcrel-adr32-relocs.s b/llvm/test/MC/ARM/pcrel-adr32-relocs.s
new file mode 100644
index 000000000000000..5e64125b7ea82e8
--- /dev/null
+++ b/llvm/test/MC/ARM/pcrel-adr32-relocs.s
@@ -0,0 +1,20 @@
+@ RUN: llvm-mc -filetype=obj -triple=armv7 %s -o %t
+@ RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELOC
+@ RUN: llvm-objdump -d --triple=armv7 %t | FileCheck %s --check-prefix=ADDEND
+
+ .section .text._func1, "ax"
+
+ .balign 4
+ .global _func1
+ .type _func1, %function
+_func1:
+ adr r0, _func2
+@ RELOC: R_ARM_ALU_PC_G0
+ .thumb
+ adr r0, _func2
+@ RELOC: R_ARM_THM_ALU_PREL_11_0
+ bx lr
+
+@ ADDEND: sub r0, pc, #8
+@ ADDEND-NEXT: adr.w r0, #-4
+
diff --git a/llvm/test/MC/ARM/pcrel-global.s b/llvm/test/MC/ARM/pcrel-global.s
index 15d46cf2063ecff..1e9e6e989356ecc 100644
--- a/llvm/test/MC/ARM/pcrel-global.s
+++ b/llvm/test/MC/ARM/pcrel-global.s
@@ -7,11 +7,9 @@
@ CHECK: There are no relocations in this file.
@ DISASM-LABEL: <bar>:
-@ DISASM-NEXT: adr.w r0, #-4
-@ DISASM-NEXT: adr.w r0, #-8
-@ DISASM-NEXT: ldr r0, [pc, #0x0] @ 0x14 <bar+0xc>
+@ DISASM-NEXT: ldr r0, [pc, #0x0] @ 0x8 <bar+0x4>
@ DISASM-NEXT: add r0, pc
-@ DISASM-NEXT: .word 0xfffffff3
+@ DISASM-NEXT: .word 0xfffffffb
@@ GNU assembler creates an R_ARM_REL32 referencing bar.
@ DISASM-NOT: {{.}}
@@ -20,16 +18,12 @@
.globl foo
foo:
vldr d0, foo @ arm_pcrel_10
-adr r2, foo @ arm_adr_pcrel_12
.thumb
.thumb_func
.type bar, %function
.globl bar
bar:
-adr r0, bar @ thumb_adr_pcrel_10
-adr.w r0, bar @ t2_adr_pcrel_12
-
ldr r0, .LCPI
.LPC0_1:
add r0, pc
diff --git a/llvm/test/MC/ARM/thumb1-relax-adr.s b/llvm/test/MC/ARM/thumb1-relax-adr.s
index fc5c7c39df5ae1c..97b566f4833e632 100644
--- a/llvm/test/MC/ARM/thumb1-relax-adr.s
+++ b/llvm/test/MC/ARM/thumb1-relax-adr.s
@@ -1,6 +1,5 @@
@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
@ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
-@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
.global func1
_func1:
|
|
@llvm/pr-subscribers-mc Author: Eleanor Bonnici (eleanor-arm) ChangesFollow-up on #72873 When ADR instructions reference a label in a different section, the This patch ensures relocations are generated for ADR instructions, Tests for big-endian targets will follow. Full diff: https://github.com/llvm/llvm-project/pull/73834.diff 6 Files Affected:
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index ca3b77e4a356535..b3e90ac7e7cb4ec 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -88,10 +88,12 @@ const MCFixupKindInfo &ARMAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
{"fixup_arm_ldst_abs_12", 0, 32, 0},
{"fixup_thumb_adr_pcrel_10", 0, 8,
- IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
- {"fixup_arm_adr_pcrel_12", 0, 32, IsPCRelConstant},
+ MCFixupKindInfo::FKF_IsPCRel |
+ MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+ {"fixup_arm_adr_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_t2_adr_pcrel_12", 0, 32,
- IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+ MCFixupKindInfo::FKF_IsPCRel |
+ MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
{"fixup_arm_condbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_arm_uncondbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
{"fixup_t2_condbranch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
index 985097fc3281051..44695a86c4e36c7 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
@@ -164,6 +164,12 @@ unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
return ELF::R_ARM_LDRS_PC_G0;
case ARM::fixup_t2_ldst_pcrel_12:
return ELF::R_ARM_THM_PC12;
+ case ARM::fixup_arm_adr_pcrel_12:
+ return ELF::R_ARM_ALU_PC_G0;
+ case ARM::fixup_thumb_adr_pcrel_10:
+ return ELF::R_ARM_THM_PC8;
+ case ARM::fixup_t2_adr_pcrel_12:
+ return ELF::R_ARM_THM_ALU_PREL_11_0;
case ARM::fixup_bf_target:
return ELF::R_ARM_THM_BF16;
case ARM::fixup_bfc_target:
diff --git a/llvm/test/MC/ARM/pcrel-adr16-relocs.s b/llvm/test/MC/ARM/pcrel-adr16-relocs.s
new file mode 100644
index 000000000000000..dd73c9d28a6822b
--- /dev/null
+++ b/llvm/test/MC/ARM/pcrel-adr16-relocs.s
@@ -0,0 +1,23 @@
+@ RUN: llvm-mc -filetype=obj --triple=thumbv6m-none-eabi %s -o %t
+@ RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELOC
+@ RUN: llvm-objdump -d --triple=thumbv6m-none-eabi %t | FileCheck %s --check-prefix=ADDEND
+
+ .section .text._func1, "ax"
+
+ .balign 4
+ .global _func1
+ .type _func1, %function
+_func1:
+ adr r0, _func2
+@ RELOC: R_ARM_THM_PC8
+ bx lr
+
+// Checking the encoding only, as the disassembly is not quite correct here.
+
+// Thumb16 encoding supports only adding of the encoded immediate (not
+// subtracting, see [Arm ARM]), therefore sign change is required if the pcrel
+// offset is negative. This makes the calculation of the addend for
+// R_ARM_THM_PC8 more complex, for details see [ELF for the Arm 32-bit
+// architecture].
+@ ADDEND: a0ff
+
diff --git a/llvm/test/MC/ARM/pcrel-adr32-relocs.s b/llvm/test/MC/ARM/pcrel-adr32-relocs.s
new file mode 100644
index 000000000000000..5e64125b7ea82e8
--- /dev/null
+++ b/llvm/test/MC/ARM/pcrel-adr32-relocs.s
@@ -0,0 +1,20 @@
+@ RUN: llvm-mc -filetype=obj -triple=armv7 %s -o %t
+@ RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELOC
+@ RUN: llvm-objdump -d --triple=armv7 %t | FileCheck %s --check-prefix=ADDEND
+
+ .section .text._func1, "ax"
+
+ .balign 4
+ .global _func1
+ .type _func1, %function
+_func1:
+ adr r0, _func2
+@ RELOC: R_ARM_ALU_PC_G0
+ .thumb
+ adr r0, _func2
+@ RELOC: R_ARM_THM_ALU_PREL_11_0
+ bx lr
+
+@ ADDEND: sub r0, pc, #8
+@ ADDEND-NEXT: adr.w r0, #-4
+
diff --git a/llvm/test/MC/ARM/pcrel-global.s b/llvm/test/MC/ARM/pcrel-global.s
index 15d46cf2063ecff..1e9e6e989356ecc 100644
--- a/llvm/test/MC/ARM/pcrel-global.s
+++ b/llvm/test/MC/ARM/pcrel-global.s
@@ -7,11 +7,9 @@
@ CHECK: There are no relocations in this file.
@ DISASM-LABEL: <bar>:
-@ DISASM-NEXT: adr.w r0, #-4
-@ DISASM-NEXT: adr.w r0, #-8
-@ DISASM-NEXT: ldr r0, [pc, #0x0] @ 0x14 <bar+0xc>
+@ DISASM-NEXT: ldr r0, [pc, #0x0] @ 0x8 <bar+0x4>
@ DISASM-NEXT: add r0, pc
-@ DISASM-NEXT: .word 0xfffffff3
+@ DISASM-NEXT: .word 0xfffffffb
@@ GNU assembler creates an R_ARM_REL32 referencing bar.
@ DISASM-NOT: {{.}}
@@ -20,16 +18,12 @@
.globl foo
foo:
vldr d0, foo @ arm_pcrel_10
-adr r2, foo @ arm_adr_pcrel_12
.thumb
.thumb_func
.type bar, %function
.globl bar
bar:
-adr r0, bar @ thumb_adr_pcrel_10
-adr.w r0, bar @ t2_adr_pcrel_12
-
ldr r0, .LCPI
.LPC0_1:
add r0, pc
diff --git a/llvm/test/MC/ARM/thumb1-relax-adr.s b/llvm/test/MC/ARM/thumb1-relax-adr.s
index fc5c7c39df5ae1c..97b566f4833e632 100644
--- a/llvm/test/MC/ARM/thumb1-relax-adr.s
+++ b/llvm/test/MC/ARM/thumb1-relax-adr.s
@@ -1,6 +1,5 @@
@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
@ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
-@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
.global func1
_func1:
|
…rgets Follow-up on llvm#72873 When ADR/LDR instructions reference a label in a different section, the offset is not known until link time, however, the assembler assumes it can resolve them in some cases. The previous patch addressed the issue for most LDR instructions, focusing on little-endian targets. This patch addresses the remaining work for ADRs and big-endian targets.
eleanor-arm
changed the title
|
@DavidSpickett The big-endian case ended up being much more trivial than expected :) |
…dian targets Address David's review comments
| @ RELOC: R_ARM_THM_PC8 | ||
| bx lr | ||
|
|
||
| // Checking the encoding only, as the disassembly is not quite correct here. |
There was a problem hiding this comment.
"here, it should be..."
There was a problem hiding this comment.
Also open an issue for the problem itself?
|
Thanks for the review! |
|
@eleanor-arm / @smithp35 / @DavidSpickett - I don't think this patch or the proceeding one is valid. I'm motivated by the code added here: https://reviews.freebsd.org/D2035 to FreeBSD where With these patches the compiler always codegen's as if the reference was in another section to the target, even when they are in the same section. That means that the compiler can't use the information as to whether the target is at a higher or lower address to the reference, and it seems that the compiler always codegen's I think that these patches need reworking so that either relocations are not used in the case where the reference and target are in the same section, or a more complicated relocation/relaxation scheme is used to allow the linker to adjust the instruction used. Alternatively, we could replace these patches with something simpler, for example the GNU assembler seems to simply error if the target for an |
|
Apologies I'm going to be away from a work computer for the next couple of weeks due to holiday, can only leave a few comments for now. In theory the assembler fixup and the linker relocation are semantically identical, the linker is expected to convert the add into a sub and vice-versa regardless of what the object file uses (https://github.com/llvm/llvm-project/blob/main/lld/ELF/Arch/ARM.cpp#L467C25). If there is an out of range error then something else may have gone wrong. Either the assembler is not writing out the object file correctly, or the linker is not resolving the relocation correctly. Are you able to help with a reproducer either for the assembler or the linker? I think Free BSD uses lld as the linker? I agree that it would be good to have the relocation resolved at assembly time if in the same section. From memory this wasn't easy to do for just these relocations in https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCAssembler.cpp#L248. Looking at the history of this problem, there is an epic thread in https://reviews.llvm.org/D72892 which looks like it involved these same BSD source files. While I think doing the extra work to either emit an error message or do the fixup (for just these relocations when the fixup is in the same section) is possible. Given that the assembler/linker combination should work, I'd prefer to get to the bottom of why this case is failing first. |
Thanks for the quick reply. I am also away until the new year now so progress on this will have to wait until then.
Yes, I believe that FreeBSD supports linking with LLD; I'm unsure as to whether they also support the GNU linkers? @emaste might know more? Thanks for the clarification on the relocation behaviour. I'm afraid that my arm/thumb knowledge is very rusty indeed and I didn't spot this in LLD ( was also in a bit of a rush before the holidays) - apologies.
Thanks for the thread link. I'll read it.
I should be able to come up with a reproducer based on that FreeBSD code. I'll work on supplying that when I get back in the new year. |
Actually, I want to clarify that the error I saw reported was: Given that, as you pointed out, the relocation patching in LLD should be equivalent to the assembler resolving the reference this could be caused by user error on my part. |
|
Thanks for the update, I think this needs an update in LLD. Essentially letting it know that R_ARM_ALU_PC_G0 is position independent so it can be used in PIC/PIE. Should be a quick fix. |
Thanks! Reproducer is here: https://godbolt.org/z/85KzbYcsn. |
Removes logic that caused some fixups to be marked as resolved in the assembler without actually resolving them. Sometimes to resolve these fixups a relocation was needed, which was not generated, resulting in invalid code. Now in situations when the fixup cannot be resolved by the assembler, either a relocation is generated, or an error is produced. This was partially addressed previously (llvm#72873, llvm#73834) specifically for LDRx and ADR instructions, This patch expands it LDRD and VLDR, which should cover all instructions affected.
[llvm][MC][ARM] Don't autoresolve fixups Removes logic that caused some fixups to be marked as resolved in the assembler without actually resolving them. Assembler must either resolve the fixup, reject the code (error out) or defer the resolution to the linker. In general assembler can resolve offsets in pcrel instructions if the symbol referred to is in the same section and it cannot make assumptions about relative position of sections. For example, when LDRD instruction in arm mode is encountered, fixup_arm_pcrel_10_unscaled is raised. Prior to llvm#72873 the assembler would always mark it as resolved without generating a relocation. The resulting code would likely be incorrect whenever the label referred to is in a different section. This patch finishes the series to prevent incorrect code being generated for pcrel instructions referring to labels in different sections. This kind of assembly code is very rare and most likely a user error, so both options (relocation or error) are acceptable. Previously this was resolved by adding relocations. Here, for VLDR instructions an error is generated as relocation does not exist for Thumb mode and we wanted the tool's behaviour to be consistent across modes. In the LDRD case, Thumb mode does not have a relocation and errors out, but LDRD in Arm mode generates relocation because its fixup kind is shared with other instructions. Patch series: llvm#72873 - LDRx llvm#73834 - ADR this PR - LDRD and VLDR
|
The fix is here - #77304 |
|
@eleanor-arm, @smithp35 thanks for the fix :) Unfortunately, our CI shows failures with other relocation types. I'm sorry for not reporting this previously. For example, in locore-v6.S see: https://godbolt.org/z/1reqb1xE6 I am also seeing similar errors for R_ARM_THM_MOVW_ABS_NC and R_ARM_THM_MOVT_ABS relocations. I'll work on getting a full set of errors out of our CI and reported to you. Although, it might be better if we could somehow identify the possible set of affected relocation types without experimentation? |
|
Looking at the compiler explorer clang for this target is passing the linker -pie by default. I think this may be the root cause of the CI failures as it looks like some position dependent code is now being linked into a position independent executable. The recent changes to the assembler don't affect any of these relocations I can reproduce this with clang-10 and lld-10 from ubuntu 20.04. If your CI code is kernel code that isn't position independent then you'll need to pass -no-pie to the compiler. From compiler explorer adding -v to the command line (see -pie) The example you have there is not PIE. There is a non-relative relocation in the .text segment (R_ARM_ABS32 is put the absolute address of the symbol into the .word) in the example. By default LLD will accept R_ARM_ABS32 in .data and .data.rel.ro but not .text (it will with -z notext) R_ARM_THM_MOVW_ABS_NC and R_ARM_THM_MOVT_ABS are also non-relative relocations that can't be used in -pie or -pic. There are PC-relative equivalents R_ARM_MOVW_PREL_NC and R_ARM_THM_MOVT_PREL that can be used instead. |
Thanks Peter. Very much appreciated. This needs further investigation from our side. |
Removes logic that caused some fixups to be marked as resolved in the assembler without actually resolving them. Assembler must either resolve the fixup, reject the code (error out) or defer the resolution to the linker. In general assembler can resolve offsets in pcrel instructions if the symbol referred to is in the same section and it cannot make assumptions about relative position of sections. For example, when LDRD instruction in arm mode is encountered, fixup_arm_pcrel_10_unscaled is raised. Prior to #72873 the assembler would always mark it as resolved without generating a relocation. The resulting code would likely be incorrect whenever the label referred to is in a different section. This patch finishes the series to prevent incorrect code being generated for pcrel instructions referring to labels in different sections. This kind of assembly code is very rare and most likely a user error, so both options (relocation or error) are acceptable. In previous patches this was resolved by adding relocations. Here, for VLDR instructions an error is generated as relocation does not exist for Thumb mode and we wanted the tool's behaviour to be consistent across modes. In the LDRD case, Thumb mode does not have a relocation and errors out, but LDRD in Arm mode generates R_ARM_LDRS_PC_G0 relocation because its fixup kind is shared with other instructions. It also fixed the case when ADR is used in the big-endian mode, which is not covered by the ADR patch. Patch series: #72873 - LDRx #73834 - ADR this PR - LDRD and VLDR
@smithp35 thanks for the information on this. To update you, it seems that the problems with the ABS relocations began with https://github.sie.sony.com/SIE-Private/cpu-toolchain-ppr/commit/42e4967140e345923a43f809ba69be57200f46ae. Unfortunately, that was close enough to this change to be tested together on CI. |
Removes logic that caused some fixups to be marked as resolved in the assembler without actually resolving them. Assembler must either resolve the fixup, reject the code (error out) or defer the resolution to the linker. In general assembler can resolve offsets in pcrel instructions if the symbol referred to is in the same section and it cannot make assumptions about relative position of sections. For example, when LDRD instruction in arm mode is encountered, fixup_arm_pcrel_10_unscaled is raised. Prior to llvm#72873 the assembler would always mark it as resolved without generating a relocation. The resulting code would likely be incorrect whenever the label referred to is in a different section. This patch finishes the series to prevent incorrect code being generated for pcrel instructions referring to labels in different sections. This kind of assembly code is very rare and most likely a user error, so both options (relocation or error) are acceptable. In previous patches this was resolved by adding relocations. Here, for VLDR instructions an error is generated as relocation does not exist for Thumb mode and we wanted the tool's behaviour to be consistent across modes. In the LDRD case, Thumb mode does not have a relocation and errors out, but LDRD in Arm mode generates R_ARM_LDRS_PC_G0 relocation because its fixup kind is shared with other instructions. It also fixed the case when ADR is used in the big-endian mode, which is not covered by the ADR patch. Patch series: llvm#72873 - LDRx llvm#73834 - ADR this PR - LDRD and VLDR
