[PATCH] D149367: Emit the CodeView `S_ARMSWITCHTABLE` debug symbol for jump tables
Daniel Paoliello via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 5 12:18:38 PDT 2023
dpaoliello marked 4 inline comments as done.
dpaoliello added inline comments.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3491
+ "EK_Inline, EK_Custom32, EK_GPRel32BlockAddress, and "
+ "EK_GPRel64BlockAddress should never be emitted for COFF");
+ case MachineJumpTableInfo::EK_BlockAddress:
----------------
efriedma wrote:
> dpaoliello wrote:
> > efriedma wrote:
> > > dpaoliello wrote:
> > > > efriedma wrote:
> > > > > dpaoliello wrote:
> > > > > > efriedma wrote:
> > > > > > > LLVM can target 32-bit ARM Windows.
> > > > > > I wasn't seeing ARM32 hit this code before, but it looks like it was because of the assumption that the branch instruction wasn't the one to use the jump table value.
> > > > > >
> > > > > > I've fixed the code to handle ARM32 and added it to the tests, but there seems to be some bug where the offsets are always 0? If I run the test via `llc` then the correct labels appear in the debug info, but somewhere between `llc -filetype=obj` and `llvm-readobj` the offsets are lost...
> > > > > In general, .secrel32 emits a relocation; `llvm-readobj --codeview` won't dump it, I think, but it should still be there. What's the relocation pointing to?
> > > > The "Branch" and "Base" are pointing to a label for the `ADD` instruction, and "Table" is pointing to the Jump Table itself:
> > > >
> > > > If I run `llc -mtriple=thumbv7a-windows < llvm\test\DebugInfo\COFF\jump-table.ll"` I get:
> > > >
> > > > ```
> > > > .text
> > > > .syntax unified
> > > > .file "jump-table.cpp"
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > '+cx8' is not a recognized feature for this target (ignoring feature)
> > > > '+fxsr' is not a recognized feature for this target (ignoring feature)
> > > > '+mmx' is not a recognized feature for this target (ignoring feature)
> > > > '+sse' is not a recognized feature for this target (ignoring feature)
> > > > '+sse2' is not a recognized feature for this target (ignoring feature)
> > > > '+x87' is not a recognized feature for this target (ignoring feature)
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > '+cx8' is not a recognized feature for this target (ignoring feature)
> > > > '+fxsr' is not a recognized feature for this target (ignoring feature)
> > > > '+mmx' is not a recognized feature for this target (ignoring feature)
> > > > '+sse' is not a recognized feature for this target (ignoring feature)
> > > > '+sse2' is not a recognized feature for this target (ignoring feature)
> > > > '+x87' is not a recognized feature for this target (ignoring feature)
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > 'x86-64' is not a recognized processor for this target (ignoring processor)
> > > > .def func;
> > > > .scl 2;
> > > > .type 32;
> > > > .endef
> > > > .globl func @ -- Begin function func
> > > > .p2align 2
> > > > .code16 @ @func
> > > > .thumb_func
> > > > func:
> > > > $Mfunc_begin0:
> > > > .cv_func_id 0
> > > > .cv_file 1 "C:\\llvm\\jump-table.cpp" "35610C7104C8080F83E2BF6A02DABFC9" 1
> > > > .cv_loc 0 1 6 0 @ .\jump-table.cpp:6:0
> > > > @ %bb.0:
> > > > push {r7, lr}
> > > > sub sp, #8
> > > > str r0, [sp, #4]
> > > > $Mtmp0:
> > > > .cv_loc 0 1 7 0 @ .\jump-table.cpp:7:0
> > > > ldr r0, [sp, #4]
> > > > cmp r0, #3
> > > > bhi ($MBB0_7)
> > > > @ %bb.1:
> > > > $Mtmp1:
> > > > .p2align 2
> > > > add r0, pc
> > > > ldrb r0, [r0, #4]
> > > > lsls r0, r0, #1
> > > > .LCPI0_0:
> > > > add pc, r0
> > > > @ %bb.2:
> > > > .p2align 2
> > > > .LJTI0_0:
> > > > .byte (($MBB0_3)-(.LCPI0_0+4))/2
> > > > .byte (($MBB0_4)-(.LCPI0_0+4))/2
> > > > .byte (($MBB0_5)-(.LCPI0_0+4))/2
> > > > .byte (($MBB0_6)-(.LCPI0_0+4))/2
> > > > .p2align 1
> > > > $MBB0_3:
> > > > $Mtmp2:
> > > > .cv_loc 0 1 8 0 @ .\jump-table.cpp:8:0
> > > > bl f1
> > > > b ($MBB0_7)
> > > > $MBB0_4:
> > > > .cv_loc 0 1 9 0 @ .\jump-table.cpp:9:0
> > > > bl f2
> > > > b ($MBB0_7)
> > > > $MBB0_5:
> > > > .cv_loc 0 1 10 0 @ .\jump-table.cpp:10:0
> > > > bl f3
> > > > b ($MBB0_7)
> > > > $MBB0_6:
> > > > .cv_loc 0 1 11 0 @ .\jump-table.cpp:11:0
> > > > bl f4
> > > > b ($MBB0_7)
> > > > $Mtmp3:
> > > > $MBB0_7:
> > > > .cv_loc 0 1 13 0 @ .\jump-table.cpp:13:0
> > > > ldr r0, [sp, #4]
> > > > subs r0, r0, #1
> > > > cmp r0, #4
> > > > bhi ($MBB0_15)
> > > > @ %bb.8:
> > > > $Mtmp4:
> > > > .p2align 2
> > > > add r0, pc
> > > > ldrb r0, [r0, #4]
> > > > lsls r0, r0, #1
> > > > .LCPI0_1:
> > > > add pc, r0
> > > > @ %bb.9:
> > > > .p2align 2
> > > > .LJTI0_1:
> > > > .byte (($MBB0_10)-(.LCPI0_1+4))/2
> > > > .byte (($MBB0_11)-(.LCPI0_1+4))/2
> > > > .byte (($MBB0_12)-(.LCPI0_1+4))/2
> > > > .byte (($MBB0_13)-(.LCPI0_1+4))/2
> > > > .byte (($MBB0_14)-(.LCPI0_1+4))/2
> > > > .p2align 1
> > > > $MBB0_10:
> > > > $Mtmp5:
> > > > .cv_loc 0 1 14 0 @ .\jump-table.cpp:14:0
> > > > bl f2
> > > > b ($MBB0_15)
> > > > $MBB0_11:
> > > > .cv_loc 0 1 15 0 @ .\jump-table.cpp:15:0
> > > > bl f3
> > > > b ($MBB0_15)
> > > > $MBB0_12:
> > > > .cv_loc 0 1 16 0 @ .\jump-table.cpp:16:0
> > > > bl f4
> > > > b ($MBB0_15)
> > > > $MBB0_13:
> > > > .cv_loc 0 1 17 0 @ .\jump-table.cpp:17:0
> > > > bl f5
> > > > b ($MBB0_15)
> > > > $MBB0_14:
> > > > .cv_loc 0 1 18 0 @ .\jump-table.cpp:18:0
> > > > bl f1
> > > > b ($MBB0_15)
> > > > $Mtmp6:
> > > > $MBB0_15:
> > > > .cv_loc 0 1 20 0 @ .\jump-table.cpp:20:0
> > > > add sp, #8
> > > > pop {r7}
> > > > pop {r0}
> > > > mov lr, r0
> > > > bx lr
> > > > $Mtmp7:
> > > > $Mfunc_end0:
> > > > @ -- End function
> > > > .section .debug$S,"dr"
> > > > .p2align 2, 0x0
> > > > .long 4 @ Debug section magic
> > > > .long 241
> > > > .long ($Mtmp9)-($Mtmp8) @ Subsection size
> > > > $Mtmp8:
> > > > .short ($Mtmp11)-($Mtmp10) @ Record length
> > > > $Mtmp10:
> > > > .short 4353 @ Record kind: S_OBJNAME
> > > > .long 0 @ Signature
> > > > .byte 0 @ Object name
> > > > .p2align 2, 0x0
> > > > $Mtmp11:
> > > > .short ($Mtmp13)-($Mtmp12) @ Record length
> > > > $Mtmp12:
> > > > .short 4412 @ Record kind: S_COMPILE3
> > > > .long 16385 @ Flags and language
> > > > .short 244 @ CPUType
> > > > .short 15 @ Frontend version
> > > > .short 0
> > > > .short 1
> > > > .short 0
> > > > .short 17000 @ Backend version
> > > > .short 0
> > > > .short 0
> > > > .short 0
> > > > .asciz "clang version 15.0.1" @ Null-terminated compiler version string
> > > > .p2align 2, 0x0
> > > > $Mtmp13:
> > > > $Mtmp9:
> > > > .p2align 2, 0x0
> > > > .long 241 @ Symbol subsection for func
> > > > .long ($Mtmp15)-($Mtmp14) @ Subsection size
> > > > $Mtmp14:
> > > > .short ($Mtmp17)-($Mtmp16) @ Record length
> > > > $Mtmp16:
> > > > .short 4423 @ Record kind: S_GPROC32_ID
> > > > .long 0 @ PtrParent
> > > > .long 0 @ PtrEnd
> > > > .long 0 @ PtrNext
> > > > .long ($Mfunc_end0)-func @ Code size
> > > > .long 0 @ Offset after prologue
> > > > .long 0 @ Offset before epilogue
> > > > .long 4098 @ Function type index
> > > > .secrel32 func @ Function section relative address
> > > > .secidx func @ Function section index
> > > > .byte 0 @ Flags
> > > > .asciz "func" @ Function name
> > > > .p2align 2, 0x0
> > > > $Mtmp17:
> > > > .short ($Mtmp19)-($Mtmp18) @ Record length
> > > > $Mtmp18:
> > > > .short 4114 @ Record kind: S_FRAMEPROC
> > > > .long 16 @ FrameSize
> > > > .long 0 @ Padding
> > > > .long 0 @ Offset of padding
> > > > .long 0 @ Bytes of callee saved registers
> > > > .long 0 @ Exception handler offset
> > > > .short 0 @ Exception handler section
> > > > .long 90112 @ Flags (defines frame register)
> > > > .p2align 2, 0x0
> > > > $Mtmp19:
> > > > .short ($Mtmp21)-($Mtmp20) @ Record length
> > > > $Mtmp20:
> > > > .short 4414 @ Record kind: S_LOCAL
> > > > .long 116 @ TypeIndex
> > > > .short 1 @ Flags
> > > > .asciz "i"
> > > > .p2align 2, 0x0
> > > > $Mtmp21:
> > > > .cv_def_range $Mtmp0 $Mtmp7, reg_rel, 23, 0, 4
> > > > .short ($Mtmp23)-($Mtmp22) @ Record length
> > > > $Mtmp22:
> > > > .short 4441 @ Record kind: S_ARMSWITCHTABLE
> > > > .secrel32 .LCPI0_0 @ Base offset
> > > > .secidx .LCPI0_0 @ Base section index
> > > > .short 7 @ Switch type
> > > > .secrel32 .LCPI0_0 @ Branch offset
> > > > .secrel32 .LJTI0_0 @ Table offset
> > > > .secidx .LCPI0_0 @ Branch section index
> > > > .secidx .LJTI0_0 @ Table section index
> > > > .long 4 @ Entries count
> > > > .p2align 2, 0x0
> > > > $Mtmp23:
> > > > .short ($Mtmp25)-($Mtmp24) @ Record length
> > > > $Mtmp24:
> > > > .short 4441 @ Record kind: S_ARMSWITCHTABLE
> > > > .secrel32 .LCPI0_1 @ Base offset
> > > > .secidx .LCPI0_1 @ Base section index
> > > > .short 7 @ Switch type
> > > > .secrel32 .LCPI0_1 @ Branch offset
> > > > .secrel32 .LJTI0_1 @ Table offset
> > > > .secidx .LCPI0_1 @ Branch section index
> > > > .secidx .LJTI0_1 @ Table section index
> > > > .long 5 @ Entries count
> > > > .p2align 2, 0x0
> > > > $Mtmp25:
> > > > .short 2 @ Record length
> > > > .short 4431 @ Record kind: S_PROC_ID_END
> > > > $Mtmp15:
> > > > .p2align 2, 0x0
> > > > .cv_linetable 0, func, $Mfunc_end0
> > > > .cv_filechecksums @ File index to string table offset subsection
> > > > .cv_stringtable @ String table
> > > > .long 241
> > > > .long ($Mtmp27)-($Mtmp26) @ Subsection size
> > > > $Mtmp26:
> > > > .short ($Mtmp29)-($Mtmp28) @ Record length
> > > > $Mtmp28:
> > > > .short 4428 @ Record kind: S_BUILDINFO
> > > > .long 4102 @ LF_BUILDINFO index
> > > > .p2align 2, 0x0
> > > > $Mtmp29:
> > > > $Mtmp27:
> > > > .p2align 2, 0x0
> > > > .section .debug$T,"dr"
> > > > .p2align 2, 0x0
> > > > .long 4 @ Debug section magic
> > > > @ ArgList (0x1000)
> > > > .short 0xa @ Record length
> > > > .short 0x1201 @ Record kind: LF_ARGLIST
> > > > .long 0x1 @ NumArgs
> > > > .long 0x74 @ Argument: int
> > > > @ Procedure (0x1001)
> > > > .short 0xe @ Record length
> > > > .short 0x1008 @ Record kind: LF_PROCEDURE
> > > > .long 0x3 @ ReturnType: void
> > > > .byte 0x0 @ CallingConvention: NearC
> > > > .byte 0x0 @ FunctionOptions
> > > > .short 0x1 @ NumParameters
> > > > .long 0x1000 @ ArgListType: (int)
> > > > @ FuncId (0x1002)
> > > > .short 0x12 @ Record length
> > > > .short 0x1601 @ Record kind: LF_FUNC_ID
> > > > .long 0x0 @ ParentScope
> > > > .long 0x1001 @ FunctionType: void (int)
> > > > .asciz "func" @ Name
> > > > .byte 243
> > > > .byte 242
> > > > .byte 241
> > > > @ StringId (0x1003)
> > > > .short 0xe @ Record length
> > > > .short 0x1605 @ Record kind: LF_STRING_ID
> > > > .long 0x0 @ Id
> > > > .asciz "C:\\llvm" @ StringData
> > > > @ StringId (0x1004)
> > > > .short 0x16 @ Record length
> > > > .short 0x1605 @ Record kind: LF_STRING_ID
> > > > .long 0x0 @ Id
> > > > .asciz "jump-table.cpp" @ StringData
> > > > .byte 241
> > > > @ StringId (0x1005)
> > > > .short 0xa @ Record length
> > > > .short 0x1605 @ Record kind: LF_STRING_ID
> > > > .long 0x0 @ Id
> > > > .byte 0 @ StringData
> > > > .byte 243
> > > > .byte 242
> > > > .byte 241
> > > > @ BuildInfo (0x1006)
> > > > .short 0x1a @ Record length
> > > > .short 0x1603 @ Record kind: LF_BUILDINFO
> > > > .short 0x5 @ NumArgs
> > > > .long 0x1003 @ Argument: C:\llvm
> > > > .long 0x0 @ Argument
> > > > .long 0x1004 @ Argument: jump-table.cpp
> > > > .long 0x1005 @ Argument
> > > > .long 0x0 @ Argument
> > > > .byte 242
> > > > .byte 241
> > > > ```
> > > That makes sense... I think it's pointing to the right location, there just isn't any offset because there's a symbol at that address. (Arguably there shouldn't be such a symbol, but that's not really related to your patch.) On other targets, the closest symbol is probably the function entry point.
> > >
> > > Maybe we could teach llvm-readobj to dump this in a more useful way; instead of just dumping the offset, it could also dump any related relocations. But it doesn't look like the compiler is doing anything wrong.
> > @efriedma I'd prefer if we didn't block this change because of the ARM32 issue: LLVM has never fully supported ARM32 Windows (for example exception unwinding isn't implemented) and from a Microsoft perspective we haven't shipped an ARM32 OS since Windows 10 IoT Core in 2018 and ARM32 support is being removed from Windows 11 (https://learn.microsoft.com/en-us/windows/arm/arm32-to-arm64)
> My point was, if I'm understanding correctly, we're actually generating the correct code; it just looks weird in the dumps. So I don't think we need to do anything... except maybe add a few CHECK lines for the relocations in the regression tests. Teaching the dumping tools to make the dumps look less weird was just a "would be nice" thought.
I've modified the test to check the assembly directly instead of using `llvm-readobj`: this also allows me to verify that the CodeView has the correct labels instead of verifying meaningless offsets.
I added another test case using `llvm-readobj` to make sure that the new symbol can be dumped correctly.
================
Comment at: llvm/test/CodeGen/Mips/jumptable_labels.ll:1
-; RUN: llc -march=mips < %s | FileCheck %s -check-prefix=O32
-; RUN: llc -march=mips64 -target-abi=n32 < %s | FileCheck %s -check-prefix=N32
-; RUN: llc -march=mips64 < %s | FileCheck %s -check-prefix=N64
+; RUN: llc -mtriple=mips-linux < %s | FileCheck %s -check-prefix=O32
+; RUN: llc -mtriple=mips64-linux -target-abi=n32 < %s | FileCheck %s -check-prefix=N32
----------------
efriedma wrote:
> What exactly was going wrong here? I guess we somehow assume we're emitting CodeView debug info, and then things explode?
>
> Can we make the relevant codepaths check getCodeViewFlag() (which is apparently the canonical way to check according to AsmPrinter::doInitialization())?
Ah, ok, adding the jump table debug info node in `TargetLowering::expandIndirectJTBranch` was gated on `isOSWindows` which caused it to still be added for these tests. Changing it to `getCodeViewFlag` fixed the issue without having to change these tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149367/new/
https://reviews.llvm.org/D149367
More information about the llvm-commits
mailing list