make operator new more standard compliant

[ramosian-glider]

Originally reported on Google Code with ID 295

Users sometimes complain that asan's operator new is not fully standard-compliant
when it comes to handling out of memory situation.

In particular: 

  - new-nothrow does not return 0 by default (it does with ASAN_OPTIONS=allocator_may_return_null=1)
  - new does not throw std::bad_alloc
  - either variant does not call new_handler 

Implementing these features appears to be surprisingly difficult.

The implementation of asan's operator new resides in asan/asan_new_delete.cc
http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_new_delete.cc?view=markup

asan's new/delete is essentially a user replacement of these operators. 
This has a side effect: a user can not replace new/delete with his own variants,
(this is undefined behavior: http://en.cppreference.com/w/cpp/memory/new/operator_new)

Throwing exceptions from asan/asan_new_delete.cc is currently not possible because
we build the entire run-time w/o exceptions. 
We can overcome this by having special flags for asan/asan_new_delete.cc.
Then there is a problem of linking with c++ lib.
asan run-time does not depend on c++ run-time and can be used with pure C w/o linking
c++.
We probably may overcome this my splitting asan_new_delete.cc into a separate library
asan-rt-cxx that will be linked in asan mode only for C++ programs. 

Calling new_handler from asan's operator new would be possible in c++11, where 
std::get_new_handler() is present, but we can't rely on this yet
(libstdc++ in gcc 4.8.2 does not have std::get_new_handler).
And again, the same problem with linking with c++ lib.

We can also simply remove our new/delete interceptors.
This will route new/delete to the standard c++ library and will solve the problem completely
(assuming new/delete call malloc/free),
but we will lose the ability to detect new/free and new[]/delete mismatch bugs. 

So, the possible plan is: 
- split asan run-time, moving asan/asan_new_delete.cc into a separate library and link
it only when libstdc++/libc++ is linked.
- make our operator new() throw bad_alloc
- declare std::get_new_handler() as weak and if it is present, handle the new_handler
case.

OMG, that sounds so hard, and for very little outcome.
I am tempted to leave it as is unless we find an easier route.
Better implementation ideas are welcome. 

Reported by konstantin.s.serebryany on 2014-04-15 12:00:10

[ramosian-glider]

By removing new/delete interceptors we also lose allocation stacks on ARM, because standard
C++ libraries are often built without frame pointers, or with incompatible frame pointers
(ARM has no standard FP location in the stack frame).

Reported by eugenis@google.com on 2014-04-15 12:11:10

[ramosian-glider]

I agree with Evgeniy - dropping new/delete replacements is a bad idea - we will almost
certainly have worse stack traces even on Linux. Splitting ASan runtime in two parts
(i.e. moving C++-specific bits into a separate library and link it if necessary) is
quite possible - we do it in UBSan already (there are clang_rt.ubsan.a and clang_rt.ubsan_cxx.a),
and have the required logic in driver.

Reported by samsonov@google.com on 2014-04-15 21:26:12

[ramosian-glider]

let us split the run-time first. 

Reported by konstantin.s.serebryany on 2014-04-16 07:20:38

[ramosian-glider]

Oh, good.
Having an example to work from will make it much easier, I suspect.

� Marshall

Reported by mclow.lists on 2014-04-16 13:41:30

[ramosian-glider]

http://llvm.org/bugs/show_bug.cgi?id=19660 is asking to support 
custom operator new()

Reported by konstantin.s.serebryany on 2014-05-06 11:42:19

[ramosian-glider]

http://llvm.org/viewvc/llvm-project?view=revision&revision=208609 splits ASan runtime
in two parts. Now it should be possible to add better support for bad_alloc, and compile
asan_cxx part with exceptions.

Reported by samsonov@google.com on 2014-05-12 18:48:53

[jwakely]

I've just spent many many hours ensuring the libstdc++ testsuite can run with sanitizers, and all our code for handling allocation failure is untestable due to this issue. Every test that tries to handle std::bad_alloc either crashes when ASan fails to allocate, or crashes when it returns null and we dereference it.

This makes ASan unusable for testing large pieces of the C++ standard library.

[jwakely]

And to be clear,

operator new is not fully standard-compliant

is inaccurate, it directly violates an absolute requirement of the standard.

That's not just a case of being "not fully standard-compliant".

[kcc]

We would be happy to accept small incremental patches fixing the problem piece-by-piece, but at the moment we don't have enough hands to fix it ourselves. While I totally agree that the current behavior is not standard compliant, the only ones to complain are the libc++ developers.

[mclow]

I tried to make a patch to compiler-rt to fix this, but ran into the fact that you build w/exceptions disabled.

[jwakely]

the only ones to complain are the libc++ developers.

And the libstdc++ maintainer.

[kcc]

mclow@, yes, we will need to change the build rules to build a small part of run-time w/ exceptions.

[rnk]

Do we really need to enable exceptions? Can't we just call __cxa_throw from __asan_new?

[jwakely]

And __cxa_allocate and construct a bad_alloc object.

For libstdc++ you can call std::__throw_bad_alloc() but that doesn't help for libc++ or anything that isn't linked to libstdc++.so (including GCC-compiled code only linked to libsupc++.so)

[mclow]

I'd be happy to make libc++ provide a __throw_bad_alloc() call.

[kcc]

A solution that will not require us to change the build flags is more welcome of course. But it will need to work with all flavors of the C++ std lib (libc++ and libstdc++, old and new) w/o recompilation of asan run-time.

[mclow]

Actually, libc++ already has a std:: __throw_bad_alloc (and it is in std, not std:__1

[nico]

It'd be cool if there was a way to set allocator_may_return_null=1 only for new (std::nothrow). In Chromium, we like regular operator new to crash, but we'd like nothrow new to return 0. We currently disable at least one test in asan mode because it catches nothrow new that would return 0.

[kcc]

(sorry for delay... yes, I afraid we need to do that)

[alekseyshl]

Awfully belated update, everything requested (except throwing std::bad_alloc) is implemented as of July 2017.

[mclow]

everything requested (except throwing std::bad_alloc) is implemented as of July 2017.

That's encouraging. Just to avoid confusion, I read this as, under ASAN:

• operator new (nothrow) returns null on allocation failure.
• All flavors of operator new (and boy, there are a lot of them!) call the new_handler and retry the allocation when it fails. But
• operator new still does not throw bad_alloc upon allocation failure.

Is that correct?

[mclow]

(and thanks for the update!)

[alekseyshl]

Ah, the new_handler, indeed...

So, to recap. With allocator_may_return_null=0, allocator always crashes on failure, the rest assumes allocator_may_return_null=1

• operator new always crashes on failure
• operator new (nothrow) returns null
• all malloc/calloc/etc implementations follow their specifications, including setting errno (except dynamic runtime on Windows case)

TODO:

• make operator new to throw std::bad_alloc upon allocation failure
• call new_handler from operator new and retry the allocation

[CaseyCarter]

the only ones to complain are the libc++ developers.

And the libstdc++ maintainer.

And an MSVC STL maintainer.

[mclow]

Nice to see MSVC joining the party. :-)

[LebedevRI]

FWIW, not just C++ STL developers. This effectively makes [oss-]fuzzing code that may legitimately allocate large memory amounts generally impossible. (Can sanitizer be queried if it can allocate N more bytes? I don't think so?)