<div dir="ltr">On Wed, Oct 23, 2013 at 2:49 PM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <div>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.<br></div></div></blockquote><div><br></div><div>Where is the code? I looked for addDeadStripRoot and deadStripNever but couldn't find it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div bgcolor="#FFFFFF" text="#000000"><div>
      Thanks<br>
      <br>
      Shankar Easwaran<div><div class="h5"><br>
      <br>
      On 10/23/2013 4:45 PM, Rui Ueyama wrote:<br>
    </div></div></div><div><div class="h5">
    <blockquote type="cite">
      <pre>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.

<a href="https://sourceware.org/binutils/docs/ld/Input-Section-Keep.html#Input-Section-Keep" target="_blank">https://sourceware.org/binutils/docs/ld/Input-Section-Keep.html#Input-Section-Keep</a>

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 <a href="mailto:rnk@google.com" target="_blank"><rnk@google.com></a> wrote:

</pre>
      <blockquote type="cite">
        <pre>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 <a href="mailto:ruiu@google.com" target="_blank"><ruiu@google.com></a> wrote:

</pre>
        <blockquote type="cite">
          <pre>Author: ruiu
Date: Fri Oct 18 18:54:55 2013
New Revision: 193017

URL: <a href="http://llvm.org/viewvc/llvm-project?rev=193017&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=193017&view=rev</a>
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:
<a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h?rev=193017&r1=193016&r2=193017&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/Atoms.h?rev=193017&r1=193016&r2=193017&view=diff</a>

==============================================================================
--- 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:
<a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp?rev=193017&r1=193016&r2=193017&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp?rev=193017&r1=193016&r2=193017&view=diff</a>

==============================================================================
--- 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:
<a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/grouped-sections.test?rev=193017&r1=193016&r2=193017&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/grouped-sections.test?rev=193017&r1=193016&r2=193017&view=diff</a>

==============================================================================
--- 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
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>

</pre>
        </blockquote>
        <pre></pre>
      </blockquote>
      <pre></pre>
      <br>
      <fieldset></fieldset>
      <br>
      <pre>_______________________________________________
llvm-commits mailing list
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
    </blockquote>
    <br>
    <br>
    </div></div><span class="HOEnZb"><font color="#888888"><pre cols="72">-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation</pre>
  </font></span></div>

</blockquote></div><br></div></div>