[llvm-commits] [ASan] remove <new> header (issue 5966077)

eugenis at google.com eugenis at google.com
Thu Apr 5 09:49:52 PDT 2012


On 2012/04/05 16:38:49, samsonov wrote:
> On 2012/04/05 16:25:08, kcc1 wrote:
> > Oh, I mean that asan_new_delete.cc should be a part of asan rtl.
> > Isn't that enough?

> No, if ASan RTL doesn't depend on anything in asan_new_delete.cc.
> asan_new_delete.o will be archived into libasan .a but will not be
> statically linked into the exe file which contains a call to "new"
> (if we're unlucky, e.g. if libstdc++ is linked earlier).

Right. This file will be dropped from the final link if it does not
define at least one otherwise unresolved symbol.

I think -Wl,-whole-archive is not this bad. It can be added by the
driver to the part of the command line we fully control (near the end of
it).

On the other hand, since we already have this if (0) stuff that attempt
to glue the whole runtime together, we could just add a call to new() or
some other function from the new file there.


> >
> > On Thu, Apr 5, 2012 at 8:46 AM, <mailto:samsonov at google.com> wrote:
> >
> > > On 2012/04/04 14:57:48, kcc1 wrote:
> > >
> > >> I'd prefer to move our custom new/delete into asan_new_delete.cc,
and
> > >>
> > > in
> > >
> > >> that file it would probably be ok to include <new>.
> > >>
> > >
> > > Yep, that's reasonable, but we have to make sure that resulting
> > > asan_new_delete.o will be loaded from libasan .a file
> > > (otherwise our "new" implementation is just not linked into exe,
and
> > > dynamic linker chooses libstdc++ implementation).
> > >
> > > We can
> > > 1) add a fake dependency between asan_rtl and asan_new_delete (in
this
> > > CL, a bit hacky)
> > > 2) hack lib/Driver/Tools.cpp and make sure that flag -lstdc++ is
passed
> > > to ld after .a with ASan RT (it is currently passed before).
(looks more
> > > hacky)
> > > 3) use -Wl,--whole-archive (omg).
> > >
> > >
> > >  --kcc
> > >>
> > >
> > >  On Wed, Apr 4, 2012 at 7:53 AM, <mailto:samsonov at google.com>
wrote:
> > >>
> > >
> > >  > Reviewers: kcc1, timurrrr_at_google_com,
> > >> >
> > >> > Message:
> > >> > #include <new> brings a number of system libraries (e.g.
<string.h>
> > >>
> > > on
> > >
> > >> > Windows), which is probably unwanted. We (hopefully) can
replace
> > >> > std::nothrow_t and std::bad_alloc by custom stubs, so that our
> > >>
> > > overload
> > >
> > >> > of new would still work in a user program.
> > >> >
> > >> >
> > >> >
> > >> > Please review this at
> > >>
> > >
> > > http://codereview.appspot.com/****5966077/%25253Chttp://**
> > >
> >

codereview.appspot.com/**5966077/<http://codereview.appspot.com/**5966077/%3Chttp://codereview.appspot.com/5966077/>
> > > >
> > >
> > >  >
> > >> > Affected files:
> > >> >  M     asan_interceptors.cc
> > >> >
> > >> >
> > >> > Index: asan_interceptors.cc
> > >> >
> > >>
> > > ==============================****============================**
> > > ==**=======
> > >
> > >  > --- asan_interceptors.cc        (revision 154006)
> > >> > +++ asan_interceptors.cc        (working copy)
> > >> > @@ -22,8 +22,6 @@
> > >> >  #include "asan_thread_registry.h"
> > >> >  #include "interception/interception.h"
> > >> >
> > >> > -#include <new>
> > >> > -
> > >> >  // Use macro to describe if specific function should be
> > >> >  // intercepted on a given platform.
> > >> >  #if !defined(_WIN32)
> > >> > @@ -331,6 +329,10 @@
> > >> >  void *operator new(size_t size) { OPERATOR_NEW_BODY; }
> > >> >  void *operator new[](size_t size) { OPERATOR_NEW_BODY; }
> > >> >  #else
> > >> > +namespace std {
> > >> > +class bad_alloc {};
> > >> > +class nothrow_t;
> > >> > +}  // namespace std
> > >> >  void *operator new(size_t size) throw(std::bad_alloc) {
> > >> > OPERATOR_NEW_BODY; }
> > >> >  void *operator new[](size_t size) throw(std::bad_alloc) {
> > >> > OPERATOR_NEW_BODY; }
> > >> >  void *operator new(size_t size, std::nothrow_t const&) throw()
> > >> >
> > >> >
> > >> >
> > >>
> > >
> > >
> > >
> > >
> >

http://codereview.appspot.com/**5966077/%253Chttp://codereview.appspot.com/5966077/>
> > >



http://codereview.appspot.com/5966077/



More information about the llvm-commits mailing list