LGTM

err... partially taking my word back.. I do want to keep this test to use STL.
Instead of this change, please create another test, e.g. SimpleTestStdio.cpp, that won't use STL, and use the new test in msan runs.

I'd prefer to not have this functionality in lsan -- it already has __lsan_do_recoverable_leak_check which should be sufficient for a user to implement this functionality on their side.
(If not, that may need to be fixed).

Matt, please test locally and land.

Maybe just change the msan-ish test to not use STL

LGTM

In D48150#1158253, @metzman wrote:

In D48150#1158252, @kcc wrote:

In D48150#1158238, @metzman wrote:

@kcc @morehouse It doesn't look like a coverage report with new instrumentation will be feasible.

I don't understand why.

I don't think sancov reports are possible using the new instrumentation. Not 100% about this, but it doesn't seem to work like it did for the old instrumentation.

In D48150#1158238, @metzman wrote:

@kcc @morehouse It doesn't look like a coverage report with new instrumentation will be feasible.

It it bothers someone, we can turn this function into a non-recursive by implementing a fixed-size queue (array of 2^16 shorts).
Or you can run the ExplodeDFSanLabelsTestDF with a large value of "ulimit -s".

yea, you are right, you can't directly use F->RunningCB if you don't have F visible.
But I don't want to have two copies of this flag, one in Fuzzer and one in TracePC.

In D48891#1156587, @kcc wrote:

LGTM

Please also update the docs (http://llvm.org/docs/LibFuzzer.html) once we are confident that msan+libfuzzer works out of the box

Please also update the docs (http://llvm.org/docs/LibFuzzer.html) once we are confident that msan+libfuzzer works out of the box

Matt, please land it

In D48800#1152873, @pdknsk wrote:

In D48800#1151316, @kcc wrote:

Why do you need the new variable InCB?
Will the existing RunningCB not work?

To be honest, my C++ knowledge becomes wobbly here. I don't think the hooks can access Fuzzer (and its RunningCB).

sure, I can commit (done). thanks!

lgtm

whatever :)
even just having the literal constant "-ignore_remaining_args=1" in two places is fine.

option two or some flavor of it would be much more preferable.
Maybe just declare a global constant kIgnoreRemaining above the definition of 'class Command' (inside the fuzzer namespace)

errr... I'd prefer not to...
do you know why this happens?

LGTM

In D48800#1150953, @pdknsk wrote:

In D48800#1150324, @kcc wrote:

Why is it safe to remove ScopedDoingMyOwnMemOrStr from the places you've removed it from?

Note that this removes ScopedDoingMyOwnMemOrStr completely. It's safe because the functions using it run outside the callback. MakeDictionaryEntryFromCMP before, operator== (used in ContainsWord) after, and operator< appears unused.

It'd be different if the functions (TPC.AddValueForMemcmp, TPC.MMT.Add) used inside the hooks (recursively) triggered the hooks, but that's not the case.

a couple of nits, then good to go.

Why is it safe to remove ScopedDoingMyOwnMemOrStr from the places you've removed it from?

LGTM

You may try projects/compiler-rt/lib/sanitizer_common/scripts/sancov.py instead of sancov, this tool is less restrictive.
pipe the output of sancov.py through llvm-symbolizer

In D48150#1148584, @morehouse wrote:

In D48150#1148532, @kevinwkt wrote:

"ERROR: Coverage points in binary and .sancov file do not match." is what I am getting which I am assuming I am sending the wrong PC. But then which PC should be sent?

Is this coming from sancov? I think it might only support trace-pc-guard...

@kcc: How can we test this with inline counters?

add a .lit test, please

the current patch is hard to review as it contains tons of formatting (or otherwise unrelated) fixed.
Plz don't mix formatting/refactoring with meaningful changes in one patch.

Hm... I thought this is intentional -- I do want to test libFuzzer with O2 by default.
Matt?

same with ShrinkControlFlowTest. I've run it 10000 times.

also:

>> 10^6 iterations already take ~20 seconds, would be hesitant to bump it more.

Any sources of non-determinism you suspect?

oh, anything could be different. I wouldn't expect seed=1 to behave the same on different platforms.
Need to check why this test is flaky (too few iterations?)

LGTM
Please watch the bots -- I can imagine it can fail in lots of ways.

Some feedback on the generated code:

is this testable (somewhere in test/fuzzer/afl-driver*)?

LGTM

what's the process for getting this landed from here?

Matt, please make the first pass.

LGTM

are tests possible here?

LGTM, thanks!

For data flow test, make sure to rebuild dfsan (ninja check-dfsan)

Yes, I think a much safer fix would be to add

unsigned struct_ustat_sz = <SOMETHING>;  // glibc >= 2.28 doesn't have <sys/ustat.h> so we can't include it and use  sizeof(struct ustat);

the ustat interceptor has been added in 2013, probably as part of the work on msan.

Is this code review stuck?

LGTM

Looking forward to trying to attack it.
But indeed, for examples like this, KLEE (or other symexec) is clearly more powerful... today.

LGTM