[PATCH] D44485: [MC] Always emit relocations for same-section function references
Rafael Ávila de Espíndola via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 14 11:29:19 PDT 2018
rafael added subscribers: junbuml, rafael.
rafael added a comment.
LGTM
Reid Kleckner via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:
> rnk created this revision.
> rnk added reviewers: inglorion, espindola, majnemer.
> Herald added subscribers: hiraditya, mehdi_amini.
>
> 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.
>
> https://reviews.llvm.org/D44485
>
> Files:
>
> llvm/lib/MC/WinCOFFObjectWriter.cpp
> llvm/test/MC/COFF/diff.s
>
> Index: llvm/test/MC/COFF/diff.s
> ===================================================================
>
> - llvm/test/MC/COFF/diff.s +++ 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 @@
> 1. %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/lib/MC/WinCOFFObjectWriter.cpp ===================================================================
> - llvm/lib/MC/WinCOFFObjectWriter.cpp +++ llvm/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);
>
> Index: llvm/test/MC/COFF/diff.s
> ===================================================================
>
> - llvm/test/MC/COFF/diff.s +++ 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 @@
> 1. %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/lib/MC/WinCOFFObjectWriter.cpp ===================================================================
> - llvm/lib/MC/WinCOFFObjectWriter.cpp +++ llvm/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); _______________________________________________ llvm-commits mailing list llvm-commits at lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
https://reviews.llvm.org/D44485
More information about the llvm-commits
mailing list