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

CI: enable fuzz test build with cmake #4743

Merged
merged 58 commits into from
Sep 10, 2024

Conversation

jouho
Copy link
Contributor

@jouho jouho commented Aug 29, 2024

Resolved issues:

Currently, fuzz tests are only built with the Make build system. This change allows us to build fuzz tests using CMake as part of our ongoing effort to migrate from Make to CMake. The primary goal of this PR is to replicate the current functionality provided by Make in CMake. Additional improvements to fuzzing are planned and are described in the call-outs section.

Description of changes:

This change enables building fuzz tests with CMake. The commands to build and run fuzz tests:

  cmake . -Bbuild \
          -DCMAKE_PREFIX_PATH=$LIBCRYPTO_ROOT \
          -DS2N_FUZZ_TEST=on
  cmake --build ./build -- -j $(nproc)
  cmake --build build --target run_fuzz 

Detailed description of changes:

CMakeLists.txt

  • Added an option to turn on fuzz test mode. This will create build target for fuzz tests, which can be invoked by running cmake --build build --target run_fuzz after compiling

codebuild/bin/s2n_codebuild.sh

  • Updated to use the CMake build system instead of Make when the test target is set to "fuzz". Note: There is currently an issue when running this in CI. Related issue: #4633.

codebuild/spec/buildspec_fuzz.yml

  • Introduced as an alternative to using s2n_codebuild.sh. This approach ensures that modifications to how fuzz tests are run can be managed through code reviews, avoiding direct edits on CodeBuild (which is what we do now).

tests/fuzz/runFuzzTest.sh

  • Limited the scope of the LD_PRELOAD path to prevent interference with external commands, such as mkdir, ensuring it only affects fuzz tests.

utils/s2n_result.c & utils/s2n_result.h

  • Addressed a compilation error when compiling function overrides like global_overrides.c, which could not find the definition of s2n_result_is_ok. Including the source resulted in multiple definition errors due to overrides defining their own definitions. This was resolved by inlining the definition of s2n_result_is_ok into s2n_result.h, as part of the optimization process discussed in this PR #4739.

Call-outs:

Waiting for PR #4739 to be merged to fully apply the s2n_result inline change.

There are several follow-up tasks for general fuzzing improvements that needs to be done. The tasks are being tracked at #4748

Testing:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Aug 29, 2024
- change link option to public
- move libfuzzer path definition to buildspec
- use target_include_directories instead
- comment to explain LD_PRELOAD issue
- address nits
codebuild/spec/buildspec_fuzz.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz.yml Show resolved Hide resolved
- remove -O0 and -fvisibility=default
- remove make specific logic in buildspec
@jouho jouho requested review from maddeleine and removed request for camshaft September 4, 2024 23:11
- use ctest to run fuzz tests
- remove unnecessary compiler options
@jouho jouho requested review from jmayclin and dougch September 5, 2024 00:47
codebuild/spec/buildspec_fuzz.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz.yml Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
jouho and others added 2 commits September 5, 2024 16:59
- change compute type
- add comment explaining cmake arg
- revert inline changes to better handle merge conflict
@jouho jouho changed the title chore: enable fuzz test build with cmake ci: enable fuzz test build with cmake Sep 5, 2024
@jouho jouho changed the title ci: enable fuzz test build with cmake CI: enable fuzz test build with cmake Sep 5, 2024
CMakeLists.txt Show resolved Hide resolved
- fix cmake command for codebuild.sh
- new message to indicate when fuzz is enabled
@jouho jouho requested a review from dougch September 6, 2024 17:27
@jouho jouho enabled auto-merge (squash) September 6, 2024 21:34
@jouho jouho merged commit a3625fb into aws:main Sep 10, 2024
36 checks passed
@jouho jouho deleted the move-fuzz-from-make-to-cmake branch September 10, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants