Skip to content

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

Merged
hathach merged 3 commits intohathach:masterfrom
nathaniel-brough:thread_local_globals
Feb 22, 2023
Merged

fix: Change all static variables to thread when fuzzing#1867
hathach merged 3 commits intohathach:masterfrom
nathaniel-brough:thread_local_globals

Conversation

@nathaniel-brough
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
Make one of the messages more accurate, in the event that the user changes `PICO_DEFAULT_RP2350_PLATFORM`
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