From 5a5ac65768d124d98a10e8520363a0a4be3f4e38 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Wed, 14 Mar 2018 19:24:32 +0000 Subject: [PATCH] [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 llvm-svn: 327557 --- llvm/lib/MC/WinCOFFObjectWriter.cpp | 12 +++++++----- llvm/test/MC/COFF/diff.s | 25 +++++++++++++++++-------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp index 9f1db46..2558410 100644 --- a/llvm/lib/MC/WinCOFFObjectWriter.cpp +++ b/llvm/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(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/llvm/test/MC/COFF/diff.s b/llvm/test/MC/COFF/diff.s index d68e628..f89e4ed 100644 --- a/llvm/test/MC/COFF/diff.s +++ b/llvm/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 -- 2.7.4