Page MenuHomePhabricator

[Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions
ClosedPublic

Authored by phosek on Apr 23 2019, 2:56 PM.

Details

Summary

This introduces a support for multilibs to Fuchsia driver. Unlike the
existing multilibs that are used primarily for handling different
architecture variants, we use multilibs to handle different variants
of Clang runtime libraries: -fsanitize=address and -fno-exceptions
are the two we support initially. This replaces the existing support
for sanitized runtimes libraries that was only used by Fuchsia driver
and it also refactors some of the logic to allow sharing between GNU
and Fuchsia drivers.

Diff Detail

Repository
rC Clang

Event Timeline

phosek created this revision.Apr 23 2019, 2:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2019, 2:56 PM
jroelofs added inline comments.Apr 23 2019, 5:41 PM
clang/lib/Driver/ToolChains/CommonArgs.h
122 ↗(On Diff #196335)

Can we enforce this precondition with an assert? The '-'-prefix-not-there part is easy, but what about the "it's a driver flag" part?

clang/test/Driver/fuchsia.cpp
53 ↗(On Diff #196335)

What about a test for -fsanitize=address + -fno-exceptions?

phosek added inline comments.Apr 23 2019, 10:33 PM
clang/lib/Driver/ToolChains/CommonArgs.h
122 ↗(On Diff #196335)

We could, but we would need to pass in the reference to Driver so we can use getOpts().ParseOneArg to parse the flag. However, we're not doing that in Multilib either at the moment even though the comment says it has to be a valid flag. I'd prefer to keep this as a pure move and then add the additional checking both here and to Multilib as a separate change, would that be fine with you?

ormris removed a subscriber: ormris.Apr 24 2019, 9:15 AM
ormris added a subscriber: ormris.
ormris removed a subscriber: ormris.
mcgrathr accepted this revision.Apr 24 2019, 4:40 PM

lgtm

clang/lib/Driver/ToolChains/CommonArgs.cpp
1507 ↗(On Diff #196335)

I'd have reduced the duplication and just used Enabled ? "+" : "-"

clang/lib/Driver/ToolChains/Fuchsia.cpp
197 ↗(On Diff #196335)

These two lines repeating the path construction logic could be CSEd into a lambda.

201 ↗(On Diff #196335)

This merits a comment about the order they're being inserted.

clang/test/Driver/fuchsia.cpp
60 ↗(On Diff #196335)

Might be worth adding a test that noexcept does *not* appear by default (or explicit -fexceptions).

This revision is now accepted and ready to land.Apr 24 2019, 4:40 PM
mcgrathr added inline comments.Apr 24 2019, 4:43 PM
clang/lib/Driver/ToolChains/Fuchsia.cpp
179 ↗(On Diff #196335)

Add a comment about the priorities.

phosek updated this revision to Diff 196791.Apr 25 2019, 8:43 PM
phosek marked 2 inline comments as done.
phosek marked 2 inline comments as done.
This revision was automatically updated to reflect the committed changes.