Skip to content

Conversation

htuch
Copy link
Member

@htuch htuch commented Apr 16, 2019

…rc sanitizers.

This allows for all --copt/--cxxopt/--linkopt to be plumbed to external
dependencies.

Risk level: Low
Testing: CI

Signed-off-by: Harvey Tuch htuch@google.com

…rc sanitizers.

This allows for all --copt/--cxxopt/--linkopt to be plumbed to external
dependencies.

Risk level: Low
Testing: CI

Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch htuch requested a review from lizan April 16, 2019 22:29
@lizan
Copy link
Member

lizan commented Apr 17, 2019

@htuch ASAN/TSAN test failed, so the flags are propagated down to dependencies?

@htuch
Copy link
Member Author

htuch commented Apr 17, 2019

@mergeconflict is looking at the TSAN failure related to evwatch.

@htuch htuch mentioned this pull request Apr 17, 2019
Signed-off-by: Harvey Tuch <htuch@google.com>
@mergeconflict
Copy link
Member

@mergeconflict is looking at the TSAN failure related to evwatch.

libevent/libevent#802

@mergeconflict
Copy link
Member

@mergeconflict is looking at the TSAN failure related to evwatch.

libevent/libevent#802

#6637

htuch pushed a commit that referenced this pull request Apr 18, 2019
 @htuch discovered a race condition in my libevent watcher implementation in the process of enabling TSAN for dependencies (#6610). Update libevent to pull in the fix (libevent/libevent#793).

Risk Level: low
Testing: bazel test //test/server:worker_impl_test -c dbg --config=clang-tsan --runs_per_test=1000 (with @htuch's patch applied).

Signed-off-by: Dan Rosen <mergeconflict@google.com>
htuch added 4 commits April 23, 2019 14:47
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added 2 commits April 24, 2019 13:52
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch
Copy link
Member Author

htuch commented Apr 24, 2019

@lizan this is ready for final review, all tests should now pass.

+ # Blacklist LuaJIT from ASAN for now.
+ # TODO(htuch): Remove this when https://github.com/envoyproxy/envoy/issues/6084 is resolved.
+ if "ENVOY_CONFIG_ASAN" in os.environ:
+ os.environ["TARGET_CFLAGS"] += " -fsanitize-blacklist=%s/com_github_luajit_luajit/clang-asan-blacklist.txt" % os.environ["PWD"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do fun:* blacklist, shall we just remove -fsanitize=address from TARGET_CFLAGS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a useful hook to leave around to experiment with making this more precise, so I'd vote to keep it as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@lizan lizan merged commit 65092ea into envoyproxy:master Apr 24, 2019
htuch added a commit to htuch/envoy that referenced this pull request Apr 26, 2019
Since envoyproxy#6610 the fuzzer build has
been broken. This is due to the interaction of rules_foreign_cc external
dependencies and the additional UBSAN blacklist maintained by the
oss-fuzz driver to workaround the fact we don't have
protocolbuffers/protobuf#5901 yet.

Thir PR moves protocolbuffers/protobuf#5901 into
Envoy proper and hence we don't need an UBSAN blacklist in the oss-fuzz
driver anymore.

Risk level: Low
Tesitng: oss-fuzz Docker build.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit to htuch/oss-fuzz that referenced this pull request Apr 26, 2019
This is no longer needed as we handle this Envoy-side. This should fix
the build that has been broken since
envoyproxy/envoy#6610.

Signed-off-by: Harvey Tuch <htuch@google.com>
inferno-chromium pushed a commit to google/oss-fuzz that referenced this pull request Apr 26, 2019
This is no longer needed as we handle this Envoy-side. This should fix
the build that has been broken since
envoyproxy/envoy#6610.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit that referenced this pull request Apr 26, 2019
Since #6610 the fuzzer build has
been broken. This is due to the interaction of rules_foreign_cc external
dependencies and the additional UBSAN blacklist maintained by the
oss-fuzz driver to workaround the fact we don't have
protocolbuffers/protobuf#5901 yet.

This PR moves protocolbuffers/protobuf#5901 into
Envoy proper and hence we don't need an UBSAN blacklist in the oss-fuzz
driver anymore.

Risk level: Low
Tesitng: oss-fuzz Docker build.

Signed-off-by: Harvey Tuch <htuch@google.com>
jeffpiazza-google pushed a commit to jeffpiazza-google/envoy that referenced this pull request May 3, 2019
Since envoyproxy#6610 the fuzzer build has
been broken. This is due to the interaction of rules_foreign_cc external
dependencies and the additional UBSAN blacklist maintained by the
oss-fuzz driver to workaround the fact we don't have
protocolbuffers/protobuf#5901 yet.

This PR moves protocolbuffers/protobuf#5901 into
Envoy proper and hence we don't need an UBSAN blacklist in the oss-fuzz
driver anymore.

Risk level: Low
Tesitng: oss-fuzz Docker build.

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Jeff Piazza <jeffpiazza@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants