ignore this for now. I've found how to make it even more interesting (by using llvm::InitializeAllTargets, etc), will send an update later.

Interesting. The intent was to optimize the calloc implementation to avoid memset of freshly mmap-ed regions.
Apparently, this optimization when somewhere else (to asan_allocator.cc? or disappeared?)
Anyway, this looks like NFC, so LGTM

LGTM

Code LGTM. What about a test?

the bots complain about a leak in the new test code.
Please fix/revert ASAP.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-clang%20asan/logs/stdio

28905==ERROR: LeakSanitizer: detected memory leaks

LGTM
(Still: OUCH!)

I'm not sure about this one.
standalone lsan is a link-time feature only, it doesn't change the compilation, so the argument of consistency doesn't apply.

LGTM

not sure I understand. you want to move this from common to asan and then back?

Why?
It would be lovely to have the same flag work in other sanitizers...

LGTM

In D33878#772375, @krytarowski wrote:

@kcc this is first step to upstream NetBSD support for sanitizers - to cleanup build issues in compiler-rt.

Why not just change the host compiler to 6.2?
Or, better, to fresh clang?

This is a rather intrusive change and I am not convinced we need it in trunk -- nobody has needed it before so it might be some very rare corner case.
Please explain in more detail why this situation happens and why it can't be avoided.
Why the library is not asan-instrumented?
Are you using dynamic asan run-time or static?

dunno :) / :(

No objections.
Just remember to not mix refactoring/NFC patches and actual changes in logic.

LGTM with a nit

LGTM with a nit.

Done (see https://github.com/google/oss-fuzz/blob/master/projects/llvm_libcxxabi/project.yaml)

Also, are you now maintaining this code?
I am trying to find someone who wants to be CC-ed to other demangler bugs automatically reported by oss-fuzz.

I also encourage you to run the fuzzer on every change in this code.

oss-fuzz finds the assertion failure in this new code:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=1834

In D33472#762849, @george.karpenkov wrote:

so can I make this change + use your test case to verify that we don't end up with extra instrumentation?

What about "is a full post-dominator AND has more than one predecessors" ?

void foo(int *a, int *b) {

if (*a > 0) {
  b[1] = 0;
  if (*a > 10) {
    b[3] = 0;
  } else {
    b[4] = 0;
  }
} else {
  if (*a < -10) {
    b[5] = 0;
  } else {
    b[6] = 0;
  }
  b[2] = 0;
}

}

Hm... I keep not liking this...
A very simple test case will get too many extra edges instrumented if we get rid of PDOMs, even with your extra optimization.
I have such test, but would you like to come up with a test of your own?
If you give up, I'll send mine.

In D33427#762782, @george.karpenkov wrote:

@kcc it has a nice property that tests remain the same.

In D33427#762731, @george.karpenkov wrote:

@kcc what about the following hard'n fast rule: do not instrument basic blocks which have no successors, but have multiple predecessors.

LGTM with a nit. Thanks!

I don't know this code and can't properly comment, but having a constant like this ("7") sounds wrong.
Why not 6, or 8, or 42?

In D31266#759809, @george.karpenkov wrote:

@kcc: sure, great!

In the spirit of disclosure, I'm now pretty certain I'm actually hitting a separate bug where the critical edges are not being split (even though SanitizerCoverage pass calls the proper function. Probably another after instrumentation adds them?.. ).

After an offline discussion...
Looks like me and Vitaly both are busy with other stuff and this thing seems to be blocking you and a few others (and maybe prevents us from finding more bugs).
So let's just delete the PDOM part as George suggests and then later come up with a better strategy (and a way to test it).

In D31266#759656, @george.karpenkov wrote:

The numbers I remember were like DOM gives 30% saving, PDOM gives 20% more, which is a lot.

But how would we know whether those numbers are good?

In D31266#759155, @george.karpenkov wrote:

I think we can make a much simpler change: what if we just skip the optimization of not instrumenting post-dominators?

The code has been reverted, you may want to sync.
Beside, maybe give a chance to the original code author to finish the work?

is a test possible?

Whenever possible, prefer if() to #if
In this case, there is a SANITIZER_ macro for this condition.

Ok.. makes sense. These did not collide before because maybeInsertDynamicShadowAtFunctionEntry is mostly used on WIndows and maybeInsertAsanInitAtFunctionEntry only on Mac. grrr.
Now it does make sense, but I would prefer if the code did not have a duplicated call to maybeInsertAsanInitAtFunctionEntry
(and, of course, a test)

I don't think this is the right fix.
Please explain more: why does this load from __asan_shadow_memory_dynamic_address happen in a function that does not have sanitize_address attribute?

LGTM

That's even worse.
Why can't you add -Dthread_local=__thread to compiler flags if the compiler is old?

LGTM, thanks!
Do you have commit access?

LGTM, but please double-check that we get proper attribute in the fresh compiler.

Yes, I don't like the change -- I really want to have portable modern C++ w/o any old stuff.
Can you do some cmake magic to add -Dthread_local=__thread for old compilers (and only for them)?

LGTM
Sounds good, but I suggest to wait with this change until we fix all the build failures that were caused by enabling libFuzzer build by default.

you don't need fresh clang to build libFuzzer itself.
besides, there is already *some* support for using libFuzzer with gcc.

LGTM

Please also add one full test with -fsanitize=fuzzer in lib/Fuzzer/test (probably, will need to create a subdir). Ok to have it in a separate change.

Mmm... No. I still want to have documentation that includes building libFuzzer manually.
I'd rather insert a paragraph explaining how to use -fsanitize=fuzzer with a fresh clang install, but leave the rest unchanged.