[PATCH] Use proper section suffix for COFF weak symbols
Rafael Espíndola
rafael.espindola at gmail.com
Thu Jul 18 12:55:53 PDT 2013
The patch itself looks good. Aaron, Michael, can you review the logic
for when the prefix should be used?
On 17 July 2013 18:50, Nico Rieck <nico.rieck at gmail.com> wrote:
> 32-bit symbols have "_" as global prefix, but when forming the name of
> COMDAT sections this prefix is ignored. The current behavior assumes that
> this prefix is always present which is not the case for 64-bit and names
> are truncated.
>
> I've updated the existing test case to emit assembly instead. Should the test be moved to CodeGen (or even to a subdirectory there)?
>
> http://llvm-reviews.chandlerc.com/D1173
>
> Files:
> include/llvm/Target/Mangler.h
> lib/CodeGen/TargetLoweringObjectFileImpl.cpp
> lib/Target/Mangler.cpp
> test/MC/COFF/weak-symbol-section-specification.ll
> test/MC/COFF/weak-symbol.ll
>
> Index: include/llvm/Target/Mangler.h
> ===================================================================
> --- include/llvm/Target/Mangler.h
> +++ include/llvm/Target/Mangler.h
> @@ -59,13 +59,14 @@
> /// and the specified global variable's name. If the global variable doesn't
> /// have a name, this fills in a unique name for the global.
> void getNameWithPrefix(SmallVectorImpl<char> &OutName, const GlobalValue *GV,
> - bool isImplicitlyPrivate);
> + bool isImplicitlyPrivate, bool UseGlobalPrefix = true);
>
> /// getNameWithPrefix - Fill OutName with the name of the appropriate prefix
> /// and the specified name as the global variable name. GVName must not be
> /// empty.
> void getNameWithPrefix(SmallVectorImpl<char> &OutName, const Twine &GVName,
> - ManglerPrefixTy PrefixTy = Mangler::Default);
> + ManglerPrefixTy PrefixTy = Mangler::Default,
> + bool UseGlobalPrefix = true);
> };
>
> } // End llvm namespace
> Index: lib/CodeGen/TargetLoweringObjectFileImpl.cpp
> ===================================================================
> --- lib/CodeGen/TargetLoweringObjectFileImpl.cpp
> +++ lib/CodeGen/TargetLoweringObjectFileImpl.cpp
> @@ -726,9 +726,8 @@
> if (GV->isWeakForLinker()) {
> Selection = COFF::IMAGE_COMDAT_SELECT_ANY;
> Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
> - MCSymbol *Sym = Mang->getSymbol(GV);
> Name.append("$");
> - Name.append(Sym->getName().begin() + 1, Sym->getName().end());
> + Mang->getNameWithPrefix(Name, GV, false, false);
> }
> return getContext().getCOFFSection(Name,
> Characteristics,
> @@ -761,8 +760,7 @@
> if (GV->isWeakForLinker()) {
> const char *Prefix = getCOFFSectionPrefixForUniqueGlobal(Kind);
> SmallString<128> Name(Prefix, Prefix+strlen(Prefix));
> - MCSymbol *Sym = Mang->getSymbol(GV);
> - Name.append(Sym->getName().begin() + 1, Sym->getName().end());
> + Mang->getNameWithPrefix(Name, GV, false, false);
>
> unsigned Characteristics = getCOFFSectionFlags(Kind);
>
> Index: lib/Target/Mangler.cpp
> ===================================================================
> --- lib/Target/Mangler.cpp
> +++ lib/Target/Mangler.cpp
> @@ -105,7 +105,8 @@
> /// and the specified name as the global variable name. GVName must not be
> /// empty.
> void Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
> - const Twine &GVName, ManglerPrefixTy PrefixTy) {
> + const Twine &GVName, ManglerPrefixTy PrefixTy,
> + bool UseGlobalPrefix) {
> SmallString<256> TmpData;
> StringRef Name = GVName.toStringRef(TmpData);
> assert(!Name.empty() && "getNameWithPrefix requires non-empty name");
> @@ -124,13 +125,16 @@
> OutName.append(Prefix, Prefix+strlen(Prefix));
> }
>
> - const char *Prefix = MAI->getGlobalPrefix();
> - if (Prefix[0] == 0)
> - ; // Common noop, no prefix.
> - else if (Prefix[1] == 0)
> - OutName.push_back(Prefix[0]); // Common, one character prefix.
> - else
> - OutName.append(Prefix, Prefix+strlen(Prefix)); // Arbitrary length prefix.
> + if (UseGlobalPrefix) {
> + const char *Prefix = MAI->getGlobalPrefix();
> + if (Prefix[0] == 0)
> + ; // Common noop, no prefix.
> + else if (Prefix[1] == 0)
> + OutName.push_back(Prefix[0]); // Common, one character prefix.
> + else
> + // Arbitrary length prefix.
> + OutName.append(Prefix, Prefix+strlen(Prefix));
> + }
> }
>
> // If this is a simple string that doesn't need escaping, just append it.
> @@ -179,8 +183,8 @@
> /// and the specified global variable's name. If the global variable doesn't
> /// have a name, this fills in a unique name for the global.
> void Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
> - const GlobalValue *GV,
> - bool isImplicitlyPrivate) {
> + const GlobalValue *GV, bool isImplicitlyPrivate,
> + bool UseGlobalPrefix) {
> ManglerPrefixTy PrefixTy = Mangler::Default;
> if (GV->hasPrivateLinkage() || isImplicitlyPrivate)
> PrefixTy = Mangler::Private;
> @@ -190,7 +194,7 @@
> // If this global has a name, handle it simply.
> if (GV->hasName()) {
> StringRef Name = GV->getName();
> - getNameWithPrefix(OutName, Name, PrefixTy);
> + getNameWithPrefix(OutName, Name, PrefixTy, UseGlobalPrefix);
> // No need to do anything else if the global has the special "do not mangle"
> // flag in the name.
> if (Name[0] == 1)
> @@ -202,7 +206,8 @@
> if (ID == 0) ID = NextAnonGlobalID++;
>
> // Must mangle the global into a unique ID.
> - getNameWithPrefix(OutName, "__unnamed_" + Twine(ID), PrefixTy);
> + getNameWithPrefix(OutName, "__unnamed_" + Twine(ID), PrefixTy,
> + UseGlobalPrefix);
> }
>
> // If we are supposed to add a microsoft-style suffix for stdcall/fastcall,
> Index: test/MC/COFF/weak-symbol-section-specification.ll
> ===================================================================
> --- test/MC/COFF/weak-symbol-section-specification.ll
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -; The purpose of this test is to verify that weak linkage type is not ignored by backend,
> -; if section was specialized.
> -
> -; RUN: llc -filetype=obj -mtriple i686-pc-win32 %s -o - | llvm-readobj -s -sd | FileCheck %s
> -
> - at a = weak unnamed_addr constant { i32, i32, i32 } { i32 0, i32 0, i32 0}, section ".data"
> -
> -; CHECK: Name: .data$a
> -; CHECK-NEXT: VirtualSize: 0
> -; CHECK-NEXT: VirtualAddress: 0
> -; CHECK-NEXT: RawDataSize: {{[0-9]+}}
> -; CHECK-NEXT: PointerToRawData: 0x{{[0-9A-F]+}}
> -; CHECK-NEXT: PointerToRelocations: 0x0
> -; CHECK-NEXT: PointerToLineNumbers: 0x0
> -; CHECK-NEXT: RelocationCount: 0
> -; CHECK-NEXT: LineNumberCount: 0
> -; CHECK-NEXT: Characteristics [ (0x40401040)
> -; CHECK-NEXT: IMAGE_SCN_ALIGN_8BYTES
> -; CHECK-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
> -; CHECK-NEXT: IMAGE_SCN_LNK_COMDAT
> -; CHECK-NEXT: IMAGE_SCN_MEM_READ
> -; CHECK-NEXT: ]
> -; CHECK-NEXT: SectionData (
> -; CHECK-NEXT: 0000: 00000000 00000000 00000000
> -; CHECK-NEXT: )
> Index: test/MC/COFF/weak-symbol.ll
> ===================================================================
> --- /dev/null
> +++ test/MC/COFF/weak-symbol.ll
> @@ -0,0 +1,44 @@
> +; Test that weak functions and globals are placed into selectany COMDAT
> +; sections with the mangled name as suffix. Ensure that the weak linkage
> +; type is not ignored by the backend if the section was specialized.
> +;
> +; RUN: llc -mtriple=i686-pc-win32 %s -o - | FileCheck %s --check-prefix=X86
> +; RUN: llc -mtriple=i686-pc-mingw32 %s -o - | FileCheck %s --check-prefix=X86
> +; RUN: llc -mtriple=x86_64-pc-win32 %s -o - | FileCheck %s --check-prefix=X64
> +; RUN: llc -mtriple=x86_64-pc-mingw32 %s -o - | FileCheck %s --check-prefix=X64
> +
> +; Mangled function
> +; X86: .section .text$_Z3foo
> +; X86: .linkonce discard
> +; X86: .globl __Z3foo
> +;
> +; X64: .section .text$_Z3foo
> +; X64: .linkonce discard
> +; X64: .globl _Z3foo
> +define weak void @_Z3foo() {
> + ret void
> +}
> +
> +; Unmangled function
> +; X86: .section .sect$f
> +; X86: .linkonce discard
> +; X86: .globl _f
> +;
> +; X64: .section .sect$f
> +; X64: .linkonce discard
> +; X64: .globl f
> +define weak void @f() section ".sect" {
> + ret void
> +}
> +
> +; Weak global
> +; X86: .section .data$a
> +; X86: .linkonce discard
> +; X86: .globl _a
> +; X86: .zero 12
> +;
> +; X64: .section .data$a
> +; X64: .linkonce discard
> +; X64: .globl a
> +; X64: .zero 12
> + at a = weak unnamed_addr constant { i32, i32, i32 } { i32 0, i32 0, i32 0}, section ".data"
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list