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

also catch rust error messages that have been caught and resumed #3216

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Ekleog-NEAR
Copy link

This, in particular, should make fuzzers built with cargo-bolero able to get the proper error message auto-detected, rather than a generic abort.

PS: Hopefully this should be small enough to not require a CLA, as I am contributing on behalf of my employer and the process to get a CLA signed would be long for no reason. If you do need a CLA even for the smallest contributions, please feel free to let me know and I’ll see what I can do. But also don’t hesitate to do the change yourselves as though this were just a bug report, I’m just opening this to make your life simpler.

This, in particular, should make fuzzers built with [cargo-bolero](https://github.com/camshaft/bolero) able to get the proper error message auto-detected, rather than a generic abort.
@google-cla
Copy link

google-cla bot commented Jul 13, 2023

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.

@Ekleog-NEAR
Copy link
Author

Turns out the CLA process was easier than I expected, so that’s now done. AFAICT the remaining CI check is just someone from google needing to look at the PR, so I can’t do anything about it.

@jonathanmetzman
Copy link
Collaborator

We do require a CLA but I think I can probably make this contribution myself.

@jonathanmetzman
Copy link
Collaborator

/gcbrun

@jonathanmetzman
Copy link
Collaborator

Could you add a test?

@Ekleog-NEAR
Copy link
Author

Ekleog-NEAR commented Aug 10, 2023

I just added a test by copy-pasting the test contents from our clusterfuzz UI, and ./local/tests/ci_tests.bash seems to pass, so hopefully this will be good to go :)

@Ekleog-NEAR
Copy link
Author

@jonathanmetzman Not sure whether you saw, but this is I think ready for review round 2 :)

@Ekleog-NEAR
Copy link
Author

@jonathanmetzman I don’t want to bother you so I’ll try just one last ping, I think this is ready for review :)

Anyway, thank you for the work you do on clusterfuzz, and hope this can help, whenever you manage to get to it!

Copy link
Collaborator

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

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

LGTM
@adetaylor @oliverchang WDYT of this change?

@Ekleog-NEAR
Copy link
Author

/gcbrun

@Ekleog-NEAR
Copy link
Author

Welp seems like I don’t have permission to run this. Anyway, what I can say for sure is, ./local/tests/ci_tests.bash passes locally :)

@Ekleog-NEAR
Copy link
Author

Actually, a thought: would it be better to get the line and char out of the summary? I’d just need to end the regex at : instead of end-of-line; and it might help with clusterfuzz automatically detecting that it’s the same fuzzer crash. OTOH, if it doesn’t actually help clusterfuzz detect that, then it’s probably better to just leave it, as it makes reading the crash easier.

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