[aarch64/mac] Correctly disassemble @TLVPPAGE(OFF) relocs

[thakis]

Summary

llvm-otool -tV foo.o and llvm-objdump --macho -d foo.o would
previously fail on object files containing @TLVPPAGE or @TLVPPAGEOFF relocs.

Move llvm-objdump-specific test from
llvm/test/MC/AArch64/arm64-tls-modifiers-darwin.s to new
llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.test
and put test for this fix to that new file.

Fixes PR52356.

[thakis]

maskray, would you be comfortable reviewing this?

[MaskRay]

llvm-objdump --macho -d can reproduce the issue as well. Perhaps perfer llvm-objdump which is used more frequently in tests.

The disassembly logic appears to be llvm/tools/llvm-objdump/MachODump.cpp specific.
Perhaps a better place is llvm/test/tools/llvm-objdump/MachO/ (may need to duplicate the test a bit)

llvm/test/MC/AArch64/arm64-tls-modifiers-darwin.s

11

If you switch to llvm-objdump --macho -d --no-show-raw-insn, you can omit the leading addresses and use CHECK-DIS-NEXT: instead.

[thakis]

move test

[thakis]

Thanks! I moved the test over.

llvm-otool -tV is basically the same as llvm-objdump --macho -d --no-show-raw-insn and shorter (cf parseOtoolOptions() in llvm/tools/llvm-objdump/llvm-objdump.cpp -- absence of -j is --no-show-raw-insn), so I kept the otool invocation. If you feel strongly about changing it to llvm-objdump, I'll change it.

[MaskRay]

In D112843#3118529, @thakis wrote:

Thanks! I moved the test over.

llvm-otool -tV is basically the same as llvm-objdump --macho -d --no-show-raw-insn and shorter (cf parseOtoolOptions() in llvm/tools/llvm-objdump/llvm-objdump.cpp -- absence of -j is --no-show-raw-insn), so I kept the otool invocation. If you feel strongly about changing it to llvm-objdump, I'll change it.

Not strong. I don't know which of llvm-otool and llvm-objdump should be canonical now...

llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.test

1

For newer tests in many test/tools directories (and lld/test), comments start with two comment markers, i.e. ;;

7

Prefer using llvm-mc to create the object file. It does duplicate a bit of work which is tested in test/MC, but it makes the test much easier to update. For test/tools directories we try very hard to avoid prebuilt binaries.

12

May be better checking the assembly as well to show that the relocation is attached to the correct instruction.
Then you'll need -NEXT:

[lhames]

Late to the party, sorry.

MaskRay's feedback covers all my questions, so as long as that's addressed -- LGTM. Thanks for working on this Nico! (and for the review MaskRay)

[thakis]

(will update in a bit)

llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.test

7

Will do in a bit. _All_ tests in llvm/test/tools/llvm-objdump/MachO currently use checked in binaries as far as I can tell, that's why I went with this.

12

Will do, but this part is just moved over from the other test.

[thakis]

tweak test

[MaskRay]

LGTM. @jhenderson usually wants to review llvm-objdump changes.

[jhenderson]

I've got nothing else really to add. LGTM, with or without my test comment addressed.

llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.s

7–9 ↗

(On Diff #385965)

Optional, but you could slightly simplify the prefixes by dropping "CHECK-". Also, I have a personal preference for the inline edit for continuing RUN lines. The logic is that the indentation makes it clear that the second line is a continuation, whilst the trailing | makes it clear that the first line is continued - again, entirely optional though.

[thakis]

llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.s

7–9 ↗

(On Diff #385965)

Dropped the CHECK prefix, kept the indentation :) Thanks!

llvm/lib/Target/AArch64/Disassembler/AArch64ExternalSymbolizer.cpp

case LLVMDisassembler_VariantKind_ARM64_PAGE:
     return MCSymbolRefExpr::VK_PAGE;
   case LLVMDisassembler_VariantKind_ARM64_PAGEOFF:
     return MCSymbolRefExpr::VK_PAGEOFF;
   case LLVMDisassembler_VariantKind_ARM64_GOTPAGE:
     return MCSymbolRefExpr::VK_GOTPAGE;
   case LLVMDisassembler_VariantKind_ARM64_GOTPAGEOFF:
     return MCSymbolRefExpr::VK_GOTPAGEOFF;
   case LLVMDisassembler_VariantKind_ARM64_TLVP:
+    return MCSymbolRefExpr::VK_TLVPPAGE;
   case LLVMDisassembler_VariantKind_ARM64_TLVOFF:
+    return MCSymbolRefExpr::VK_TLVPPAGEOFF;
   default:
     llvm_unreachable("bad LLVMDisassembler_VariantKind");
   }
 }
 
 /// tryAddingSymbolicOperand - tryAddingSymbolicOperand trys to add a symbolic
 /// operand in place of the immediate Value in the MCInst.  The immediate
 /// Value has not had any PC adjustment made by the caller. If the instruction

llvm/test/MC/AArch64/arm64-tls-modifiers-darwin.s

 ; RUN: llvm-mc -triple=arm64-apple-ios7.0 %s -o - | FileCheck %s
-; RUN: llvm-mc -triple=arm64-apple-ios7.0 -filetype=obj %s -o - | llvm-objdump -r - | FileCheck %s --check-prefix=CHECK-OBJ
 
         adrp x2, _var@TLVPPAGE
         ldr x0, [x15, _var@TLVPPAGEOFF]
         add x30, x0, _var@TLVPPAGEOFF
 ; CHECK: adrp x2, _var@TLVPPAG
 ; CHECK: ldr x0, [x15, _var@TLVPPAGEOFF]
 ; CHECK: add x30, x0, _var@TLVPPAGEOFF
-
-; CHECK-OBJ: 8 ARM64_RELOC_TLVP_LOAD_PAGEOFF12 _var
-; CHECK-OBJ: 4 ARM64_RELOC_TLVP_LOAD_PAGEOFF12 _var
-; CHECK-OBJ: 0 ARM64_RELOC_TLVP_LOAD_PAGE21 _var

llvm/test/tools/llvm-objdump/MachO/disassemble-arm64-tlv-modifers.test

+; %p/Inputs/macho-tlv.o was created with
+; `llvm-mc -triple=arm64-apple-ios7.0 -filetype=obj tlv.s` with this tlv.s:
+;       adrp x2, _var@TLVPPAGE
+;       ldr x0, [x15, _var@TLVPPAGEOFF]
+;       add x30, x0, _var@TLVPPAGEOFF
+
+; RUN: llvm-objdump -r %p/Inputs/macho-tlv.o \
+; RUN:     | FileCheck %s --check-prefix=CHECK-OBJ
+; RUN: llvm-otool -tV %p/Inputs/macho-tlv.o \
+; RUN:     | FileCheck %s --check-prefix=CHECK-DIS
+
+; CHECK-OBJ: 8 ARM64_RELOC_TLVP_LOAD_PAGEOFF12 _var
+; CHECK-OBJ: 4 ARM64_RELOC_TLVP_LOAD_PAGEOFF12 _var
+; CHECK-OBJ: 0 ARM64_RELOC_TLVP_LOAD_PAGE21 _var
+
+; CHECK-DIS:      0000000000000000 adrp x2, _var@TLVPPAGE
+; CHECK-DIS-NEXT: 0000000000000004 ldr x0, [x15, _var@TLVPPAGEOFF]
+; CHECK-DIS-NEXT: 0000000000000008 add x30, x0, _var@TLVPPAGEOFF