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

Fri May 5 02:58:59 PDT 2023

alvinhochun added a comment.

LGTM except for the issue with the test. I guess wait a few days to see if anyone else have other comments, or someone else can accept too.

================
Comment at: lld/test/COFF/ordinals-override.test:65
+    Alignment:       1
+    SectionData:     2020202F6578706F72743A62617A3D3F62617A4040594158585A202F44454641554C544C49423A224C4942434D5422202F44454641554C544C49423A224F4C444E414D455322202F4558504F52543A3F666F6F4040594158585A202F4558504F52543A3F6261724040594158585A20
+  - Name:            '.debug$S'
----------------
aganea wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > Can you add a comment what the contained directive string is?
> > This is marked "done" but I don't see the comment. Anyway, decoding this shows `/export:baz=?baz@@YAXXZ /DEFAULTLIB:"LIBCMT" /DEFAULTLIB:"OLDNAMES" /EXPORT:?foo@@YAXXZ /EXPORT:?bar@@YAXXZ`, which seems to be why the pre-merge checks failed on Debian, so I guess they should be cleaned up a bit.
> > 
> > ```
> > lld-link: error: could not open 'LIBCMT.lib': No such file or directory
> > lld-link: error: could not open 'OLDNAMES.lib': No such file or directory
> > ```
> Sorry about that, I lost some changes between my rebases. I added them back, please see the new revision. I fixed the linker errors as well.
The pre-merge check is still failing. Can you remove the `/DEFAULTLIB:` flags from .drectve?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149611