llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.44k stars 12.16k forks source link

Win64 jump tables should not be executable and should not break SEH unwind info #30836

Open rnk opened 7 years ago

rnk commented 7 years ago
Bugzilla Link 31488
Version trunk
OS Windows NT

Extended Description

r290569 switched Win64 from the PIC to static relocation model. This uncovered some questionable jump table codegen for COFF objects.

The following LLVM IR switch generates a jump table:

target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-msvc19.0.24215"
define void @f(i32 %x) {
entry:
  switch i32 %x, label %sw.epilog [
    i32 0, label %sw.bb
    i32 1, label %sw.bb1
    i32 2, label %sw.bb2
    i32 3, label %sw.bb3
  ]
sw.bb:                                            ; preds = %entry
  tail call void @g(i32 0) #2
  br label %sw.epilog
sw.bb1:                                           ; preds = %entry
  tail call void @g(i32 1) #2
  br label %sw.epilog
sw.bb2:                                           ; preds = %entry
  tail call void @g(i32 2) #2
  br label %sw.epilog
sw.bb3:                                           ; preds = %entry
  tail call void @g(i32 3) #2
  br label %sw.epilog
sw.epilog:                                        ; preds = %entry, %sw.bb3, %sw.bb2, %sw.bb1, %sw.bb
  tail call void @g(i32 10) #2
  ret void
}
declare void @g(i32)

Compiling twice with llc -relocation-model pic and -relocation-model static produces these differing assemblies:

$ diff -u static.s pic.s
--- static.s    2016-12-28 09:44:15.132872900 -0800
+++ pic.s       2016-12-28 09:44:19.827504200 -0800
@@ -18,7 +18,10 @@
        ja      .LBB0_7
 # BB#1:                                 # %entry
        movl    %ecx, %eax
-       jmpq    *.LJTI0_0(,%rax,8)
+       leaq    .LJTI0_0(%rip), %rcx
+       movslq  (%rcx,%rax,4), %rax
+       addq    %rcx, %rax
+       jmpq    *%rax
 .LBB0_2:                                # %sw.bb
        xorl    %ecx, %ecx
        jmp     .LBB0_6
@@ -36,15 +39,14 @@
        movl    $10, %ecx
        addq    $40, %rsp
        jmp     g                       # TAILCALL
-       .section        .rdata,"dr"
-       .p2align        3
+       .p2align        2, 0x90
 .LJTI0_0:
-       .quad   .LBB0_2
-       .quad   .LBB0_3
-       .quad   .LBB0_4
-       .quad   .LBB0_5
+       .long   .LBB0_2-.LJTI0_0
+       .long   .LBB0_3-.LJTI0_0
+       .long   .LBB0_4-.LJTI0_0
+       .long   .LBB0_5-.LJTI0_0
        .seh_handlerdata
-       .section        .rdata,"dr"
+       .text
 .Lcfi3:
        .seh_endproc

Problem one is that .seh_endproc needs to occur in the same section as .seh_proc, and the .rdata switch breaks that.

Problem two is that in today's PIC model, this jump table is unnecessarily in the .text section. MSVC also does this, FWIW, but we can definitely put this jump table in a separate COMDAT-associative .rdata section.

We definitely don't want the .quad codegen, since that will produce runtime relocations and waste space. This leaves two choices: image relative relocations, or '.LBBN_M - .Lfunc_beginN', which can be fixed up by the assembler. MSVC uses image relative relocations, which I suppose are nice if you have some kind of post-link tool, but the fixups seem nicer from an object size perspective.

rnk commented 7 years ago

r290678 fixes the .seh_endproc issue, so we can reland r290569 safely.