[MC] Always emit relocations for same-section function references

Summary:
We already emit relocations in this case when the "incremental linker
compatible" flag is set, but it turns out these relocations are also
required for /guard:cf. Now that we have two use cases for this
behavior, let's make it unconditional to try to keep things simple.

We never hit this problem in Clang because it always sets the
"incremental linker compatible" flag when targeting MSVC. However, LLD
LTO doesn't set this flag, so we'd get CFG failures at runtime when
using ThinLTO and /guard:cf. We probably don't want LLD LTO to set the
"incremental linker compatible" assembler flag, since this has nothing
to do with incremental linking, and we don't need to timestamp LTO
temporary objects.

Fixes PR36624.

Reviewers: inglorion, espindola, majnemer

Subscribers: mehdi_amini, llvm-commits, hiraditya

Differential Revision: https://reviews.llvm.org/D44485

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@327557 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/MC/WinCOFFObjectWriter.cpp b/lib/MC/WinCOFFObjectWriter.cpp
index 9f1db46..2558410 100644 (file)
--- a/lib/MC/WinCOFFObjectWriter.cpp
+++ b/lib/MC/WinCOFFObjectWriter.cpp
@@ -697,12 +697,14 @@ void WinCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
 bool WinCOFFObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(
     const MCAssembler &Asm, const MCSymbol &SymA, const MCFragment &FB,
     bool InSet, bool IsPCRel) const {
-  // MS LINK expects to be able to replace all references to a function with a
-  // thunk to implement their /INCREMENTAL feature.  Make sure we don't optimize
-  // away any relocations to functions.
+  // Don't drop relocations between functions, even if they are in the same text
+  // section. Multiple Visual C++ linker features depend on having the
+  // relocations present. The /INCREMENTAL flag will cause these relocations to
+  // point to thunks, and the /GUARD:CF flag assumes that it can use relocations
+  // to approximate the set of all address taken functions. LLD's implementation
+  // of /GUARD:CF also relies on the existance of these relocations.
   uint16_t Type = cast<MCSymbolCOFF>(SymA).getType();
-  if (Asm.isIncrementalLinkerCompatible() &&
-      (Type >> COFF::SCT_COMPLEX_TYPE_SHIFT) == COFF::IMAGE_SYM_DTYPE_FUNCTION)
+  if ((Type >> COFF::SCT_COMPLEX_TYPE_SHIFT) == COFF::IMAGE_SYM_DTYPE_FUNCTION)
     return false;
   return MCObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(Asm, SymA, FB,
                                                                 InSet, IsPCRel);

diff --git a/test/MC/COFF/diff.s b/test/MC/COFF/diff.s
index d68e628..f89e4ed 100644 (file)
--- a/test/MC/COFF/diff.s
+++ b/test/MC/COFF/diff.s
@@ -1,19 +1,14 @@
 // RUN: llvm-mc -filetype=obj -triple i686-pc-mingw32 %s | llvm-readobj -s -sr -sd | FileCheck %s
 
+// COFF resolves differences between labels in the same section, unless that
+// label is declared with function type.
+
 .section baz, "xr"
-       .def    X
-       .scl    2;
-       .type   32;
-       .endef
        .globl  X
 X:
        mov     Y-X+42, %eax
        retl
 
-       .def    Y
-       .scl    2;
-       .type   32;
-       .endef
        .globl  Y
 Y:
        retl
@@ -30,6 +25,11 @@ _foobar:                                # @foobar
 # %bb.0:
        ret
 
+       .globl  _baz
+_baz:
+       calll   _foobar
+       retl
+
        .data
        .globl  _rust_crate             # @rust_crate
        .align  4
@@ -39,6 +39,15 @@ _rust_crate:
        .long   _foobar-_rust_crate
        .long   _foobar-_rust_crate
 
+// Even though _baz and _foobar are in the same .text section, we keep the
+// relocation for compatibility with the VC linker's /guard:cf and /incremental
+// flags, even on mingw.
+
+// CHECK:        Name: .text
+// CHECK:        Relocations [
+// CHECK-NEXT:     0x12 IMAGE_REL_I386_REL32 _foobar
+// CHECK-NEXT:   ]
+
 // CHECK:        Name: .data
 // CHECK:        Relocations [
 // CHECK-NEXT:     0x4 IMAGE_REL_I386_DIR32 _foobar