-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Clang msan support #918
Comments
We should consider doing this again now that we are under CNCF and have access to more CI resources. MSAN has found a few bugs internally that would have been caught by CI before merging. |
It seems like a lot has changed since this issue originated, I see some configuration for msan on CI and a few bazel configs for it, has anyone tried this recently? I'm curious about how many false positives we would find |
It looks like at this point libc++ is setup correctly for this at least envoyproxy/envoy-build-tools@f741613 |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
Given this test PR #20680 I think we could probably enable this on CI, starting with an opt out list of places that need to be investigated, but it might be too many violations to be ready for prime time. |
Fixing android's api to 29 since it seems like api 30 has some bazel proguard issues which are similar to: bazelbuild/bazel#3777 For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: fixate api_level to be 29 Risk Level: low Testing: ci Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <jp@jpsim.com>
Fixing android's api to 29 since it seems like api 30 has some bazel proguard issues which are similar to: bazelbuild/bazel#3777 For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: fixate api_level to be 29 Risk Level: low Testing: ci Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <jp@jpsim.com>
Clang ASAN/TSAN support was introduced under #443 and #904.
MSAN isn't going to be easy, and questionably worth doing in our current CI environment. You basically need to rebuild lib[std]c++ and all external deps, see https://github.com/google/sanitizers/wiki/MemorySanitizerLibcxxHowTo. I tried it without this and I see false positives such as:
We should consider adding this when we have better CI resourcing for dynamically rebuilding all libraries (so we don't need to bloat the Docker CI image).
The text was updated successfully, but these errors were encountered: