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

[Reid Kleckner via Phabricator via llvm-commits]

This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327557: [MC] Always emit relocations for same-section function references (authored by rnk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44485?vs=138421&id=138435#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44485

Files:
  llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
  llvm/trunk/test/MC/COFF/diff.s


Index: llvm/trunk/test/MC/COFF/diff.s
===================================================================
--- llvm/trunk/test/MC/COFF/diff.s
+++ llvm/trunk/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 @@
 # %bb.0:
 	ret
 
+	.globl	_baz
+_baz:
+	calll	_foobar
+	retl
+
 	.data
 	.globl	_rust_crate             # @rust_crate
 	.align	4
@@ -39,6 +39,15 @@
 	.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
Index: llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
===================================================================
--- llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
+++ llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
@@ -697,12 +697,14 @@
 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44485.138435.patch
Type: text/x-patch
Size: 2772 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180314/9a0ab3d0/attachment.bin>