- User Since
- Oct 3 2012, 4:55 AM (477 w, 3 d)
Mon, Nov 22
LGTM, but please wait for Vitaly
Fri, Nov 19
Tue, Nov 9
addressed review comments
Mon, Nov 8
Sep 21 2021
Sep 20 2021
Sep 1 2021
Aug 24 2021
Aug 20 2021
one more test fix (can't expect xxd to be present)
fix a test
Aug 19 2021
fix dfsan build
Aug 18 2021
What's the code size implications?
Aug 4 2021
limit the test to x86_64 linux as it is too experimental to be used elsewhere,
and __libfuzzer_extra_counters are linux-only anyway, currently.
fixing incomplete fix, sorry
addressed one more review comment
addressed review comments
Aug 3 2021
One comment that I have is a request to limit the number of #ifdefs in the code to at most one.
We typically achieve this by having platform-specific code in a platform-specific file, guarded with an ifdef.
Aug 2 2021
Jul 15 2021
Jul 12 2021
Thanks for the change!
Indeed, the current single-pass merge is far from perfect, and it's nice to see your numbers.
Jun 17 2021
Yey, great idea! :)
(I am not reviewing the code; but the change looks straightforward)
Jun 7 2021
+Vitaly Buka <email@example.com> +Matt Morehouse <firstname.lastname@example.org>
Mar 2 2021
We can't possibly maintain two variants of scudo.
All effort is currently spent on the newer (standalone) version.
I am afraid we will have to delete the older (non-standalone) variant entirely.
(And the sooner the better)
Feb 24 2021
If I were the maintainer of this file, I would run away from this change.
Not because there is something wrong with it functionality-wise, but because of the ifdefs :(
We ourselves in the sanitizer land would reject a change with this many ifdefs w/o looking further.
Feb 17 2021
(2 bytes per bit!)
1 byte per bit, hopefully. (for the new 8-bit mode only)
DFSan is a separate ABI, you can't mix DFSan-ified code with non-DFSan-ified code.
There is no annotation that we can imagine to work in this case.
DFSan's metadata is per-byte, if we mix different taints in a single byte (8 packet bits) we get an overtaint, i.e. a false positive.
The only other solution for our users is to not use vector<bool>
Feb 11 2021
LGTM, thanks for the better fix!
Feb 8 2021
yea, I am afraid that removing -O1 weakens our ability to find subtle bugs in how sanitizers work with the optimized code.
After all, most of the uses for the sanitizers are with -O1 and higher, so by testing with -O0 we are hiding potential problems.
I think the best is to prevent inlining (noinline attribute, or a command line if available)
Jan 11 2021
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
Oct 19 2020
But I'm not sure how best to integrate this -- are there existing crashing tests somewhere I should add this to?
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
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.
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
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
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?
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
This made our ubsan bots red. Please fix or revert ASAP