Issue 5966077: [ASan] remove <new> header (Closed)
|
Created:
12 years ago by samsonov Modified:
12 years ago CC:
llvm-commits_cs.uiuc.edu Base URL:
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/asan/ Visibility:
Public. |
Patch Set 1 : z #Patch Set 2 : Move new/delete to other file. #
MessagesTotal messages: 9
#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.
Sign in to reply to this message.
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>. --kcc On Wed, Apr 4, 2012 at 7:53 AM, <samsonov@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/<http://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() > > >
Sign in to reply to this message.
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@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/%3Chttp://codereview.appspot.com/5966...> > > > > 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() > > > > > >
Sign in to reply to this message.
Oh, I mean that asan_new_delete.cc should be a part of asan rtl. Isn't that enough? On Thu, Apr 5, 2012 at 8:46 AM, <samsonov@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@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/%3Chttp://** > 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/<http://codereview.appspot.com/5966077/> >
Sign in to reply to this message.
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). > > On Thu, Apr 5, 2012 at 8:46 AM, <mailto:samsonov@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@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/%253Chttp://** > > > 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/%3Chttp://codereview.appspot.com/5966...> > >
Sign in to reply to this message.
On Thu, Apr 5, 2012 at 9:38 AM, <samsonov@google.com> 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. > We know how to fix it. Juts make an empty InitNewAndDelete() and call it from another file. > 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). > > > > On Thu, Apr 5, 2012 at 8:46 AM, <mailto:samsonov@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@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/%253Chttp://**<http://codereview.... >> > >> > > codereview.appspot.com/****5966077/<http://codereview.appspot.com/**5966077/> > <http://codereview.**appspot.com/**5966077/%3Chttp:** > //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/%3Chttp://** > codereview.appspot.com/**5966077/<http://codereview.appspot.com/**5966077/%3Chttp://codereview.appspot.com/5966077/> > > > >> > >> > > > > http://codereview.appspot.com/**5966077/<http://codereview.appspot.com/5966077/> >
Sign in to reply to this message.
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@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@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/59...> > > >
Sign in to reply to this message.
On Thu, Apr 5, 2012 at 8:48 PM, Kostya Serebryany <kcc@google.com> wrote: > > > On Thu, Apr 5, 2012 at 9:38 AM, <samsonov@google.com> 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. >> > > We know how to fix it. > Juts make an empty InitNewAndDelete() and call it from another file. > It's in the patch already. > > >> 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). >> >> >> >> On Thu, Apr 5, 2012 at 8:46 AM, <mailto:samsonov@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@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/%253Chttp://**<http://codereview.... >>> > >>> >> >> codereview.appspot.com/****5966077/<http://codereview.appspot.com/**5966077/> >> <http://codereview.**appspot.com/**5966077/%3Chttp:** >> //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/%3Chttp://** >> codereview.appspot.com/**5966077/<http://codereview.appspot.com/**5966077/%3Chttp://codereview.appspot.com/5966077/> >> > >> >>> > >>> >> >> >> >> http://codereview.appspot.com/**5966077/<http://codereview.appspot.com/5966077/> >> > > -- Alexey Samsonov, MSK
Sign in to reply to this message.
Looks good now (assuming the tests pass). Thanks! --kcc On Thu, Apr 5, 2012 at 10:10 AM, Alexey Samsonov <samsonov@google.com>wrote: > > > On Thu, Apr 5, 2012 at 8:48 PM, Kostya Serebryany <kcc@google.com> wrote: > >> >> >> On Thu, Apr 5, 2012 at 9:38 AM, <samsonov@google.com> 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. >>> >> >> We know how to fix it. >> Juts make an empty InitNewAndDelete() and call it from another file. >> > > It's in the patch already. > > >> >> >>> 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). >>> >>> >>> >>> On Thu, Apr 5, 2012 at 8:46 AM, <mailto:samsonov@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@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/%253Chttp://**<http://codereview.... >>>> > >>>> >>> >>> codereview.appspot.com/****5966077/<http://codereview.appspot.com/**5966077/> >>> <http://codereview.**appspot.com/**5966077/%3Chttp:** >>> //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/%3Chttp://** >>> codereview.appspot.com/**5966077/<http://codereview.appspot.com/**5966077/%3Chttp://codereview.appspot.com/5966077/> >>> > >>> >>>> > >>>> >>> >>> >>> >>> http://codereview.appspot.com/**5966077/<http://codereview.appspot.com/5966077/> >>> >> >> > > > -- > Alexey Samsonov, MSK > >
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
