Page MenuHomePhabricator

[libcxx] [test] Include more libraries that normally are linked automatically
ClosedPublic

Authored by mstorsjo on Apr 28 2021, 2:25 AM.

Details

Summary

As the libcxx tests link with -nostdlib, libraries that normally
are added by default by the compiler driver has to be added
manually.

The "oldnames" library is automatically added when driving linking
with clang-cl. When linking with the plain clang driver, as the
libcxx tests do, the clang driver does the same but only since Clang
12.0). But when linking with -nostdlib, like the libcxx tests do,
the driver defaults aren't added at all, and we need to specify the
defaults manually.

This allows removing a TODO from the Windows CI setup; it turns out
that upgrading to Clang 12.0 didn't help here as expected, sorry about
that mixup.

Diff Detail

Event Timeline

mstorsjo created this revision.Apr 28 2021, 2:25 AM
mstorsjo requested review of this revision.Apr 28 2021, 2:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 2:25 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius accepted this revision as: curdeius.Apr 28 2021, 3:04 AM
curdeius added a subscriber: curdeius.

LGTM. Is there any other option on clang-cl to avoid linking MSVCRT but linking other stuff? Is there going to be any change in clang 13?

LGTM. Is there any other option on clang-cl to avoid linking MSVCRT but linking other stuff? Is there going to be any change in clang 13?

I don't know of any change wrt this in clang 13, and oldnames.lib is actually an extension to the regular msvcrt library (it mainly consists of aliases from functions like getcwd to the form _getcwd which exists in the import libraries, so we don't need to reroute calls to the underscored form in the test code), so I don't think it's practically doable to omit one of them but not the other.

This SGTM, but I see the Runtimes build fails. Can you verify this has nothing to do with this patch?

This SGTM, but I see the Runtimes build fails. Can you verify this has nothing to do with this patch?

It's pretty certainly unrelated, as this patch only touches windows specific codepaths, and there were similar spurious breakage in D101437 (both ran on top of the same llvm-project commit). I can of course put it another round through CI before pushing...

mstorsjo updated this revision to Diff 341420.Apr 29 2021, 12:17 AM

Rerun CI to hopefully get rid of spurious failure.

Mordante accepted this revision.Apr 29 2021, 9:30 AM

Thanks for the return! LGTM!

This revision is now accepted and ready to land.Apr 29 2021, 9:30 AM