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

Rui Ueyama ruiu at google.com
Wed Oct 23 14:45:42 PDT 2013


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> 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> 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 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/20131023/7bdd9968/attachment.html>


More information about the llvm-commits mailing list