This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia
ClosedPublic

Authored by phosek on Oct 29 2018, 6:14 PM.

Details

Summary

This avoids introducing unnecessary DT_NEEDED entries when using
C++ driver for linking C code or C++ code that doesn't use C++
standard library.

Diff Detail

Repository
rC Clang

Event Timeline

phosek created this revision.Oct 29 2018, 6:14 PM
mcgrathr accepted this revision.Oct 30 2018, 4:37 PM
mcgrathr added inline comments.
clang/lib/Driver/ToolChains/Fuchsia.cpp
125

I'd use the -- version of all these GNU-compatible options since that's the GNU-compatible syntax.
(But -static is still single-dash.)

132–133

-lm belongs inside the --as-needed umbrella too.

This revision is now accepted and ready to land.Oct 30 2018, 4:37 PM
phosek updated this revision to Diff 171948.Oct 31 2018, 9:50 AM
phosek marked 2 inline comments as done.
This revision was automatically updated to reflect the committed changes.
MaskRay added a subscriber: MaskRay.Nov 2 2018, 9:32 PM
MaskRay added inline comments.
lib/Driver/ToolChains/Fuchsia.cpp
128 ↗(On Diff #172478)

If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in that -static switches the whole link to its special static mode. (as usually while you link libstdc++/libc++ statically, you can still link other libraries normally)

In ld.bfd/lld, -Bstatic is synonym with -static.

mcgrathr added inline comments.Nov 3 2018, 5:04 PM
lib/Driver/ToolChains/Fuchsia.cpp
128 ↗(On Diff #172478)

I think this part of the change was unintentional and should be undone.
Using --pop-state obviates the need for -Bdynamic to undo -Bstatic, but -Bstatic is still the right switch here.

mcgrathr reopened this revision.Nov 3 2018, 6:25 PM
mcgrathr added inline comments.
lib/Driver/ToolChains/Fuchsia.cpp
128 ↗(On Diff #172478)

Actually, it's wrong two ways: the --pop-state should come before -lm. Neither -static nor -Bstatic should apply to -lm (or to -lc that comes later, which -static might). -static vs -Bstatic is only a latent bug given lld, but the -lm issue breaks the Zircon build today.

This revision is now accepted and ready to land.Nov 3 2018, 6:25 PM
phosek closed this revision.Nov 3 2018, 9:26 PM
phosek added inline comments.
lib/Driver/ToolChains/Fuchsia.cpp
128 ↗(On Diff #172478)

Addressed in D54082.