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

Large fuzz target fails due to _HF_PC_GAURD_MAX being too small #19

Closed
ghost opened this issue Jan 28, 2019 · 7 comments
Closed

Large fuzz target fails due to _HF_PC_GAURD_MAX being too small #19

ghost opened this issue Jan 28, 2019 · 7 comments

Comments

@ghost
Copy link

ghost commented Jan 28, 2019

Our CI fuzzing recently broke as our fuzz target became too large so that honggfuzz would fail with errors like This process has too many PC guards. It seems this is based on a hard-coded constant _HF_PC_GUARD_MAX in the honggfuzz.h header which can only be changed when honggfuzz is built.

Hence, for now I resorted to adding

// increase _HF_PC_GUARD_MAX
let status = Command::new("sed")
    .args(&["-e", "s/^#define _HF_PC_GUARD_MAX .*/#define _HF_PC_GUARD_MAX (2U * 1024U * 1024U * 16U)/", "-i", "honggfuzz/honggfuzz.h"])
    .status()
    .expect("failed to patch hongfuzz.h using sed");
assert!(status.success());

to this crate's build script.

Would this be something you would accept upstream (controlled via an environment variable by the Cargo hfuzz subcommand)? Do you have any other ideas how to avoid this problem? Thank you for your help!

@PaulGrandperrin
Copy link
Member

Hi @adam-rhebo,
No problem we'll do something to solve your use-case.
I'll just ping @robertswiecki (the upstream developer of the "real" honggfuzz code base) just to know if he has a better idea of how to implement that and if there's something that could be done directly in his code base :-)

@robertswiecki
Copy link

We can probably change it for everybody, is 32Mo guards sufficient for your tests?

@ghost
Copy link
Author

ghost commented Jan 28, 2019

We can probably change it for everybody, is 32Mo guards sufficient for your tests?

For now 2U * seems to suffice, but I only expect to add more code to that particular fuzz target, so I would be happy with 64 MB as well. But I admittedly do not understand the downsides of investing a larger amount of memory. Will performance suffer is the guards array becomes bigger?

@robertswiecki
Copy link

Unsure, it's probably large enough already that CPU caching doesn't matter much at this point, so maybe not.

@robertswiecki
Copy link

Please try at HEAD or with google/honggfuzz@e2be7a9

@ghost
Copy link
Author

ghost commented Jan 28, 2019

Please try at HEAD or with google/honggfuzz@e2be7a9

Works like a charm and I am seeing no significant reduction fuzzing speed. Thank you very much for your swift help!

@PaulGrandperrin
Copy link
Member

That's great! I'll pull that and issue a new release asap 🙂

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

No branches or pull requests

2 participants