User Details
- User Since
- Oct 3 2012, 4:55 AM (432 w, 6 d)
Mon, Jan 11
I am reluctant to extend the public interface in ways that
a) are likely to be useful for only few cases
b) are likely to remain libFuzzer-specific
c) already have an existing functionality that can be used instead). I mean the existing -dict flag (it's not exactly what you describe though)
Dec 4 2020
This header is intentionally private, so that the fuzz targets remain engine-neutral.
Dec 2 2020
This worked for us for many years.
Changing the default is likely to break some of the existing users.
Nov 3 2020
did you consider approaches where the emitted code doesn't change, but the binary contains a debug-like metadata that corresponds to the trap instructions?
Matt (CC-ed) has a patch if this kind (for a different purpose) in the works .
Oct 20 2020
LGTM, thanks!
Oct 19 2020
But I'm not sure how best to integrate this -- are there existing crashing tests somewhere I should add this to?
compiler-rt/test/fuzzer
please no #ifdefs.
please add a test.
Oct 16 2020
Sep 23 2020
a drive-by comment -- I would really appreciate *not* adding any new uses of C preprocessor.
Sep 2 2020
Aug 17 2020
+Matt
Aug 14 2020
Would it be possible to add a threaded test that fails w/o this change?
LGTM otherwise, thanks!
Aug 11 2020
Aug 10 2020
would it be acceptable to have an environment variable or launch parameter that could allow the silent creation of these directories?
Aug 6 2020
O, wow, thanks for catching this.
Could you please add a test (in compiler-rt/test/fuzzer) that would reliably fail currently
and reliably pass with this change?
Aug 4 2020
From the description:
this PR adds automatic directory creation for locations in which libFuzzer expects to write data.
I'd rather fail instead of silently creating new dirs, to be consistent with the other behavior
Please fix two nits, then good to go.
Thanks!
Jul 31 2020
Jul 30 2020
Do we need a version for 32-bit at all?
Not having a private version of libc++ is likely to cause subtle stability issues.
The compiler change seems to be completely independent from the libFuzzer change.
Please split this change into two.
Jul 29 2020
Jul 28 2020
Jul 27 2020
LGTM.
Matt, please help land it
Jul 24 2020
Code LGTM, thanks!
Please add a section in docs/LibFuzzer.html.
I'd add it after "Startup initialization", something like "Using libFuzzer as a library".
Jul 23 2020
Yep, cool.
LGTM from me, but please get another pair if eyes (Vitaly?)
I am concerned about this change.
We've essentially exposed an implementation detail (both the function FuzzerDriver
and this header file, with all of its other internal details) to outside users.
This means we have more things to support as an API.
Maybe we could revert it and get back to the drawing board?
Jul 22 2020
In what cases do we still call __dfsan_union?
LGTM
Jul 21 2020
Also, we don't have to err in these functions at all, it's fine to just return silently.
and that's fine. I want this mode to be as simple as possible.
I think this is an overkill.
fast16labels mode should be even simpler:
there are always 16 primary labels, they don't have any descriptions or properties controlled by dfsan.
Jul 8 2020
No strong opinion on whether this needs to be done.
If you feel strong, and if it will help, sure. (you may indeed have to test on various platforms, or rely on the post-commit bots)
OTOH, the new profiler should not require all of these functions, you can probably get away with a custom-tailored variant of MapDynamicShadow.
Will adding attribute((no_sanitize("address"))) to your global solve the problem you are trying to solve?
(sorry for being too terse last time)
Jul 7 2020
can we instead slap an attribute on these special variables?
Jul 6 2020
My preference would be to reject weird file names instead of adding this extra complexity.
Jun 5 2020
LGTM (even though it's sad...)
Jun 4 2020
also, please avoid #ifdefs.
OS-specific code should go to an OS-specific file.
Jun 2 2020
Hi,
This made our ubsan bots red. Please fix or revert ASAP
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/42256
Jun 1 2020
Submitted: https://github.com/llvm/llvm-project/commit/2e6c3e3e7b5eb46452b1819c69919fab820b4233
(had some trouble with arc... pushed via git push instead of arc land)
(update)
(fix typo)
May 29 2020
May 27 2020
Vitaly, please review (and land if ok).
May 26 2020
May 18 2020
(let me land it)
Thanks for this work, and the effort to make the code better!
May 15 2020
May 1 2020
Sorry for the delay. Mostly naming/style nits left.
Apr 22 2020
Please take out the time-related changes for now. If anything, extra changes make the code review process quadratic.
This change causes a performance regression in tsan, as detected on our LLVM buildbot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/49850/steps/tsan%20analyze/logs/stdio
Commenting on just to issues, not the hole patch.
Apr 16 2020
Apr 8 2020
The code is ok, but I'd like to see an ACK from Dmitry.
Feb 28 2020
We *may* need to add more arguments to these callbacks (with the data pointers) later.
Right now I am not sure.
Feb 27 2020
Code LGTM, but please add some comments near the flag definition.
LGTM
Feb 6 2020
thanks!
Feb 3 2020
Sounds good. Max and I will do the next round(s) of review.
Jan 31 2020
LGTM, thanks!
It's exciting that such a small change can bring such a great improvement.
Thanks for the contribution.
Jan 29 2020
It looks like this patch has at least two independent changes.
Please prefer to send two+ individual changes next time. (even if the patches are tiny).
Jan 14 2020
Dec 30 2019
How is this going to work when one thread calls __sanitizer_for_each_extra_stack_range with another thread's ID,
while that other thread creates and discards frames, or while that other thread is being destroyed?
Dec 12 2019
Dec 11 2019
E.g. we have -print_stats that prints machine-readable output, we can do something like that and guarantee it doesn't change.
We want the user to also have user-readable output, though.
Dec 10 2019
We're using an autogenerated trait for this anyway, so we get this for free.
Dec 6 2019
BTW, may I ask you to provide some details of your Rust fuzz target examples?
(like the code of the fuzz target and the output with your patch)
Understood.
Before agreeing with the approach I'd like to hear from more users who need this and some details about their use cases. I've pinged some users.
Dec 5 2019
[sorry for delay, I was OOO]
So, this patch will cause LLVMFuzzerCustomOutput to be called on the reproducer input
which in turn will cause an arbitrarily large input to be printed to stderr (stdin)?
Or in fact, it will cause an arbitrary action to be performed with {Data,Size}
Nov 26 2019
I'd prefer to not overload the public interface and this use-case sounds a bit too corner-case-ish.
Am I wrong?
Nov 4 2019
please avoid #ifdefs -- they are pure evil.
Can you move this function to FuzzerUtil*.cpp?
In this case it will have it's own implementation in FuzzerUtilFuchsia.cpp
Oct 30 2019
LGTM
LGTM, thanks!
Oct 21 2019
Oct 10 2019
I was actually hoping to get rid of this code entirely.
Why do you need this change?