From c79dd2f80ac78b262944a55a6e8eaee3c62ea7a5 Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Fri, 7 Mar 2014 23:05:10 +0000 Subject: [PATCH] [PECOFF] Support a new type of weak symbol. Summary: COMDAT_SELECT_SAME_SIZE is a COMDAT type that I presume exist only in COFF. The semantics of the type is that linker should merge such COMDAT sections if their sizes are the same. Otherwise it's an error. Reviewers: Bigcheese, shankarke, kledzik CC: llvm-commits Differential Revision: http://llvm-reviews.chandlerc.com/D2996 llvm-svn: 203308 --- lld/include/lld/Core/DefinedAtom.h | 9 ++-- lld/lib/Core/SymbolTable.cpp | 42 ++++++++++--------- lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp | 4 +- .../ReaderWriter/YAML/ReaderWriterYAML.cpp | 4 +- .../pecoff/Inputs/merge-same-size1.obj.yaml | 25 +++++++++++ .../pecoff/Inputs/merge-same-size2.obj.yaml | 25 +++++++++++ lld/test/pecoff/merge-same-size.test | 24 +++++++++++ 7 files changed, 107 insertions(+), 26 deletions(-) create mode 100644 lld/test/pecoff/Inputs/merge-same-size1.obj.yaml create mode 100644 lld/test/pecoff/Inputs/merge-same-size2.obj.yaml create mode 100644 lld/test/pecoff/merge-same-size.test diff --git a/lld/include/lld/Core/DefinedAtom.h b/lld/include/lld/Core/DefinedAtom.h index b3e05a56b039..5fc7a1fff5bd 100644 --- a/lld/include/lld/Core/DefinedAtom.h +++ b/lld/include/lld/Core/DefinedAtom.h @@ -93,12 +93,13 @@ public: enum Merge { mergeNo, // Another atom with same name is error mergeAsTentative, // Is ANSI C tentative definition, can be coalesced - mergeAsWeak, // is C++ inline definition that was not inlined, + mergeAsWeak, // Is C++ inline definition that was not inlined, // but address was not taken, so atom can be hidden // by linker - mergeAsWeakAndAddressUsed,// is C++ definition inline definition whose - // address was taken. - mergeByContent // merge with other constants with same content + mergeAsWeakAndAddressUsed, // Is C++ definition inline definition whose + // address was taken. + mergeByContent, // Merge with other constants with same content + mergeSameNameAndSize, // Another atom with different size is error }; enum ContentType { diff --git a/lld/lib/Core/SymbolTable.cpp b/lld/lib/Core/SymbolTable.cpp index 1208f52c1b75..a6089116da56 100644 --- a/lld/lib/Core/SymbolTable.cpp +++ b/lld/lib/Core/SymbolTable.cpp @@ -98,27 +98,17 @@ enum MergeResolution { MCR_First, MCR_Second, MCR_Largest, + MCR_SameSize, MCR_Error }; -static MergeResolution mergeCases[4][4] = { - // no tentative weak weakAddressUsed - { - // first is no - MCR_Error, MCR_First, MCR_First, MCR_First - }, - { - // first is tentative - MCR_Second, MCR_Largest, MCR_Second, MCR_Second - }, - { - // first is weak - MCR_Second, MCR_First, MCR_First, MCR_Second - }, - { - // first is weakAddressUsed - MCR_Second, MCR_First, MCR_First, MCR_First - } +static MergeResolution mergeCases[][5] = { + // no tentative weak weakAddress sameNameAndSize + {MCR_Error, MCR_First, MCR_First, MCR_First, MCR_SameSize}, // no + {MCR_Second, MCR_Largest, MCR_Second, MCR_Second, MCR_SameSize}, // tentative + {MCR_Second, MCR_First, MCR_First, MCR_Second, MCR_SameSize}, // weak + {MCR_Second, MCR_First, MCR_First, MCR_First, MCR_SameSize}, // weakAddress + {MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize, MCR_SameSize}, // sameSize }; static MergeResolution mergeSelect(DefinedAtom::Merge first, @@ -126,7 +116,7 @@ static MergeResolution mergeSelect(DefinedAtom::Merge first, return mergeCases[first][second]; } -void SymbolTable::addByName(const Atom & newAtom) { +void SymbolTable::addByName(const Atom &newAtom) { StringRef name = newAtom.name(); assert(!name.empty()); const Atom *existing = this->findByName(name); @@ -149,7 +139,7 @@ void SymbolTable::addByName(const Atom & newAtom) { assert(existing->definition() == Atom::definitionRegular); assert(newAtom.definition() == Atom::definitionRegular); switch (mergeSelect(((DefinedAtom*)existing)->merge(), - ((DefinedAtom*)(&newAtom))->merge())) { + ((DefinedAtom*)&newAtom)->merge())) { case MCR_First: useNew = false; break; @@ -159,6 +149,18 @@ void SymbolTable::addByName(const Atom & newAtom) { case MCR_Largest: useNew = true; break; + case MCR_SameSize: { + uint64_t sa = ((DefinedAtom*)existing)->size(); + uint64_t sb = ((DefinedAtom*)&newAtom)->size(); + if (sa == sb) { + useNew = true; + break; + } + llvm::errs() << "Size mismatch: " + << existing->name() << " (" << sa << ") " + << newAtom.name() << " (" << sb << ")\n"; + // fallthrough + } case MCR_Error: llvm::errs() << "Duplicate symbols: " << existing->name() diff --git a/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp b/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp index 4fb045664b40..54acbc829bff 100644 --- a/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp +++ b/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp @@ -249,8 +249,10 @@ DefinedAtom::Merge getMerge(const coff_aux_section_definition *auxsym) { return DefinedAtom::mergeNo; case llvm::COFF::IMAGE_COMDAT_SELECT_ANY: return DefinedAtom::mergeAsWeakAndAddressUsed; - case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE: case llvm::COFF::IMAGE_COMDAT_SELECT_EXACT_MATCH: + return DefinedAtom::mergeByContent; + case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE: + return DefinedAtom::mergeSameNameAndSize; case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE: case llvm::COFF::IMAGE_COMDAT_SELECT_LARGEST: case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST: diff --git a/lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp b/lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp index 30ede7698a99..03a224477345 100644 --- a/lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp +++ b/lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp @@ -337,7 +337,9 @@ template <> struct ScalarEnumerationTraits { io.enumCase(value, "as-weak", lld::DefinedAtom::mergeAsWeak); io.enumCase(value, "as-addressed-weak", lld::DefinedAtom::mergeAsWeakAndAddressUsed); - io.enumCase(value, "by-content", lld::DefinedAtom::mergeByContent); + io.enumCase(value, "by-content", lld::DefinedAtom::mergeByContent); + io.enumCase(value, "same-name-and-size", + lld::DefinedAtom::mergeSameNameAndSize); } }; diff --git a/lld/test/pecoff/Inputs/merge-same-size1.obj.yaml b/lld/test/pecoff/Inputs/merge-same-size1.obj.yaml new file mode 100644 index 000000000000..6b71a6d10f74 --- /dev/null +++ b/lld/test/pecoff/Inputs/merge-same-size1.obj.yaml @@ -0,0 +1,25 @@ +--- +header: + Machine: IMAGE_FILE_MACHINE_I386 + Characteristics: [ ] +sections: + - Name: .text + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: FFFFFFFFFFFFFF +symbols: + - Name: .text + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + NumberOfAuxSymbols: 1 + AuxiliaryData: 0700000000000000C979F796000003000000 + - Name: "_foo" + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL +... diff --git a/lld/test/pecoff/Inputs/merge-same-size2.obj.yaml b/lld/test/pecoff/Inputs/merge-same-size2.obj.yaml new file mode 100644 index 000000000000..b956af1a7c92 --- /dev/null +++ b/lld/test/pecoff/Inputs/merge-same-size2.obj.yaml @@ -0,0 +1,25 @@ +--- +header: + Machine: IMAGE_FILE_MACHINE_I386 + Characteristics: [ ] +sections: + - Name: .text + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: AAAAAAAAAAAAAA +symbols: + - Name: .text + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + NumberOfAuxSymbols: 1 + AuxiliaryData: 0700000000000000C979F796000003000000 + - Name: "_foo" + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL +... diff --git a/lld/test/pecoff/merge-same-size.test b/lld/test/pecoff/merge-same-size.test new file mode 100644 index 000000000000..26433685bff8 --- /dev/null +++ b/lld/test/pecoff/merge-same-size.test @@ -0,0 +1,24 @@ +# RUN: yaml2obj %p/Inputs/merge-same-size1.obj.yaml > %t1.obj +# RUN: yaml2obj %p/Inputs/merge-same-size2.obj.yaml > %t2.obj +# +# RUN: lld -flavor link /out:%t.exe /subsystem:console /opt:noref /force \ +# RUN: -- %t1.obj %t2.obj 2>&1 > %t.log +# +# FileCheck complains if the input files is empty, so add a dummy line. +# RUN: echo foo >> %t.log +# RUN: FileCheck -check-prefix=STDERR %s < %t.log +# +# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=READOBJ %s + +STDERR-NOT: duplicate symbol error + +READOBJ: Format: COFF-i386 +READOBJ-NEXT: Arch: i386 +READOBJ-NEXT: AddressSize: 32bit +READOBJ-NEXT: Sections [ +READOBJ-NEXT: Section { +READOBJ-NEXT: Number: 1 +READOBJ-NEXT: Name: .text (2E 74 65 78 74 00 00 00) +READOBJ-NEXT: VirtualSize: 0x7 +READOBJ-NEXT: VirtualAddress: 0x1000 +READOBJ-NEXT: RawDataSize: 7