[PATCH] D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports

[Alvin Wong via Phabricator via llvm-commits]

alvinhochun requested changes to this revision.
alvinhochun added inline comments.
This revision now requires changes to proceed.


================
Comment at: lld/COFF/DLL.cpp:558
 public:
-  ExportDirectoryChunk(int i, int j, Chunk *d, Chunk *a, Chunk *n, Chunk *o)
-      : maxOrdinal(i), nameTabSize(j), dllName(d), addressTab(a), nameTab(n),
-        ordinalTab(o) {}
+  ExportDirectoryChunk(int i, int j, int k, Chunk *d, Chunk *a, Chunk *n,
+                       Chunk *o)
----------------
Can we at least have a more descriptive name for the new argument for `baseOrdinal`?


================
Comment at: lld/COFF/DLL.cpp:601
     for (const Export &e : ctx.config.exports) {
       assert(e.ordinal != 0 && "Export symbol has invalid ordinal");
+      // Subtract the OrdinalBase to get the index.
----------------



================
Comment at: lld/COFF/DLL.cpp:651
         continue;
       assert(e.ordinal != 0 && "Export symbol has invalid ordinal");
+      // This table stores unbiased indices, so subtract OrdinalBase.
----------------



================
Comment at: lld/COFF/DLL.cpp:846
+  }
+  assert(baseOrdinal >= 1);
 
----------------
Can you check whether MSVC link.exe allows using export ordinal 0?


================
Comment at: lld/test/COFF/export.test:17
 CHECK2:      DLL name: export.test.tmp.dll
+CHECK2:      Ordinal base: 5
 CHECK2:      Ordinal      RVA  Name
----------------
This seems to depend on changes to llvm-objdump? There should be a separate parent patch to add this output.

Also this patch despite being marked as depending on D149610 somehow does not include the changes in it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149611