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

Fri May 5 03:31:43 PDT 2023

mstorsjo added inline comments.

================
Comment at: lld/test/COFF/ordinals-override.test:65
+    Alignment:       1
+    SectionData:     2020202F6578706F72743A62617A3D3F62617A4040594158585A202F44454641554C544C49423A224C4942434D5422202F44454641554C544C49423A224F4C444E414D455322202F4558504F52543A3F666F6F4040594158585A202F4558504F52543A3F6261724040594158585A20
+  - Name:            '.debug$S'
----------------
alvinhochun wrote:
> 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?
Relatedly, it would probably be good if the lld tests would run with some flag that tells it to not look at the surrounding environment, to avoid accidentally making tests that refer to libraries available outside of the tests? Or perhaps it's not too much of an issue as it'd be hit immediately by all buildbots anyway, and the premerge CI in this case too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149611