This is an archive of the discontinued LLVM Phabricator instance.

Add iOS/watchOS/tvOS support for ASan (clang part)
ClosedPublic

Authored by zaks.anna on Dec 17 2015, 12:38 PM.

Details

Summary

Change the clang driver to accept ASan on iOS/watchOS/tvOS.

This change along with the corresponding changes in llvm and compiler-rt complete ASan support for iOS/watchOS/tvOS.

Diff Detail

Repository
rL LLVM

Event Timeline

zaks.anna updated this revision to Diff 43171.Dec 17 2015, 12:38 PM
zaks.anna retitled this revision from to Add iOS/watchOS/tvOS support for ASan (clang part).
zaks.anna updated this object.
zaks.anna added reviewers: kcc, cfe-commits.
kcc accepted this revision.Dec 17 2015, 12:40 PM
kcc edited edge metadata.

LGTM

This revision is now accepted and ready to land.Dec 17 2015, 12:40 PM
samsonov added inline comments.
lib/Driver/ToolChains.cpp
368 ↗(On Diff #43171)

Wait, this looks horrible. Can we teach toolchain to give us OS name?

zaks.anna added inline comments.Dec 17 2015, 3:57 PM
lib/Driver/ToolChains.cpp
368 ↗(On Diff #43171)

These are not OS names; they are prefixes we use to name the library. I believe "appletvsimulator" would be a platform name, which is too long to append to the dylib name.

I could factor this out into a separate function, but there are no other immediate users. Not sure if the profiler library could be refactored to use it, logic there seems more complicated.

Sorry, lost track of this.

lib/Driver/ToolChains.cpp
368 ↗(On Diff #43171)

Then we can name it as getOSLibraryNamePrefix() or smth. like that. I worry that this logic would hardly be discoverable inside AddLinkSanitizerLibArgs if it's going to be reused somewhere (which is likely).

zaks.anna updated this revision to Diff 45437.Jan 20 2016, 1:12 PM
zaks.anna edited edge metadata.

Refactor the code to address Samsonov's review.

zaks.anna marked an inline comment as done.Jan 20 2016, 1:14 PM

I've introduced the helper function. Looks like addProfileRTLibs might be able to use it in the future (after support for autoconf is dropped) as well.

Thanks.

lib/Driver/ToolChains.cpp
322 ↗(On Diff #45437)

Fun fact: if TargetPlatform is TvOSSimulator, this function will return iossim because isTargetIOSSimulator will return true.
I suggest to just switch on the value of TargetPlatform instead of using these helpers.

330 ↗(On Diff #45437)

Also, is it possible to add a test case for this?

zaks.anna updated this revision to Diff 45720.Jan 22 2016, 11:37 AM

Thanks for spotting the bug! The bug is fixed and the tests are added in this revision.

samsonov accepted this revision.Jan 22 2016, 1:15 PM
samsonov added a reviewer: samsonov.

LGTM

lib/Driver/ToolChains.cpp
339 ↗(On Diff #45720)

You might need llvm_unreachable at the very end to suppress GCC warnings (http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations)

This revision was automatically updated to reflect the committed changes.