Created attachment 10203 [details] Example program - taken from libc++ test suite After calling set_new_handler(), if I attempt to allocate memory, and that fails, I expect to have my new handler called. This works fine w/o ASan. But with ASan, the new handler is not called.
Adding kcc@....
Before fixing this, I'd like to understand why anyone would care about this. Does someone really rely on this working with asan, or this is "nice to have in order to pass all and every C++ standard tests" The docs for set_new_handler say: -------------- The new-handler function may try to make more storage available for a new attempt to allocate the storage. If -and only if- the function succeeds in making more storage available, it may return. Otherwise it shall either throw a bad_alloc exception (or a derived class) or terminate the program (such as by calling abort or exit). ------------- There is no way for the asan allocator to "make more storage available for a new attempt" so the custom new_handler will have to abort or throw. asan's operator new does not throw bad_alloc, it just dies. Since asan run-time is build w/o exceptions, it can not throw from operator new.
The failing libc++ test was definitely not written to expect a user-supplied operator new. It is looking for the behavior required for the default operator new: loop on failure, calling new_handler or if new_handler is null, throw bad_alloc: void * operator new(std::size_t size) #if !__has_feature(cxx_noexcept) throw(std::bad_alloc) #endif { if (size == 0) size = 1; void* p; while ((p = std::malloc(size)) == 0) { std::new_handler nh = std::get_new_handler(); if (nh) nh(); else throw std::bad_alloc(); } return p; } This looping, or even calling the new handler isn't required by a replaced operator new, though the replaced operator new is required to throw bad_alloc on failure, which asan doesn't do. This may not be an important test for asan to pass, I'm not positive at the moment. This test is: libcxx/test/language.support/support.dynamic/new.delete/new.delete.single/new.pass.cpp Also failing are: new.delete/new.delete.array/new_array.pass.cpp new.delete/new.delete.array/new_array_nothrow_replace.pass.cpp new.delete/new.delete.array/new_array_replace.pass.cpp The new_array.pass.cpp is similar to new.pass.cpp: It is expecting the default operator new's behavior of calling the new_handler and throwing a bad_alloc when the new_handler is null. The latter two may be more problematic. These are testing the ability to replace operator new[] by simply replacing operator new (see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2158.html). Two questions: 1. What is the motivation for the asan allocator to not throw bad_alloc on failure? 2. What is asan's intended strategy for when the client overloads operator new?
(In reply to comment #2) > asan's operator new does not throw bad_alloc, it just dies. That is not what I am seeing. What I am observing is that a message of the form: "==57431==WARNING: AddressSanitizer failed to allocate 0xffffffffffffffff bytes" gets written to the console, and the call to new returns NULL.
Created attachment 10208 [details] Not-quite-ready patch I've attached a patch that does everything I think needs to be done, except for enabling -fexceptions for asan_new_delete.cc, which I do not know how to do. This patch only impacts SANITIZER_MAC. On this platform, due to conformance to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2158.html, asan should only overload operator new, and not the other 3 variants. The default implementation of the other 3 variants forward to operator new, whether overridden or not. Just removing these 3 overridden operators new/delete fixes 3 of the libc++ testsuite failures, and looses no asan functionality whatsoever. The rest of the patch is about trying to make the one remaining overridden operator new both conforming (throw bad_alloc on failure), and a good emulator of the default new (call new_handler in a loop). If asan absolutely can not throw a bad_alloc from operator new, I see no way to make it C++ conforming, and the one remaining libc++ test suite failure can simply be known to fail. Fwiw, here is the Required behavior of every overridden operator new: Required behavior: Return a non-null pointer to suitably aligned storage (3.7.4), or else throw a bad_alloc exception. This requirement is binding on a replacement version of this function.
> What I am observing is that a message of the form: > "==57431==WARNING: AddressSanitizer failed to allocate 0xffffffffffffffff > bytes" gets written to the console, and the call to new returns NULL. Ah, my bad. Indeed, if the size is completely insane, the allocator prints a warning and returns 0 instead of crashing. I think it should actually crash instead. But if the allocator is given some sane size which it can not allocate due to out-of-memory > Two questions: > > 1. What is the motivation for the asan allocator to not throw bad_alloc on > failure? The main motivation is that it's not that trivial to implement (will require some extra code to write, test and maintain) and no one yet asked. You are the first. As I just mentioned, there are cases when the allocator will simply die due to OOM. We'll need to replace them with if (bad_thing_happened) return something; since we can not use exceptions in the guts of the allocator. > > 2. What is asan's intended strategy for when the client overloads operator > new? This is not currently supported (e.g. I had to edit one of the SPEC benchmarks to remove their custom operator new). This has not been a real problem before, so it did not get any attention. I am not against supporting this, just don't see how to make it easily (and, preferably, for all platforms)
(In reply to comment #5) > Created attachment 10208 [details] > Not-quite-ready patch > > I've attached a patch that does everything I think needs to be done, except > for enabling -fexceptions for asan_new_delete.cc, which I do not know how to > do. Thanks! I am not an Apple expert, glider@ is better to see this. The patch will need test(s). And anyway, please send it to llvm-dev > > This patch only impacts SANITIZER_MAC. > > On this platform, due to conformance to > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2158.html, asan > should only overload operator new, and not the other 3 variants. The > default implementation of the other 3 variants forward to operator new, > whether overridden or not. Just removing these 3 overridden operators > new/delete fixes 3 of the libc++ testsuite failures, and looses no asan > functionality whatsoever. > > The rest of the patch is about trying to make the one remaining overridden > operator new both conforming (throw bad_alloc on failure), and a good > emulator of the default new (call new_handler in a loop). > > If asan absolutely can not throw a bad_alloc from operator new, I see no way > to make it C++ conforming, and the one remaining libc++ test suite failure > can simply be known to fail. we can make it throw on insane sizes (where currently the allocator returns 0) This will require some make/cmake machinery to allow exceptions in a single module. BTW, I'd suggest to have asan_new_delete_mac.cc asan_new_delete_linux.cc asan_new_delete_android.cc # maybe not, as we don't do it on Android at all. asan_new_delete_windows.cc # Timur? instead of the ifdef clatter. One of the possible ways of supporting user-defined operator new is to link asan_new_delete_xxx as a separate library. Again, quite a bit of work to make it happen, but doable. It may be harder to make it throw on out-of-memory condition, where currently the allocator dies. Or maybe I am mistaken. Can we unwind through the code that is build with "-fno-exceptions -funwind-tables" ? If we can, it will be much simpler. > > Fwiw, here is the Required behavior of every overridden operator new: > > Required behavior: Return a non-null pointer to suitably aligned storage > (3.7.4), or else throw a bad_alloc exception. This requirement is binding on > a replacement version of this function.
The current ASan implementation of custom new()/delete() does not work reliably on Mac (it may occasionally, but that's fake safety). Because the ASan runtime library is a DSO, we end up having the main executable depend on two DSOs (libstdc++ and libclang_rt.asan_osx_dynamic) each having their own new/delete implementation. It's not determined which of them is picked at startup (I saw both versions already). Until we decide how to make ASan reliably intercept new/delete on OS X we just need to disable our custom interceptors - that should fix the new_handler issue.
Another reason for having new/delete interceptors is to implement checks for delete() size for the -fsized-deallocation extension (see LLVM r214296, http://llvm.org/viewvc/llvm-project?view=revision&revision=214296)