[lld] r188025 - [PECOFF] Support COMDAT section that contains mergeable atoms.
Reid Kleckner
rnk at google.com
Thu Aug 8 16:46:48 PDT 2013
On Thu, Aug 8, 2013 at 4:31 PM, Rui Ueyama <ruiu at google.com> wrote:
> Author: ruiu
> Date: Thu Aug 8 18:31:50 2013
> New Revision: 188025
>
> URL: http://llvm.org/viewvc/llvm-project?rev=188025&view=rev
> Log:
> [PECOFF] Support COMDAT section that contains mergeable atoms.
>
> The COMDAT section is a section with a special attribute to tell the linker
> whether the symbols in the section are allowed to be merged or not. This
> patch
> add a function to interpret the COMDAT data and set "merge" attribute to
> the
> atoms accordingly.
>
> LLD supports multiple policies to merge atoms; atoms can be merged by name
> or
> by content. COFF supports them, and in addition to that, it supports
> choose-the-largest-atom policy, which LLD currently does not support. I
> simply
> mapped it to merge-by-name attribute for now, but we eventually have to
> support
> that policy in the core linker.
>
> Added:
> lld/trunk/test/pecoff/Inputs/comdat.obj.yaml
> lld/trunk/test/pecoff/comdat.test
> Modified:
> lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h
> lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
>
> Modified: lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h?rev=188025&r1=188024&r2=188025&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h (original)
> +++ lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h Thu Aug 8 18:31:50 2013
> @@ -175,15 +175,17 @@ class COFFDefinedAtom : public COFFDefin
> public:
> COFFDefinedAtom(const File &file, StringRef name, StringRef sectionName,
> Scope scope, ContentType type, ContentPermissions perms,
> - ArrayRef<uint8_t> data, uint64_t ordinal)
> + Merge merge, ArrayRef<uint8_t> data, uint64_t ordinal)
> : COFFDefinedFileAtom(file, name, sectionName, scope, type, perms,
> ordinal),
> - _dataref(data) {}
> + _merge(merge), _dataref(data) {}
>
> + virtual Merge merge() const { return _merge; }
> virtual uint64_t size() const { return _dataref.size(); }
> virtual ArrayRef<uint8_t> rawContent() const { return _dataref; }
>
> private:
> + Merge _merge;
> ArrayRef<uint8_t> _dataref;
> };
>
>
> Modified: lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp?rev=188025&r1=188024&r2=188025&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp (original)
> +++ lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp Thu Aug 8 18:31:50
> 2013
> @@ -38,6 +38,7 @@ using lld::coff::COFFDefinedAtom;
> using lld::coff::COFFDefinedFileAtom;
> using lld::coff::COFFReference;
> using lld::coff::COFFUndefinedAtom;
> +using llvm::object::coff_aux_section_definition;
> using llvm::object::coff_relocation;
> using llvm::object::coff_section;
> using llvm::object::coff_symbol;
> @@ -46,6 +47,7 @@ using namespace lld;
>
> namespace {
>
> +// Converts the COFF symbol attribute to the LLD's atom attribute.
> Atom::Scope getScope(const coff_symbol *symbol) {
> switch (symbol->StorageClass) {
> case llvm::COFF::IMAGE_SYM_CLASS_EXTERNAL:
> @@ -79,6 +81,16 @@ DefinedAtom::ContentPermissions getPermi
> return DefinedAtom::perm___;
> }
>
> +DefinedAtom::Merge getMerge(const coff_aux_section_definition *auxsym) {
> + switch (auxsym->Selection) {
> + case llvm::COFF::IMAGE_COMDAT_SELECT_NODUPLICATES:
> + return DefinedAtom::mergeNo;
> + default:
> + // FIXME: COFF supports more mergable symbol attributes.
>
Maybe it would be better to list the fully cover the switch and return
mergeNo for any case that lld doesn't support yet?
+ return DefinedAtom::mergeAsWeakAndAddressUsed;
> + }
> +}
> +
> class FileCOFF : public File {
> private:
> typedef vector<const coff_symbol *> SymbolVectorT;
> @@ -201,6 +213,13 @@ private:
> /// we need to know the adjacent symbols.
> error_code createDefinedSymbols(const SymbolVectorT &symbols,
> vector<const DefinedAtom *> &result) {
> + // A defined atom can be merged if its section attribute allows its
> contents
> + // to be merged. In COFF, it's not very easy to get the section
> attribute
> + // for the symbol, so scan all sections in advance and cache the
> attributes
> + // for later use.
> + if (error_code ec = cacheSectionAttributes())
> + return ec;
> +
> // Filter non-defined atoms, and group defined atoms by its section.
> SectionToSymbolsT definedSymbols;
> for (const coff_symbol *sym : symbols) {
> @@ -223,6 +242,22 @@ private:
> sym->SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)
> continue;
>
> + const coff_section *sec;
> + if (error_code ec = _obj->getSection(sym->SectionNumber, sec))
> + return ec;
> + assert(sec && "SectionIndex > 0, Sec must be non-null!");
> +
> + // SKip if it's a section symbol for a COMDAT section. A section
> symbol
>
Typo on SKip.
> + // has the name of the section and value 0. A translation unit may
> contain
> + // multiple COMDAT sections whose section name are the same. We
> don't want
> + // to make atoms for them as they would become duplicate symbols.
> + StringRef sectionName;
> + if (error_code ec = _obj->getSectionName(sec, sectionName))
> + return ec;
> + if (_symbolName[sym] == sectionName && sym->Value == 0 &&
> + _merge[sec] != DefinedAtom::mergeNo)
> + continue;
> +
> uint8_t sc = sym->StorageClass;
> if (sc != llvm::COFF::IMAGE_SYM_CLASS_EXTERNAL &&
> sc != llvm::COFF::IMAGE_SYM_CLASS_STATIC &&
> @@ -233,10 +268,6 @@ private:
> return llvm::object::object_error::parse_failed;
> }
>
> - const coff_section *sec;
> - if (error_code ec = _obj->getSection(sym->SectionNumber, sec))
> - return ec;
> - assert(sec && "SectionIndex > 0, Sec must be non-null!");
> definedSymbols[sec].push_back(sym);
> }
>
> @@ -247,6 +278,55 @@ private:
> return error_code::success();
> }
>
> + // Cache the COMDAT attributes, which indicate whether the symbols in
> the
> + // section can be merged or not.
> + error_code cacheSectionAttributes() {
> + const llvm::object::coff_file_header *header = nullptr;
> + if (error_code ec = _obj->getHeader(header))
> + return ec;
> +
> + // The COMDAT section attribute is not an attribute of coff_section,
> but is
> + // stored in the auxiliary symbol for the first symbol referring a
> COMDAT
> + // section. It feels to me that it's unnecessarily complicated, but
> this is
> + // how COFF works.
> + for (uint32_t i = 0, e = header->NumberOfSymbols; i != e; ++i) {
> + const coff_symbol *sym;
> + if (error_code ec = _obj->getSymbol(i, sym))
> + return ec;
> + if (sym->SectionNumber == llvm::COFF::IMAGE_SYM_ABSOLUTE ||
> + sym->SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)
> + continue;
> +
> + const coff_section *sec;
> + if (error_code ec = _obj->getSection(sym->SectionNumber, sec))
> + return ec;
> +
> + if (_merge.count(sec))
> + continue;
> + if (!(sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_COMDAT))
> + continue;
> +
> + if (sym->NumberOfAuxSymbols == 0)
> + return llvm::object::object_error::parse_failed;
> + const coff_aux_section_definition *aux = nullptr;
> + if (error_code ec = _obj->getAuxSymbol(i + 1, aux))
> + return ec;
> +
> + _merge[sec] = getMerge(aux);
> + }
> +
> + // The sections that does not have auxiliary symbol are regular
> sections, in
> + // which symbols are not allowed to be merged.
> + error_code ec;
> + for (auto si = _obj->begin_sections(), se = _obj->end_sections(); si
> != se;
> + si.increment(ec)) {
> + const coff_section *sec = _obj->getCOFFSection(si);
> + if (!_merge.count(sec))
> + _merge[sec] = DefinedAtom::mergeNo;
> + }
> + return error_code::success();
> + }
> +
> /// Atomize \p symbols and append the results to \p atoms. The symbols
> are
> /// assumed to have been defined in the \p section.
> error_code
> @@ -310,7 +390,7 @@ private:
> ArrayRef<uint8_t> data(secData.data(), secData.size());
> auto *atom = new (_alloc) COFFDefinedAtom(
> *this, "", sectionName, Atom::scopeTranslationUnit, type, perms,
> - data, 0);
> + _merge[section], data, 0);
> atoms.push_back(atom);
> _definedAtomLocations[section][0] = atom;
> return error_code::success();
> @@ -323,7 +403,7 @@ private:
> ArrayRef<uint8_t> data(secData.data(), size);
> auto *atom = new (_alloc) COFFDefinedAtom(
> *this, "", sectionName, Atom::scopeTranslationUnit, type, perms,
> - data, ++ordinal);
> + _merge[section], data, ++ordinal);
> atoms.push_back(atom);
> _definedAtomLocations[section][0] = atom;
> }
> @@ -337,7 +417,7 @@ private:
> ArrayRef<uint8_t> data(start, end);
> auto *atom = new (_alloc) COFFDefinedAtom(
> *this, _symbolName[*si], sectionName, getScope(*si), type,
> perms,
> - data, ++ordinal);
> + _merge[section], data, ++ordinal);
> atoms.push_back(atom);
> _symbolAtom[*si] = atom;
> _definedAtomLocations[section][(*si)->Value] = atom;
> @@ -519,6 +599,9 @@ private:
> // A map from section to its atoms.
> std::map<const coff_section *, vector<COFFDefinedFileAtom *>>
> _sectionAtoms;
>
> + // A map to get whether the section allows its contents to be merged or
> not.
> + std::map<const coff_section *, DefinedAtom::Merge> _merge;
> +
> // A sorted map to find an atom from a section and an offset within
> // the section.
> std::map<const coff_section *,
>
> Added: lld/trunk/test/pecoff/Inputs/comdat.obj.yaml
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/Inputs/comdat.obj.yaml?rev=188025&view=auto
>
> ==============================================================================
> --- lld/trunk/test/pecoff/Inputs/comdat.obj.yaml (added)
> +++ lld/trunk/test/pecoff/Inputs/comdat.obj.yaml Thu Aug 8 18:31:50 2013
> @@ -0,0 +1,43 @@
> +---
> +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: 558BEC33C05DC3
> + - Name: .text
> + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT,
> IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
> + Alignment: 16
> + SectionData: 558BEC33C05DC3
> +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: 0700000000000000C979F796000002000000
> + - Name: .text
> + Value: 0
> + SectionNumber: 2
> + SimpleType: IMAGE_SYM_TYPE_NULL
> + ComplexType: IMAGE_SYM_DTYPE_NULL
> + StorageClass: IMAGE_SYM_CLASS_STATIC
> + NumberOfAuxSymbols: 1
> + AuxiliaryData: 0700000000000000C979F796000002000000
> + - Name: "?inlinefn1@@YAHXZ"
> + Value: 0
> + SectionNumber: 1
> + SimpleType: IMAGE_SYM_TYPE_NULL
> + ComplexType: IMAGE_SYM_DTYPE_FUNCTION
> + StorageClass: IMAGE_SYM_CLASS_EXTERNAL
> + - Name: "?inlinefn2@@YAHXZ"
> + Value: 0
> + SectionNumber: 2
> + SimpleType: IMAGE_SYM_TYPE_NULL
> + ComplexType: IMAGE_SYM_DTYPE_FUNCTION
> + StorageClass: IMAGE_SYM_CLASS_EXTERNAL
> +...
>
> Added: lld/trunk/test/pecoff/comdat.test
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/comdat.test?rev=188025&view=auto
>
> ==============================================================================
> --- lld/trunk/test/pecoff/comdat.test (added)
> +++ lld/trunk/test/pecoff/comdat.test Thu Aug 8 18:31:50 2013
> @@ -0,0 +1,12 @@
> +# RUN: yaml2obj %p/Inputs/comdat.obj.yaml > %t1.obj
> +# RUN: yaml2obj %p/Inputs/comdat.obj.yaml > %t2.obj
> +#
> +# RUN: lld -flavor link /out:%t3.exe /subsystem:console /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 %s < %t.log
> +
> +CHECK-NOT: duplicate symbol error
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130808/7caed37a/attachment.html>
More information about the llvm-commits
mailing list