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.

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

[tarantool] Enable centipede engine #8990

Merged
merged 2 commits into from
Nov 25, 2022
Merged

[tarantool] Enable centipede engine #8990

merged 2 commits into from
Nov 25, 2022

Conversation

ligurio
Copy link
Contributor

@ligurio ligurio commented Nov 16, 2022

No description provided.

@oliverchang
Copy link
Collaborator

Looks like the build is failing: https://github.com/google/oss-fuzz/actions/runs/3479908728/jobs/5819035559

@alan32liu any ideas why?

@DonggeLiu
Copy link
Contributor

DonggeLiu commented Nov 22, 2022

I reckon this is another side-effect of Centipede's linking requirement.
It expects us to separate build flags from linking flags, but we did not.
Here when we are building C objects for tzcode, we also pass -ldl, -lrt, -lpthread, and its weak.o, which should be used when linking.

Sometimes that only gives a warning, and other times (e.g., when using -Werror) this exits with an error.
I am not sure if there is an easy way to fix it: We also had similar issues with some benchmarks in FuzzBench and I can only exclude those benchmarks from Centipede's experiments.

@DonggeLiu
Copy link
Contributor

We just came up with a solution (#9030) to let -Werror ignore the unused argument.
once that PR passes all CI tests, I will merge it to master and merge master to your PR : )

@DonggeLiu
Copy link
Contributor

Merged #9030 to master but it seems the new version has not taken effect yet.
The CI tests still use the old set of flags.
Maybe rerun that test in a few hours.

@DonggeLiu
Copy link
Contributor

We updated Centipede to its latest version in the main branch.
Merged that change here as well.

DonggeLiu added a commit to google/fuzzbench that referenced this pull request Nov 23, 2022
Some projects use `-Werror` to turn all warnings into errors.
This affects `Centipede` as we do not separate build and linking flags
as it expects, which leads to `unused-command-line-argument` warnings.
This PR disables turning that specific warning into errors and keeps the
rest the same.

