[lld] r193017 - [PECOFF] Only COMDAT symbols are allowed to be dead-stripped.

Rui Ueyama ruiu at google.com
Wed Oct 23 15:13:04 PDT 2013


On Wed, Oct 23, 2013 at 2:49 PM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

>  It would remove only if .init_array is not added to the root set. By
> default the reader should make sure certain sections like
> .ctors/.dtors/.init_array ... gets added to the rootset.
>

Where is the code? I looked for addDeadStripRoot and deadStripNever but
couldn't find it.


>  Thanks
>
> Shankar Easwaran
>
>
> On 10/23/2013 4:45 PM, Rui Ueyama wrote:
>
> I think GNU ld keeps .init_array sections using KEEP linker script macro
> command. Because LLD currently does not the command, I believe it would
> remove .init_array sections, which is wrong.
> https://sourceware.org/binutils/docs/ld/Input-Section-Keep.html#Input-Section-Keep
>
> In COFF, looks like there's no generic mechanism to tell the linker which
> sections are safe to be GC'ed, and the linker is allowed to remove only
> COMDAT symbols. That's different from ELF.
>
>
> On Wed, Oct 23, 2013 at 2:14 PM, Reid Kleckner <rnk at google.com> <rnk at google.com> wrote:
>
>
>  ELF uses exactly the same technique for .init_array, and you can use
> .init_array.priority to get adjust the order of your initializers.  Are you
> handling it the same way that ELF does, or is this something else?
>
> I think link.exe will only dead strip if you use /Gy, which has the effect
> of putting all functions in COMDATs.  It looks like it will not dead strip
> data like these function pointers.
>
>
> On Fri, Oct 18, 2013 at 4:54 PM, Rui Ueyama <ruiu at google.com> <ruiu at google.com> wrote:
>
>
>  Author: ruiu
> Date: Fri Oct 18 18:54:55 2013
> New Revision: 193017
>
> URL: http://llvm.org/viewvc/llvm-project?rev=193017&view=rev
> Log:
> [PECOFF] Only COMDAT symbols are allowed to be dead-stripped.
>
> We should dead-strip atoms only if they are created for COMDAT symbols.
> If we
> remove non-COMDAT atoms from a binary, it will no longer be guaranteed
> that
> the binary will work correctly.
>
> In COFF, you can manipulate the order of section contents in the resulting
> binary by section name. For example, if you have four sections
> .data$unique_prefix_{a,b,c,d}, it's guaranteed that the contents of A, B,
> C,
> and D will be consecutive in the resulting .data section in that order.
> Thus, you can access B's and C's contents by incrementing a pointer
> pointing
> to A until it reached to D. That's why we cannot dead-strip B or C even if
> no one is directly referencing to them.
>
> Some object files in the standard library actually use that technique.
>
> Modified:
>     lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h
>     lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
>     lld/trunk/test/pecoff/grouped-sections.test
>
> Modified: lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h
> URL:http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h?rev=193017&r1=193016&r2=193017&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h (original)
> +++ lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h Fri Oct 18 18:54:55 2013
> @@ -181,17 +181,24 @@ private:
>  class COFFDefinedAtom : public COFFDefinedFileAtom {
>  public:
>    COFFDefinedAtom(const File &file, StringRef name, StringRef
> sectionName,
> -                  Scope scope, ContentType type, ContentPermissions
> perms,
> -                  Merge merge, ArrayRef<uint8_t> data, uint64_t ordinal)
> +                  Scope scope, ContentType type, bool isComdat,
> +                  ContentPermissions perms, Merge merge,
> ArrayRef<uint8_t> data,
> +                  uint64_t ordinal)
>        : COFFDefinedFileAtom(file, name, sectionName, scope, type, perms,
>                              ordinal),
> -        _merge(merge), _dataref(data) {}
> +        _isComdat(isComdat), _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; }
>
> +  virtual DeadStripKind deadStrip() const {
> +    // Only COMDAT symbols would be dead-stripped.
> +    return _isComdat ? deadStripNormal : deadStripNever;
> +  }
> +
>  private:
> +  bool _isComdat;
>    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=193017&r1=193016&r2=193017&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp (original)
> +++ lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp Fri Oct 18 18:54:55
> 2013
> @@ -397,6 +397,8 @@ private:
>        if (!(sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_COMDAT))
>          continue;
>
> +      _comdatSections.insert(sec);
> +
>        if (sym->NumberOfAuxSymbols == 0)
>          return llvm::object::object_error::parse_failed;
>        const coff_aux_section_definition *aux =
> @@ -473,13 +475,14 @@ private:
>
>      DefinedAtom::ContentType type = getContentType(section);
>      DefinedAtom::ContentPermissions perms = getPermissions(section);
> +    bool isComdat = (_comdatSections.count(section) == 1);
>
>      // Create an atom for the entire section.
>      if (symbols.empty()) {
>        ArrayRef<uint8_t> data(secData.data(), secData.size());
>        auto *atom = new (_alloc)
>            COFFDefinedAtom(*this, "", sectionName,
> Atom::scopeTranslationUnit,
> -                          type, perms, _merge[section], data, 0);
> +                          type, isComdat, perms, _merge[section], data,
> 0);
>        atoms.push_back(atom);
>        _definedAtomLocations[section][0].push_back(atom);
>        return error_code::success();
> @@ -490,9 +493,9 @@ private:
>      if (symbols[0]->Value != 0) {
>        uint64_t size = symbols[0]->Value;
>        ArrayRef<uint8_t> data(secData.data(), size);
> -      auto *atom = new (_alloc)
> -          COFFDefinedAtom(*this, "", sectionName,
> Atom::scopeTranslationUnit,
> -                          type, perms, _merge[section], data, ++ordinal);
> +      auto *atom = new (_alloc) COFFDefinedAtom(
> +          *this, "", sectionName, Atom::scopeTranslationUnit, type,
> isComdat,
> +          perms, _merge[section], data, ++ordinal);
>        atoms.push_back(atom);
>        _definedAtomLocations[section][0].push_back(atom);
>      }
> @@ -503,9 +506,9 @@ private:
>        const uint8_t *end = (si + 1 == se) ? secData.data() +
> secData.size()
>                                            : secData.data() + (*(si +
> 1))->Value;
>        ArrayRef<uint8_t> data(start, end);
> -      auto *atom = new (_alloc)
> -          COFFDefinedAtom(*this, _symbolName[*si], sectionName,
> getScope(*si),
> -                          type, perms, _merge[section], data, ++ordinal);
> +      auto *atom = new (_alloc) COFFDefinedAtom(
> +          *this, _symbolName[*si], sectionName, getScope(*si), type,
> isComdat,
> +          perms, _merge[section], data, ++ordinal);
>        atoms.push_back(atom);
>        _symbolAtom[*si] = atom;
>        _definedAtomLocations[section][(*si)->Value].push_back(atom);
> @@ -696,6 +699,9 @@ private:
>    // A map from section to its atoms.
>    std::map<const coff_section *, vector<COFFDefinedFileAtom *> >
> _sectionAtoms;
>
> +  // A set of COMDAT sections.
> +  std::set<const coff_section *> _comdatSections;
> +
>    // A map to get whether the section allows its contents to be merged
> or not.
>    std::map<const coff_section *, DefinedAtom::Merge> _merge;
>
>
> Modified: lld/trunk/test/pecoff/grouped-sections.test
> URL:http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/grouped-sections.test?rev=193017&r1=193016&r2=193017&view=diff
>
> ==============================================================================
> --- lld/trunk/test/pecoff/grouped-sections.test (original)
> +++ lld/trunk/test/pecoff/grouped-sections.test Fri Oct 18 18:54:55 2013
> @@ -1,6 +1,6 @@
>  # RUN: yaml2obj %p/Inputs/grouped-sections.obj.yaml > %t.obj
>  #
> -# RUN: lld -flavor link /out:%t1 /subsystem:console /force /opt:noref \
> +# RUN: lld -flavor link /out:%t1 /subsystem:console /force \
>  # RUN:   -- %t.obj && llvm-objdump -s %t1 | FileCheck %s
>  #
>  # The file "grouped-sections.obj" has three data sections in the
> following
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131023/da1ddec1/attachment.html>


More information about the llvm-commits mailing list