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.

Sign up for GitHub

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

build: fix ubsan oss-fuzz build. #5875

Merged
merged 2 commits into from
Feb 7, 2019
Merged

build: fix ubsan oss-fuzz build. #5875

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ac752e
build: fix ubsan oss-fuzz build.
htuch Feb 7, 2019
f903605
Fix check_format.
htuch Feb 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions bazel/cc_wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ def main():
#
# Similar behavior exists for Clang's `-stdlib=libc++` flag, so we handle
# it in the same test.
if "-static-libstdc++" in sys.argv[1:] or "-stdlib=libc++" in sys.argv[1:]:
if ("-static-libstdc++" in sys.argv[1:] or "-stdlib=libc++" in sys.argv[1:] or
"-std=c++0x" in sys.argv[1:]):
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot figure out why, but this change breaks local builds with --config libc++ (seems to be working fine on the CI, though...).

Copy link
Member Author

Choose a reason for hiding this comment

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

Have you looked any further into this? How would you like to move proceed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did not, and I won't have time to dig further into it anytime soon, sorry!

It's also unclear to me as to why did we need this change to fix UBSan on oss-fuzz, since everything else seems to work fine without it, and without access to oss-fuzz, it's really hard to fix this without breaking UBSan again.

Perhaps @lizan can dig into it? He was the one that originally asked for libc++ support.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was due to the fact that we invoke envoy_cc_wrapper for both CC and CXX for the external deps under external_cmake. The external_cmake build for some deps (e.g. zlib, c-ares) tries to build test/build binaries etc. that don't statically link, and doesn't supply the above flags, but it does set -std=c++0x. The use of the wrong compiler causes confusion specifically because of missing ubsan symbols. At least, that's roughly what I remember.

You can definitely repeat this yourself if you'd like, to run the oss-fuzz build, follow the instructions at https://github.com/envoyproxy/envoy/tree/master/test/fuzz#running-fuzz-tests-locally and replace --sanitizer=address with --sanitizer=undefined.

Copy link
Contributor

Choose a reason for hiding this comment

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

Moved to #5966 to track this.

compiler = envoy_real_cxx
else:
compiler = envoy_real_cc
Expand All @@ -62,7 +63,7 @@ def main():
# unless the user has explicitly set environment variables
# before starting Bazel. But here in $PWD is the Bazel sandbox,
# which will be deleted automatically after the compiler exits.
(flagfile_fd, flagfile_path) = tempfile.mkstemp(dir='./', suffix=".linker-params")
(flagfile_fd, flagfile_path) = tempfile.mkstemp(dir="./", suffix=".linker-params")
with closing_fd(flagfile_fd):
sanitize_flagfile(arg[len("-Wl,@"):], flagfile_fd)
argv.append("-Wl,@" + flagfile_path)
Expand Down