[COFF] Make the relocation scanning for CFG more discriminating

[hans]

Summary

link.exe ignores REL32 relocations on 32-bit x86, as well as relocations against non-function symbols such as labels. This makes lld do the same.

[thakis]

lgtm

[hans]

I tried this out on 32-bit Chrome's setup.exe, and the cf function table shrinks from 4302 functions to 1160.

[rnk]

Thanks!

test/COFF/gfids-relocations32.s

11

If readobj could symbolize, perhaps with the export table, we could make this readable.

COFF/Writer.cpp

static void markSymbolsWithRelocations(ObjFile *File,
                                        SymbolRVASet &UsedSymbols) {
   for (Chunk *C : File->getChunks()) {
     // We only care about live section chunks. Common chunks and other chunks
     // don't generally contain relocations.
     SectionChunk *SC = dyn_cast<SectionChunk>(C);
     if (!SC || !SC->isLive())
       continue;
 
-    // Look for relocations in this section against symbols in executable output
-    // sections.
-    for (Symbol *Ref : SC->symbols()) {
-      // FIXME: Do further testing to see if the relocation type matters,
-      // especially for 32-bit where taking the address of something usually
-      // uses an absolute relocation instead of a relative one.
-      if (auto *D = dyn_cast_or_null<Defined>(Ref)) {
+    for (const coff_relocation &Reloc : SC->Relocs) {
+      if (Config->Machine == I386 && Reloc.Type == COFF::IMAGE_REL_I386_REL32)
+        // Ignore relative relocations on x86. On x86_64 they can't be ignored
+        // since they're also used to compute absolute addresses.
+        continue;
+
+      Symbol *Ref = SC->File->getSymbol(Reloc.SymbolTableIndex);
+      if (auto *D = dyn_cast_or_null<DefinedCOFF>(Ref)) {
+        if (D->getCOFFSymbol().getComplexType() != COFF::IMAGE_SYM_DTYPE_FUNCTION)
+          // Ignore relocations against non-functions (e.g. labels).
+          continue;
+
+        // Mark the symbol if it's in an executable section.
         Chunk *RefChunk = D->getChunk();
         OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
         if (OS && OS->Header.Characteristics & IMAGE_SCN_MEM_EXECUTE)
           addSymbolToRVASet(UsedSymbols, D);
       }
     }
   }
 }

test/COFF/gfids-relocations32.s

+# REQUIRES: x86
+# RUN: llvm-mc -triple i686-pc-win32 %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -guard:cf -out:%t.exe -entry:main
+# RUN: llvm-readobj -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK
+
+# Only f and _main should go in the table.
+# (use /lldmap:map.txt to check their addresses).
+#
+# CHECK: GuardFidTable [
+# CHECK-NEXT: 0x401000
+# CHECK-NEXT: 0x401030
+# CHECK-NEXT: ]
+
+# The input was loosly based on studying this program:
+#
+#  void foo() { return; }
+#  void bar() { return; }
+#  int main() {
+#    foo();
+#    void (*arr[])() = { &bar };
+#    (*arr[0])();
+#    return 0;
+#  }
+# cl /c a.cc && dumpbin /disasm a.obj > a.txt &&
+#   link a.obj /guard:cf /map:map.txt && dumpbin /loadconfig a.exe
+
+
+
+        .def    f;
+        .scl    3;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,f
+        .p2align 4
+f:      movl $1, %eax
+        ret
+
+
+        .def    g;
+        .scl    3;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,g
+        .p2align 4
+g:      movl $2, %eax
+        ret
+
+
+        .def    label;
+        .scl    6;     # StorageClass: Label
+        .type   0;     # Type: Not a function.
+        .endef
+        .section       .text,"xr",one_only,label
+        .p2align 4
+label:  ret
+
+
+        .data
+        .globl fp
+        .p2align 4
+fp:     .long f        # DIR32 relocation to function
+        .long label    # DIR32 relocation to label
+
+
+        .def    _main;
+        .scl    2;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,_main
+        .globl  _main
+        .p2align 4
+_main:  call *fp       # DIR32 relocation to data
+        call g         # REL32 relocation to function
+        ret
+
+
+# Load configuration directory entry (winnt.h _IMAGE_LOAD_CONFIG_DIRECTORY32).
+# The linker will define the ___guard_* symbols.
+        .section .rdata,"dr"
+.globl __load_config_used
+__load_config_used:
+        .long 104  # Size.
+        .fill 76, 1, 0
+        .long ___guard_fids_table
+        .long ___guard_fids_count
+        .long ___guard_flags
+        .fill 12, 1, 0

test/COFF/gfids-relocations64.s

+# REQUIRES: x86
+# RUN: llvm-mc -triple x86_64-pc-win32 %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -guard:cf -out:%t.exe -entry:main
+# RUN: llvm-readobj -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK
+
+# f, g, and main go in the table.
+# Including g isn't strictly necessary since it's not an indirect call target,
+# however the linker can't know that because relative relocations are used both
+# for direct calls and for getting the absolute address of a function.
+# (use /lldmap:map.txt to check their addresses).
+#
+# CHECK: GuardFidTable [
+# CHECK-NEXT: 0x140001000
+# CHECK-NEXT: 0x140001010
+# CHECK-NEXT: 0x140001030
+# CHECK-NEXT: ]
+
+        .def    f;
+        .scl    3;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,f
+        .p2align 4
+f:      movl $1, %eax
+        ret
+
+
+        .def    g;
+        .scl    3;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,g
+        .p2align 4
+g:      movl $2, %eax
+        ret
+
+
+        .def    label;
+        .scl    6;     # StorageClass: Label
+        .type   0;     # Type: Not a function.
+        .endef
+        .section       .text,"xr",one_only,label
+        .p2align 4
+label:  ret
+
+
+        .data
+        .globl fp
+        .p2align 4
+fp:     .quad f        # DIR32 relocation to function
+        .quad label    # DIR32 relocation to label
+
+
+        .def    main;
+        .scl    2;
+        .type   32;
+        .endef
+        .section       .text,"xr",one_only,main
+        .globl  main
+        .p2align 4
+main:   call *fp       # DIR32 relocation to data
+        call g         # REL32 relocation to function
+        ret
+
+
+# Load configuration directory entry (winnt.h _IMAGE_LOAD_CONFIG_DIRECTORY64).
+# The linker will define the __guard_* symbols.
+        .section .rdata,"dr"
+.globl _load_config_used
+_load_config_used:
+        .long 256
+        .fill 124, 1, 0
+        .quad __guard_fids_table
+        .quad __guard_fids_count
+        .long __guard_flags
+        .fill 128, 1, 0