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

Support GCC for unit test mode #1393

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

aleksisch
Copy link
Contributor

Official documentation says:

GCC is not yet supported. Unittest mode should work but it's not yet tested.

Right now it fails to compile. GCC doesn't support the flag sanitize-coverage which is used inside fuzztest to disable (I guess) sanitizers in some internal directories.

I believe we should do any of this:

  1. Report an error (or warning) for all cases with GCC like it was done here.
  2. Skip this option in case of GCC (what I did in this PR).
    2.1 I don't know the reason behind this sanitize-coverage=0, maybe we can remove it.

Copy link

google-cla bot commented Sep 28, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@russkel
Copy link

russkel commented Oct 23, 2024

Any chance this can be merged?

@lszekeres lszekeres self-requested a review October 26, 2024 01:14
Copy link
Member

@lszekeres lszekeres left a comment

Choose a reason for hiding this comment

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

Thank you for the PR and thank you for your patience!

This "codelab" CI workflow is failing. It's because we pass the PR's branch name here to CMake over here, and it fails because that branch doesn't exist.
It doesn't exist because it's from your repo, so I guess we'd need to do with the repo the same as the branch over here.

Do you think you could fix this?

@lszekeres
Copy link
Member

Any chance this can be merged?

Yes! (we just need to make the CI green)

@aleksisch
Copy link
Contributor Author

Thank you for the PR and thank you for your patience!

This "codelab" CI workflow is failing. It's because we pass the PR's branch name here to CMake over here, and it fails because that branch doesn't exist. It doesn't exist because it's from your repo, so I guess we'd need to do with the repo the same as the branch over here.

Do you think you could fix this?

Hi. Thank you for your response. Could you explain how I can create a branch in the google/fuzztest repo? I didn't find a way to do it

@lszekeres
Copy link
Member

Hi. Thank you for your response. Could you explain how I can create a branch in the google/fuzztest repo? I didn't find a way to do it

No, what I meant was that I think we need to pass in github.repository similarly as we pass in github.ref_name here to CMake here.

Copy link
Member

@lszekeres lszekeres left a comment

Choose a reason for hiding this comment

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

Thanks!

@copybara-service copybara-service bot merged commit b19cc13 into google:main Nov 7, 2024
15 of 16 checks passed
-D CMAKE_BUILD_TYPE=RelWithDebug \
-D FUZZTEST_BUILD_TESTING=on \
&& cmake --build build -j $(nproc)
# TODO: Rewrite helper `PrintValue` to obtain same output under clang and gcc
Copy link
Member

Choose a reason for hiding this comment

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

@aleksisch, just wanted to check if you are planning to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aleksisch, just wanted to check if you are planning to do this.

Yes, I'll investigate it

@russkel
Copy link

russkel commented Nov 7, 2024

I hope this marks the start of decent GCC support ❤️

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