-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
infra: refine fuzztest work #8992
Conversation
This is draft. Signed-off-by: David Korczynski <david@adalogics.com>
This is needed for tensorflow. Signed-off-by: David Korczynski <david@adalogics.com>
Cleanup is needed. Signed-off-by: David Korczynski <david@adalogics.com>
Split between CFLAGS and CXXFLAGS. This is needed to make tensorflow work. Related: google/oss-fuzz#8992 Signed-off-by: David Korczynski <david@adalogics.com>
Split CFLAGS and CXXFLAGS into language-specific bazel constructs. This is needed to make Tensorflow work. Related: google/oss-fuzz#8992 Signed-off-by: David Korczynski <david@adalogics.com>
CI is failing because of timeouts, however, in UBSAN we get past the point where the UBSAN FuzzTest fuzzers are build (showing the building works):
|
Signed-off-by: David Korczynski <david@adalogics.com>
@oliverchang I switched this up to focus on fuzztest as it now has a potential fix for #8997 (was missing $ in front of this_dir in the wrapper script). I can do the Tensorflow aspects, which will be only in the Tensorflow project folder afterwards when tensorflow/tensorflow#58646 is merged. This is ready to go from my perspective. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This mostly LGTM. I think just needs some more clarification in terms of documentation.
|
||
# Bazel target names of the fuzz binaries. | ||
FUZZ_TEST_BINARIES=$(bazel query 'kind("cc_test", rdeps(..., @com_google_fuzztest//fuzztest:fuzztest_gtest_main))') | ||
#FUZZ_TEST_BINARIES=$(bazel query 'kind("cc_test", rdeps(..., @com_google_fuzztest//fuzztest:fuzztest_gtest_main))') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just delete this commented line?
chmod +x $OUT/$TARGET_FUZZER | ||
done | ||
done | ||
|
||
# synchronise coverage directory to bazel generated code. | ||
if [ "$SANITIZER" = "coverage" ] | ||
if [[ "$SANITIZER" = "coverage" && ${FUZZTEST_DO_SYNC:-"yes"} == "yes" ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add more details on when FUZZTEST_DO_SYNC
is needed in comments here?
-e 'bazel-testlogs')" | ||
for link in $project_folders; do | ||
if [[ -d "${PWD}"/$link/external ]] | ||
then | ||
rsync -avLk "${RSYNC_FILTER_ARGS[@]}" "${PWD}"/$link/external "${REMAP_PATH}" | ||
fi | ||
if [[ -d "${PWD}"/$link/k8-opt ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise here -- can you add more context in comments for future readers of this code?
@@ -15,24 +15,34 @@ | |||
# | |||
################################################################################ | |||
|
|||
set -x | |||
|
|||
if [[ ${FUZZTEST_TARGET_FOLDER:-"unset"} == "unset" ]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need a bit more documentation on what this is needed for.
@oliverchang updated the docs, let me know what you think! |
Signed-off-by: David Korczynski <david@adalogics.com>
/gcbrun trial_build.py all --sanitizer address --fuzzing-engine libfuzzer |
Thanks! Let's merge this on Monday to avoid any potential breakages over the weekend. |
/gcbrun trial_build.py all --sanitizer address --fuzzing-engine libfuzzer |
/gcbrun trial_build.py all --sanitizer address --fuzzing-engine libfuzzer |
@oliverchang am still not seeing results in the dashboard, is this and google/clusterfuzz#2861 deployed? |
Thanks for flagging @DavidKorczynski. We fixed a few more issues that came up in ClusterFuzz, and things should be moving along now. We should hopefully have things visible on the dashbboard in the next day or two. |
Enable Fuzztest fuzzers for Tensorflow. This depends on google/fuzztest#79 and eventually a PR on the tensorflow repo with the (to be refined) diff in this PR. Signed-off-by: David Korczynski <david@adalogics.com> Signed-off-by: David Korczynski <david@adalogics.com> Co-authored-by: Oliver Chang <oliverchang@users.noreply.github.com>
Enable Fuzztest fuzzers for Tensorflow.
This depends on google/fuzztest#79 and eventually a PR on the tensorflow repo with the (to be refined) diff in this PR.
Signed-off-by: David Korczynski david@adalogics.com