See [the same PR from
OSS-Fuzz](google/oss-fuzz#9030) for more info
and [the error in this PR](google/oss-fuzz#8990)
for its use case.
@ligurio
Copy link
Contributor Author

ligurio commented Nov 23, 2022

Rebased to master branch and force-pushed.

@ligurio
Copy link
Contributor Author

ligurio commented Nov 23, 2022

Seems problem still exist:

[53/139] Linking C static library src/lib/csv/libcsv.a
[54/139] Linking C static library src/lib/csv/libcsv.a
[54/139] Linking C executable test/fuzz/csv_fuzzer
[55/139] Building C object src/lib/uri/CMakeFiles/uri.dir/uri.c.o
clang-15: warning: -ldl: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: -lrt: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: -lpthread: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: /src/centipede/weak.o: 'linker' input unused [-Wunused-command-line-argument]

[55/139] Building C object src/lib/uri/CMakeFiles/uri.dir/uri_parser.c.o
[56/139] Building C object src/lib/salad/CMakeFiles/salad.dir/rtree.c.o
clang-15: warning: -ldl: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: -lrt: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: -lpthread: 'linker' input unused [-Wunused-command-line-argument]
clang-15: warning: /src/centipede/weak.o: 'linker' input unused [-Wunused-command-line-argument]

https://github.com/google/oss-fuzz/actions/runs/3533878458/jobs/5930078187

@DonggeLiu
Copy link
Contributor

DonggeLiu commented Nov 23, 2022

Thanks, I will have another look at it today.
A quick guess is that although our flag -Wno-error=unused-command-line-argument is more specific than their general flags -Werror -Wall and can always overwrite theirs regardless of the flags' relative location, they added another specific flag -Wno-unused-parameter at the very end which overwrites our -Wno-error=unused-command-line-argument.
I will look for solutions and keep everyone updated.

@oliverchang
Copy link
Collaborator

Hmm, this looks different:

: && /usr/local/bin/clang -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize-coverage=trace-loads -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -O2 -gline-tables-only  -Wno-error=unused-command-line-argument -ldl -lrt -lpthread /src/centipede/weak.o  -fsanitize=fuzzer-no-link -fexceptions -funwind-tables -fno-common -msse2  -fmacro-prefix-map=/src/tarantool=. -std=c11 -Wall -Wextra -Wno-gnu-alignof-expression -Wno-cast-function-type -Werror -Wno-unused-parameter -g -ggdb -O0 -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize-coverage=trace-loads -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -O2 -gline-tables-only  -Wno-error=unused-command-line-argument -ldl -lrt -lpthread /src/centipede/weak.o  -fsanitize=fuzzer-no-link -stdlib=libc++    -rdynamic test/fuzz/CMakeFiles/csv_fuzzer.dir/csv_fuzzer.c.o -o test/fuzz/csv_fuzzer  src/lib/csv/libcsv.a  -Wl,-Bstatic  -lFuzzingEngine  -Wl,-Bdynamic && :
/usr/bin/ld: /lib/libFuzzingEngine.a(runner.pic.o): in function `LLVMFuzzerMutate':
runner.cc:(.text.LLVMFuzzerMutate+0x43): undefined reference to `operator new(unsigned long)'
/usr/bin/ld: runner.cc:(.text.LLVMFuzzerMutate+0xae): undefined reference to `operator delete(void*)'
/usr/bin/ld: runner.cc:(.text.LLVMFuzzerMutate+0xeb): undefined reference to `operator delete(void*)'
/usr/bin/ld: /lib/libFuzzingEngine.a(runner.pic.o): in function `centipede::DumpPcTable(char const*)':

@DonggeLiu
Copy link
Contributor

Hmm, this looks different:

: && /usr/local/bin/clang -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize-coverage=trace-loads -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -O2 -gline-tables-only  -Wno-error=unused-command-line-argument -ldl -lrt -lpthread /src/centipede/weak.o  -fsanitize=fuzzer-no-link -fexceptions -funwind-tables -fno-common -msse2  -fmacro-prefix-map=/src/tarantool=. -std=c11 -Wall -Wextra -Wno-gnu-alignof-expression -Wno-cast-function-type -Werror -Wno-unused-parameter -g -ggdb -O0 -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize-coverage=trace-loads -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -O2 -gline-tables-only  -Wno-error=unused-command-line-argument -ldl -lrt -lpthread /src/centipede/weak.o  -fsanitize=fuzzer-no-link -stdlib=libc++    -rdynamic test/fuzz/CMakeFiles/csv_fuzzer.dir/csv_fuzzer.c.o -o test/fuzz/csv_fuzzer  src/lib/csv/libcsv.a  -Wl,-Bstatic  -lFuzzingEngine  -Wl,-Bdynamic && :
/usr/bin/ld: /lib/libFuzzingEngine.a(runner.pic.o): in function `LLVMFuzzerMutate':
runner.cc:(.text.LLVMFuzzerMutate+0x43): undefined reference to `operator new(unsigned long)'
/usr/bin/ld: runner.cc:(.text.LLVMFuzzerMutate+0xae): undefined reference to `operator delete(void*)'
/usr/bin/ld: runner.cc:(.text.LLVMFuzzerMutate+0xeb): undefined reference to `operator delete(void*)'
/usr/bin/ld: /lib/libFuzzingEngine.a(runner.pic.o): in function `centipede::DumpPcTable(char const*)':

Ah thanks, I just started looking at the errors.
Very happy it is not about flags again: FuzzBench had that issue, and I do not know the solution to it.

This is also new to me, I will see what I can do to fix this.

@DonggeLiu
Copy link
Contributor

DonggeLiu commented Nov 24, 2022

Hi @ligurio, we fixed the second error by modifying the CMakeLists.txt.
Would there be a better way to implement it upstream? (i.e., when the fuzzer is centipede, always add -lc++ after centipede's LIB_FUZZING_ENGINE during compilation)
Thanks!

@DonggeLiu DonggeLiu merged commit 772a7bd into google:master Nov 25, 2022
eamonnmcmanus pushed a commit to eamonnmcmanus/oss-fuzz that referenced this pull request Mar 15, 2023
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