This is an archive of the discontinued LLVM Phabricator instance.

Enable builds of darwin lsan by default
ClosedPublic

Authored by fjricci on Mar 23 2017, 2:13 PM.

Event Timeline

fjricci created this revision.Mar 23 2017, 2:13 PM
kubamracek added inline comments.Mar 27 2017, 3:53 PM
lib/asan/asan_flags.cc
64

asan_flags.cc currently doesn't have any platform-specific code. Could this be moved to some _mac.cc file? Or perhaps into sanitizer_flags.inc?

If not, then this at least deserves a comment explaining that we're bringing up LSan and it doesn't fully work yet on Darwin.

fjricci updated this revision to Diff 93200.Mar 27 2017, 5:00 PM

Account for default detect_leaks value when setting asan default value

fjricci added inline comments.Mar 27 2017, 5:01 PM
lib/asan/asan_flags.cc
64

I think the correct behavior here would be to account for the default value of cf.detect_leaks, as set in sanitizer_flags.inc when determining the default value for asan. However, this appears to be causing some problems with 32-bit lsan tests, where things should also be disabled by default. I'm looking into those test failures now.

64

I think the correct behavior here would be to account for the default value of cf.detect_leaks, as set in sanitizer_flags.inc when determining the default value for asan. However, this appears to be causing some problems with 32-bit lsan tests, where things should also be disabled by default. I'm looking into those test failures now.

Ok, pending D31430, this now passes check-asan tests. Working on Linux check-lsan now, and running into more issues with i386 builds.

Updated that diff, and now check-all passes locally on darwin and linux.

Ping - I think @kubamracek was already mostly fine with this, except the comment I addressed.

kubamracek accepted this revision.Apr 6 2017, 10:11 AM

A patch that removes more lines than adds? LGTM!

This revision is now accepted and ready to land.Apr 6 2017, 10:11 AM
This revision was automatically updated to reflect the committed changes.