[PATCH] D101569: [LLD] [COFF] Fix automatic export of symbols from LTO objects

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Nice, I think this is a reasonable direction.



================
Comment at: lld/COFF/InputFiles.cpp:1045
 
+class FakeSection : public coff_section {
+public:
----------------
Please put this in an anonymous namespace and add a comment that this is just adding a constructor to help stamp out COFF section headers. For some reason I thought it was adding fields. Actually, maybe it would be less surprising if the coff_section was a plain field, and then we can take its address to construct the SectionChunk objects.


================
Comment at: lld/COFF/InputFiles.cpp:1047
+public:
+  FakeSection(int C) {
+    Characteristics = C;
----------------
Can this be marked constexpr? We want to avoid any dynamic initialization if possible.


================
Comment at: lld/COFF/InputFiles.cpp:1052
+
+FakeSection ltoTextSection(IMAGE_SCN_MEM_EXECUTE);
+FakeSection ltoDataSection(IMAGE_SCN_CNT_INITIALIZED_DATA);
----------------
Please mark the globals static.


================
Comment at: lld/COFF/InputFiles.cpp:1054
+FakeSection ltoDataSection(IMAGE_SCN_CNT_INITIALIZED_DATA);
+SectionChunk ltoTextSectionChunk(nullptr, &ltoTextSection);
+SectionChunk ltoDataSectionChunk(nullptr, &ltoDataSection);
----------------
The SectionChunk constructor is non-trivial, so this will definitely create dynamic initialization. I guess that's OK, LLD isn't really structured as a library anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101569/new/

https://reviews.llvm.org/D101569