MC, COFF: Use relocations for function references inside the section

Referencing one symbol from another in the same section does not
generally require a relocation.  However, the MS linker has a feature
called /INCREMENTAL which enables incremental links.  It achieves this
by creating thunks to the actual function and redirecting all
relocations to point to the thunk.

This breaks down with the old scheme if you have a function which
references, say, itself.  On x86_64, we would use %rip relative
addressing to reference the start of the function from out current
position.  This would lead to miscompiles because other references might
reference the thunk instead, breaking function pointer equality.

This fixes PR21520.

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

diff --git a/include/llvm/Object/COFF.h b/include/llvm/Object/COFF.h
index d9c0cd3870df7bafb3aa107f926e18543c6873d1..579cbadbc9454b266a20f73284128a5b30148231 100644 (file)
--- a/include/llvm/Object/COFF.h
+++ b/include/llvm/Object/COFF.h
@@ -299,7 +299,9 @@ public:
 
   uint8_t getBaseType() const { return getType() & 0x0F; }
 
-  uint8_t getComplexType() const { return (getType() & 0xF0) >> 4; }
+  uint8_t getComplexType() const {
+    return (getType() & 0xF0) >> COFF::SCT_COMPLEX_TYPE_SHIFT;
+  }
 
   bool isExternal() const {
     return getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL;

diff --git a/lib/MC/WinCOFFObjectWriter.cpp b/lib/MC/WinCOFFObjectWriter.cpp
index e5f4565dafc1bd2ec8226e12ae237bbeb25c87e5..1046e047161607fc14031b00ad56682fb33c3177 100644 (file)
--- a/lib/MC/WinCOFFObjectWriter.cpp
+++ b/lib/MC/WinCOFFObjectWriter.cpp
@@ -170,6 +170,11 @@ public:
   void ExecutePostLayoutBinding(MCAssembler &Asm,
                                 const MCAsmLayout &Layout) override;
 
+  bool IsSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
+                                              const MCSymbolData &DataA,
+                                              const MCFragment &FB, bool InSet,
+                                              bool IsPCRel) const override;
+
   void RecordRelocation(const MCAssembler &Asm, const MCAsmLayout &Layout,
                         const MCFragment *Fragment, const MCFixup &Fixup,
                         MCValue Target, bool &IsPCRel,
@@ -643,6 +648,19 @@ void WinCOFFObjectWriter::ExecutePostLayoutBinding(MCAssembler &Asm,
       DefineSymbol(SD, Asm, Layout);
 }
 
+bool WinCOFFObjectWriter::IsSymbolRefDifferenceFullyResolvedImpl(
+    const MCAssembler &Asm, const MCSymbolData &DataA, 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.
+  if ((((DataA.getFlags() & COFF::SF_TypeMask) >> COFF::SF_TypeShift) >>
+       COFF::SCT_COMPLEX_TYPE_SHIFT) == COFF::IMAGE_SYM_DTYPE_FUNCTION)
+    return false;
+  return MCObjectWriter::IsSymbolRefDifferenceFullyResolvedImpl(Asm, DataA, FB,
+                                                                InSet, IsPCRel);
+}
+
 void WinCOFFObjectWriter::RecordRelocation(const MCAssembler &Asm,
                                            const MCAsmLayout &Layout,
                                            const MCFragment *Fragment,

diff --git a/lib/MC/WinCOFFStreamer.cpp b/lib/MC/WinCOFFStreamer.cpp
index 32813a1d26313751b053cc1df86b1670c9a63a01..6a8054d7486d8c3403aa87cec50408f25c304549 100644 (file)
--- a/lib/MC/WinCOFFStreamer.cpp
+++ b/lib/MC/WinCOFFStreamer.cpp
@@ -133,7 +133,7 @@ void MCWinCOFFStreamer::EmitCOFFSymbolStorageClass(int StorageClass) {
   if (!CurSymbol)
     FatalError("storage class specified outside of symbol definition");
 
-  if (StorageClass & ~0xff)
+  if (StorageClass & ~COFF::SSC_Invalid)
     FatalError(Twine("storage class value '") + itostr(StorageClass) +
                "' out of range");
 

diff --git a/test/MC/COFF/simple-fixups.s b/test/MC/COFF/simple-fixups.s
index 2a74f21f12d030e119d7a3cea4b6f0a73d81c299..cb5d7642ee6ba006f747119496c65b2310293310 100644 (file)
--- a/test/MC/COFF/simple-fixups.s
+++ b/test/MC/COFF/simple-fixups.s
@@ -1,5 +1,6 @@
-// The purpose of this test is to verify that we do not produce unneeded
-// relocations when symbols are in the same section and we know their offset.
+// The purpose of this test is to verify that we produce relocations for
+// references to functions.  Failing to do so might cause pointer-to-function
+// equality to fail if /INCREMENTAL links are used.
 
 // RUN: llvm-mc -filetype=obj -triple i686-pc-win32 %s | llvm-readobj -s | FileCheck %s
 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-win32 %s | llvm-readobj -s | FileCheck %s
@@ -46,4 +47,4 @@ Ltmp0:
        ret
 
 // CHECK:     Sections [
-// CHECK-NOT: RelocationCount: {{[^0]}}
+// CHECK: RelocationCount: 1