[PATCH] D149367: Emit the CodeView `S_ARMSWITCHTABLE` debug symbol for jump tables

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 9 14:05:37 PDT 2023


efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3483
+            // and the jump table that it uses in the debug info.
+            if (MO.getType() == MachineOperand::MO_JumpTableIndex) {
+              // For label-difference jump tables, find the base expression.
----------------
I'm concerned this could fail to find the jump table, or find the wrong jump table.  It is possible to hoist some jump-table-related operations into an earlier block.  For example:

```
 extern "C" void f1();
 extern "C" void f2();
 extern "C" void f3();
 extern "C" void f4();
 extern "C" void f5();
 extern "C" void func(int i, int j){
   for (int k = 0; k < j; ++k) {
     switch (i) {
         case 0: f1(); break;
         case 1: f2(); break;
         case 2: f3(); break;
         case 3: f4(); break;
     }
   }
 }
```

On AArch64, we produce:

```
// %bb.1:
        mov     w19, w1
        mov     w20, w0
        mov     w21, w0
        adrp    x22, .LJTI0_0
        add     x22, x22, :lo12:.LJTI0_0
        b       .LBB0_4
.LBB0_2:                                // %sw.bb3
                                        //   in Loop: Header=BB0_4 Depth=1
        bl      f4
.LBB0_3:                                // %for.inc
                                        //   in Loop: Header=BB0_4 Depth=1
        subs    w19, w19, #1
        b.eq    .LBB0_9
.LBB0_4:                                // %for.body
                                        // =>This Inner Loop Header: Depth=1
        cmp     w20, #3
        b.hi    .LBB0_3
// %bb.5:                               // %for.body
                                        //   in Loop: Header=BB0_4 Depth=1
        adr     x8, .LBB0_2
        ldrb    w9, [x22, x21]
        add     x8, x8, x9, lsl #2
        br      x8
```

Off the top of my head, I'm not sure how to write a testcase where it actually finds the wrong table, but I suspect it's possible.

We might need to encode the associated jump table into the MachineInstrs some other way.  I'm not sure what that looks like, exactly; maybe a pseudo-instruction just before the jump?


================
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:
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149367/new/

https://reviews.llvm.org/D149367



More information about the llvm-commits mailing list