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

fix: Change all static variables to thread when fuzzing #1867

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

nathaniel-brough
Copy link
Contributor

Describe the PR
Currently the AFL fuzzing engine makes the assumption that it can run a library in multiple threads without any shared state. To make this assumption consistent with tinyusb we need to make static globals thread local when fuzzing.

Additional context
Discussed here:
#1715 (comment)
Actual issue here (see stability)
https://oss-fuzz.com/fuzzer-stats?project=tinyusb&fuzzer=afl&job=afl_asan_tinyusb&group_by=by-fuzzer

Copy link
Owner

@hathach hathach 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 very much for your PR and sorry for the delay, I was too busy with other paid works. I back off a bit on using TU_STATIC, instead I would prefer to add _fuzz_thread as additional keyword. Even though it is a bit more verbose, the static keyword is more apparent to other user and easier to read.

I also rename macro FUZZ to _FUZZ to avoid if user application define it in the future .

Note: we could change _fuzz_thread to less verbose word in the future if you have any other suggestion. For now, I think this is good for merge

@hathach hathach merged commit 557bf82 into hathach:master Feb 22, 2023
@HiFiPhile
Copy link
Collaborator

Personally I prefer to use TU_STATIC instead of adding _fuzz_thread keyword.

For most people who doesn't use fuzzing it's confusing what this keyword does, and I feel also the code is less neat putting a test keyword inside the stack.

For anyone who contribute they also need to learn what fuzzer does instead of copy TU_STATIC from existing code.

@hathach
Copy link
Owner

hathach commented Feb 22, 2023

_fuzz_thread is __thread which is an actual keyword for thread_local (https://gcc.gnu.org/onlinedocs/gcc/Thread-Local.html#Thread-Local) . I was thinking to actual make use of it later on. Though you made the point, I think we should stick with tu_static instead.

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