- User Since
- Oct 3 2012, 4:55 AM (289 w, 5 d)
Fri, Apr 20
is this testable?
LGTM with a nit.
Thu, Apr 19
plz don't forget to update the documentation (the asm snippet) at clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html
Wed, Apr 18
Why is it not enough to cal atexit() in LLVMFuzzerInitialize?
Ok, sounds good.
Yes, please add a test that calls Mutate_CopyPart many times and always sees it return MaxSize
libFuzzer is in-process fuzzer, exit is not a libFuzzer-friendly thing.
I am reluctant to allow exits even under a flag -- it will discourage people from making their APIs more fuzzable.
For legacy code that nobody is willing to change we have the out-of-process AFL
Tue, Apr 17
why is this needed?
Mon, Apr 16
Don't we already have something like this in another form?
I wonder how you can observe the change?
It's just a slight change in probabilities.
Or not slight?
Fri, Apr 13
Yep, that's the one, thanks!
Vitaly, please land.
This doesn't affect this review, but next time please upload patches with full context (the 'arc' tool is pretty good at it).
This test is great, but I'd like to also see some more direct test, maybe something in tools/clang/test/Driver/aarch64-fixed-x18.c?
Thu, Apr 12
Hm... interesting. how did you find it? what does this affect now?
I think it should be fixed in Resize().
Also, please add a test here: lib/sanitizer_common/tests/sanitizer_vector_test.cc
Don't you need a test?
srhines@, what kind of documentation change need to be done?
Tue, Apr 3
Tue, Mar 27
+Vitaly for another set of eyes.
LGTM modulo prolog vs prlogue and epilog vs epilogue
Mar 22 2018
please also add a short comparison with Intel CET.
[didn't look at the code yet, just at the docs]
Mar 19 2018
Also: this change has been committed w/o an approval from any of the
please try to avoid this in future.
Is this related to https://github.com/google/sanitizers/issues/914 or this is another problem?
Mar 13 2018
Feb 28 2018
Feb 23 2018
I am indeed interested in experimenting with bounded path coverage, similar to this.
My prior experiments demonstrated some value but also huge corpus expansion (bad).
It might be worth submitting something like this to simplify further experiments.
Feb 22 2018
Feb 21 2018
ouch. 10 is too magic.
I've seen hundreds of these printed in a row, i.e. 10 will be too small.
in other cases 10 will be to large.
I wonder if we want to print it as one more number in regular Fuzzer::PrintStats lines, e.g. "lim: 123", like here:
#3145 REDUCE cov: 6 ft: 7 corp: 5/9b lim: 123 exec/s: 0 rss: 37Mb
Feb 20 2018
We use function attributes in similar situations for asan/tsan/msan.
Similar, but not equivalent, so I don't know if we must follow the same pattern here.
The difference here is that we should keep the ability to turn optimization on and off from command line, regardless of what are the coverage instrumentation flags.
Feb 16 2018
Feb 13 2018
Feb 12 2018
Feb 9 2018
please add a lit test.
Feb 5 2018
LGTM with a nit
Feb 2 2018
Jan 31 2018
Jan 16 2018
How about this:
A -fsanitize-address-poison-all-array-new or similar (it would be all *except* placement new... Haven't got a better name, though).
That way, a user would be able to poison more array-new operators than the current solution. But we wouldn't break any legal C++ code.
Jan 12 2018
Technically it is. Just like overriding malloc,
Jan 11 2018
Let me rephrase the question.
Is the code in new_array_cookie_with_new_from_class.cc a valid C++?
I.e. is the code allowed to access *reinterpret_cast<uintptr_t*>(Foo::allocated) at line 38?
LGTM with two nits, feel free to address them separately.
Jan 10 2018
The original commit doesn't provide any rationale for this test,
Dec 21 2017
Dec 20 2017
Dec 19 2017
LGTM with a nit
I suggest to restart the discussion of this topic with the owners of fortify.
So far I am not convinced that we need/want this code in asan.
The discussion about asan+fortify has been going on for ages and I don't think we ever reached an agreement on how to proceed. Did we?
Dec 18 2017
Dec 14 2017
Dec 13 2017
Matt, please land
Aleksey, please review.
Please also remove samsonov@ from owners (he is not active in LLVM any more, AFAICT) and replace with yourself.
Dec 12 2017
LGTM with one optional question.
Dec 9 2017
Dec 8 2017
LGTM, let's iterate from here.
My top level comment: can we delete all non-aarch64 code?
The arch owners can reinstate it if needed, but they will only need it if/when they have the TBI feature in HW.
Dec 7 2017
LGTM, please wait for (at least) Aleksey's review.
Common code LGTM
Dec 6 2017
please give at least Aleksey a chance to review as well.
Please document the new attribute and explain why the old attribute doesn't work for us (there are cases when we need one, but not the other, in both directions)