[PATCH] D52749: [LLD][COFF] Fix CRT global initializers

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 1 14:31:03 PDT 2018


ruiu added a comment.

Wow, I was surprised that we still have this kind of silly bugs... Thank you for find this out.



================
Comment at: COFF/Writer.cpp:748
+      //
+      // It is therefore critical to sort the chunks containing the function
+      // pointers in the order that they are listed in the object file,
----------------
Maybe you should mention that C++ guarantees that global variables in a single compilation unit are initialized from top to bottom.


================
Comment at: COFF/Writer.cpp:1594
+void Writer::sortSectionChunks(std::vector<Chunk *> &Chunks) {
+  if (Chunks.size() > 1) {
+    auto SectionChunkOrder = [](const Chunk *A, const Chunk *B) {
----------------
I don't think you need this guard -- you can still sort an empty or a singleton list.


================
Comment at: COFF/Writer.cpp:1614
+    };
+    sort(parallel::par, Chunks.begin(), Chunks.end(), SectionChunkOrder);
+  }
----------------
I don't think you need a parallel sort, because the number of sections in a .CRT shouldn't be that large.


================
Comment at: test/COFF/crt-dyn-initializer-order.test:1
+
+# // a.cpp
----------------
Remove an unnecessary blank line.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D52749





More information about the llvm-commits mailing list