-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix oss-fuzz build #2869
Fix oss-fuzz build #2869
Conversation
THis fixes incorrect module path and also add proper tags for FuzzUIDMap. Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32109#c1 Signed-off-by: Daniel Dao <dqminh89@gmail.com>
@AdamKorcz PTAL |
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.
LGTM, and thanks!
compile_go_fuzzer ./libcontainer/system FuzzUIDMap id_map_fuzzer linux | ||
compile_go_fuzzer ./libcontainer/user FuzzUser user_fuzzer | ||
compile_go_fuzzer ./libcontainer/configs FuzzUnmarshalJSON configs_fuzzer | ||
compile_go_fuzzer github.com/opencontainers/runc/libcontainer/system FuzzUIDMap id_map_fuzzer linux,gofuzz |
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.
The linux
tag is probably redundant -- as long as fuzzing is done on Linux.
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.
OSS-fuzz only fuzz on Linux.
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.
OSS-fuzz only fuzz on Linux.
This means that the linux tag is there implicitly (set by the platform), and so it is redundant.
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.
Anyway, this can be addressed separately
@dqminh Thank you for fixing this! The broken build is the coverage build. To test that build specifically locally you need to run: The specific part of the documentation is here: https://google.github.io/oss-fuzz/advanced-topics/code-coverage/#build-fuzz-targets When I test the updated build file locally I get the error: Could you tell me if this was tested with the |
I tested this one again, and it works fine with both address sanitizer and coverage sanitizer which is exactly what we need. My previous test might have been with an OSS-fuzz image that was not the latest. LGTM |
close/reopen to kick the latest ci |
@AkihiroSuda PTAL |
Build was fixed in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32109#c6 🥳 |
THis fixes incorrect module path and also add proper tags for
FuzzUIDMap.
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32109#c1
Signed-off-by: Daniel Dao dqminh89@gmail.com