[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