Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sanitizer support to Apple platforms #12772

Conversation

keith
Copy link
Member

@keith keith commented Jan 5, 2021

This adds support for asan, tsan, and ubsan on Apple platforms.

Fixes #4984 and #6932

@trybka
Copy link
Contributor

trybka commented Jan 5, 2021

Yeah, this is generally fine.

I'd note that we also set the following flags when any of the sanitizers are in use:

                    flags = [
                        "-O1",
                        "-gmlt",
                        # All failed checks are fatal.
                        "-fno-sanitize-recover=all",
                        "-fno-omit-frame-pointer",

@keith
Copy link
Member Author

keith commented Jan 6, 2021

Would you recommend I add those here? I saw some flags in a previous PR that were similar, but I didn't want to add them w/o knowing if it was something that made sense generally or just for google

@aiuto aiuto requested a review from oquenchil January 6, 2021 04:35
@trybka
Copy link
Contributor

trybka commented Jan 6, 2021

I think they're generally useful.

These two are recommended by the Clang ASan Usage docs:

  • -O1 is to get reasonable performance, especially if your dbg or fastbuild configs default to -O0.
  • -fno-omit-frame-pointer gets better stack traces.

For the other two:

  • -fno-sanitize-recover=all is a good default if you want tests and whatnot to fail immediately, this will be workflow dependent. For a heavy CI usage, you often just want to catch the first failure.
  • -gmlt is just "line number tables only"

YMMV

This adds support for asan, tsan, and ubsan on Apple platforms.

Fixes bazelbuild#4984 and bazelbuild#6932
@keith keith force-pushed the ks/add-sanitizer-support-to-apple-platforms branch from e5d712c to 8e51fdf Compare January 6, 2021 23:32
@keith
Copy link
Member Author

keith commented Jan 6, 2021

Fair enough! Added them here

@jin
Copy link
Member

jin commented Jan 15, 2021

@trybka could you approve this PR? I'll import it as well.

@bazel-io bazel-io closed this in 8959dff Jan 19, 2021
@keith keith deleted the ks/add-sanitizer-support-to-apple-platforms branch January 19, 2021 05:17
@keith
Copy link
Member Author

keith commented Jan 25, 2021

Thanks!

philwo pushed a commit that referenced this pull request Apr 19, 2021
This adds support for asan, tsan, and ubsan on Apple platforms.

Fixes #4984 and #6932

Closes #12772.

PiperOrigin-RevId: 352489421
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for asan/tsan/ubsan on Apple builds on OS X crosstool
3 participants