[llvm-objdump] --adjust-vma: Call getInstruction with adjusted address #140471
Conversation
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Fangrui Song (MaskRay) Changesllvm-objdump currently calls MCDisassembler::getInstruction with Specify an adjust address to fix SystemZInstPrinter output. The added test utilizes llvm/utils/update_test_body.py to make updates Full diff: https://github.com/llvm/llvm-project/pull/140471.diff 2 Files Affected:
diff --git a/llvm/test/tools/llvm-objdump/ELF/SystemZ/adjust-vma.test b/llvm/test/tools/llvm-objdump/ELF/SystemZ/adjust-vma.test
new file mode 100644
index 0000000000000..9cc93fb1af8d8
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/SystemZ/adjust-vma.test
@@ -0,0 +1,96 @@
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: yaml2obj a.yaml -o out
+# RUN: llvm-objdump -td --adjust-vma=0x200000 --no-show-raw-insn out | FileCheck %s --match-full-lines
+
+# CHECK: SYMBOL TABLE:
+# CHECK-NEXT: 0000000001200104 l .text 0000000000000000 f1
+# CHECK-NEXT: 0000000001200106 l .text 0000000000000000 f2
+# CHECK-NEXT: 0000000000000800 l *ABS* 0000000000000000 abs
+
+# CHECK: 00000000012000f8 <_start>:
+# CHECK-NEXT: 12000f8: brasl %r14, 0x1200104
+# CHECK-NEXT: 12000fe: brasl %r14, 0x1200106
+# CHECK-EMPTY:
+# CHECK-NEXT: 0000000001200104 <f1>:
+# CHECK-NEXT: 1200104: br %r14
+# CHECK-EMPTY:
+# CHECK-NEXT: 0000000001200106 <f2>:
+# CHECK-NEXT: 1200106: br %r14
+
+#--- a.s
+.globl _start
+_start:
+ brasl %r14, f1
+ brasl %r14, f2
+
+f1:
+ br %r14
+f2:
+ br %r14
+
+abs = 0x800
+#--- gen
+LLD_IN_TEST=1 clang --target=s390x-linux -no-pie -nostdlib -Wl,--no-rosegment,-zseparate-code,-znorelro,-znognustack -fuse-ld=lld a.s -o a
+obj2yaml a
+#--- a.yaml
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2MSB
+ Type: ET_EXEC
+ Machine: EM_S390
+ Entry: 0x10000F8
+ProgramHeaders:
+ - Type: PT_PHDR
+ Flags: [ PF_R ]
+ VAddr: 0x1000040
+ Align: 0x8
+ Offset: 0x40
+ - Type: PT_INTERP
+ Flags: [ PF_R ]
+ FirstSec: .interp
+ LastSec: .interp
+ VAddr: 0x10000E8
+ Offset: 0xE8
+ - Type: PT_LOAD
+ Flags: [ PF_X, PF_R ]
+ FirstSec: .interp
+ LastSec: .text
+ VAddr: 0x1000000
+ Align: 0x1000
+ Offset: 0x0
+Sections:
+ - Name: .interp
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ Address: 0x10000E8
+ AddressAlign: 0x1
+ Content: 2F6C69622F6C6436342E736F2E3100
+ - Name: .text
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+ Address: 0x10000F8
+ AddressAlign: 0x4
+ Content: C0E500000006C0E50000000407FE07FE
+ - Name: .comment
+ Type: SHT_PROGBITS
+ Flags: [ SHF_MERGE, SHF_STRINGS ]
+ AddressAlign: 0x1
+ EntSize: 0x1
+ Offset: 0x1000
+ Content: 4C696E6B65723A204C4C442032312E302E3000
+Symbols:
+ - Name: f1
+ Section: .text
+ Value: 0x1000104
+ - Name: f2
+ Section: .text
+ Value: 0x1000106
+ - Name: abs
+ Index: SHN_ABS
+ Value: 0x800
+ - Name: _start
+ Section: .text
+ Binding: STB_GLOBAL
+ Value: 0x10000F8
+...
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 7a778da2d3a49..99bb41f5b95a7 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -2323,7 +2323,7 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
// provided
MCInst Inst;
ArrayRef<uint8_t> ThisBytes = Bytes.slice(Index);
- uint64_t ThisAddr = SectionAddr + Index;
+ uint64_t ThisAddr = SectionAddr + Index + VMAAdjustment;
bool Disassembled = DT->DisAsm->getInstruction(
Inst, Size, ThisBytes, ThisAddr, CommentStream);
if (Size == 0)
|
|
Hi @MaskRay , I'm also confused why SystemZ is supposedly doing anything special here. The only use of the Also, there seem to be several other calls to |
Created using spr 1.3.5-bogner
Branch target addresses are dependent on the current instruction address. It seems that we have two mechanisms. Actually an older interface for Disassembler had existed before my changes, used by SystemZ. I think handling the branch target display in InstPrinter makes more sense, as a decoded instruction
Handling --adjust-vma= with another option is like a whac-a-mole game.... I think we will just make change when we see a new issue. One call is for --symbolize-operands, where the address doesn't seem to matter. Another call is used by AMDGPU, which I do not yet to touch in this PR. In the future we should probably just adjust the variable |
It is indeed true that SystemZ does not yet support The way I see it is that there are two considerations when printing a branch target label:
The first is done via Either way, I think this PR is certainly correct. I will work on supporting |
MaskRay
deleted the
users/MaskRay/spr/llvm-objdump-adjust-vma-call-getinstruction-with-adjusted-address
branch
…sted address llvm-objdump currently calls MCDisassembler::getInstruction with unadjusted address and MCInstPrinter::printInst with adjusted address. The decoded branch targets will be adjusted as expected for most targets (as the getInstruction address is insignificant) but not for SystemZ (where the getInstruction address is displayed). Specify an adjust address to fix SystemZInstPrinter output. The added test utilizes llvm/utils/update_test_body.py to make updates easier and additionally checks that we don't adjust SHN_ABS symbol addresses. Pull Request: llvm/llvm-project#140471
|
Hi there, this PR failed test and breaks our buildbots. Could you please take a look? Thanks!
buildbot: https://lab.llvm.org/buildbot/#/builders/10/builds/5709 |
Sorry. I forgot to add |
Thanks for the quick fix! |
llvm-objdump currently calls MCDisassembler::getInstruction with unadjusted address and MCInstPrinter::printInst with adjusted address. The decoded branch targets will be adjusted as expected for most targets (as the getInstruction address is insignificant) but not for SystemZ (where the getInstruction address is displayed). Specify an adjust address to fix SystemZInstPrinter output. The added test utilizes llvm/utils/update_test_body.py to make updates easier and additionally checks that we don't adjust SHN_ABS symbol addresses. Pull Request: llvm#140471
As noticed in llvm#140471, the SystemZ target currently implements disassembly of PC-relative target addresses differently from other back-ends. This patch brings SystemZ in line with other targets. Specifically, this patch changes the relevant MCInst instructions to carry a PC-relative displacement instead of an absolute target address in their immediate fields. When printing the instruction, this displacement will either be shown as is (e.g. for llvm-mc), or else translated into an absolute address at print time (e.g. for llvm-objdump). The existing llvm-mc based tests using PC-relative operands no longer work and have to be rewritten, but printing displacements makes those tests easier to maintain anyway.
This is now #141064 |
As noticed in #140471, the SystemZ target currently implements disassembly of PC-relative target addresses differently from other back-ends. This patch brings SystemZ in line with other targets. Specifically, this patch changes the relevant MCInst instructions to carry a PC-relative displacement instead of an absolute target address in their immediate fields. When printing the instruction, this displacement will either be shown as is (e.g. for llvm-mc), or else translated into an absolute address at print time (e.g. for llvm-objdump). The existing llvm-mc based tests using PC-relative operands no longer work and have to be rewritten, but printing displacements makes those tests easier to maintain anyway.
…(#141064) As noticed in llvm/llvm-project#140471, the SystemZ target currently implements disassembly of PC-relative target addresses differently from other back-ends. This patch brings SystemZ in line with other targets. Specifically, this patch changes the relevant MCInst instructions to carry a PC-relative displacement instead of an absolute target address in their immediate fields. When printing the instruction, this displacement will either be shown as is (e.g. for llvm-mc), or else translated into an absolute address at print time (e.g. for llvm-objdump). The existing llvm-mc based tests using PC-relative operands no longer work and have to be rewritten, but printing displacements makes those tests easier to maintain anyway.
llvm-objdump currently calls MCDisassembler::getInstruction with unadjusted address and MCInstPrinter::printInst with adjusted address. The decoded branch targets will be adjusted as expected for most targets (as the getInstruction address is insignificant) but not for SystemZ (where the getInstruction address is displayed). Specify an adjust address to fix SystemZInstPrinter output. The added test utilizes llvm/utils/update_test_body.py to make updates easier and additionally checks that we don't adjust SHN_ABS symbol addresses. Pull Request: llvm#140471
As noticed in llvm#140471, the SystemZ target currently implements disassembly of PC-relative target addresses differently from other back-ends. This patch brings SystemZ in line with other targets. Specifically, this patch changes the relevant MCInst instructions to carry a PC-relative displacement instead of an absolute target address in their immediate fields. When printing the instruction, this displacement will either be shown as is (e.g. for llvm-mc), or else translated into an absolute address at print time (e.g. for llvm-objdump). The existing llvm-mc based tests using PC-relative operands no longer work and have to be rewritten, but printing displacements makes those tests easier to maintain anyway.
llvm-objdump currently calls MCDisassembler::getInstruction with unadjusted address and MCInstPrinter::printInst with adjusted address. The decoded branch targets will be adjusted as expected for most targets (as the getInstruction address is insignificant) but not for SystemZ (where the getInstruction address is displayed). Specify an adjust address to fix SystemZInstPrinter output. The added test utilizes llvm/utils/update_test_body.py to make updates easier and additionally checks that we don't adjust SHN_ABS symbol addresses. Pull Request: llvm#140471
As noticed in llvm#140471, the SystemZ target currently implements disassembly of PC-relative target addresses differently from other back-ends. This patch brings SystemZ in line with other targets. Specifically, this patch changes the relevant MCInst instructions to carry a PC-relative displacement instead of an absolute target address in their immediate fields. When printing the instruction, this displacement will either be shown as is (e.g. for llvm-mc), or else translated into an absolute address at print time (e.g. for llvm-objdump). The existing llvm-mc based tests using PC-relative operands no longer work and have to be rewritten, but printing displacements makes those tests easier to maintain anyway.
llvm-objdump currently calls MCDisassembler::getInstruction with
unadjusted address and MCInstPrinter::printInst with adjusted address.
The decoded branch targets will be adjusted as expected for most targets
(as the getInstruction address is insignificant) but not for SystemZ
(where the getInstruction address is displayed).
Specify an adjust address to fix SystemZInstPrinter output.
The added test utilizes llvm/utils/update_test_body.py to make updates
easier and additionally checks that we don't adjust SHN_ABS symbol
addresses.