Issue 6554077: [TSan] new/delete operators

Issue 6554077: [TSan] new/delete operators (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 4 months ago by samsonov

Modified:
12 years, 3 months ago

Reviewers:
kcc1, dvyukov

Base URL:
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/tsan/rtl/

Visibility:
Public.

More Reviews

Patch Set 1 : z #

Total comments: 1

Created: 12 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+181 lines, -178 lines)			Patch
M	CMakeLists.txt	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	Makefile.old	View	1 chunk	+2 lines, -0 lines	0 comments	Download
A	tsan_interceptors.h	View	1 chunk	+54 lines, -0 lines	0 comments	Download
M	tsan_interceptors.cc	View	6 chunks	+23 lines, -178 lines	0 comments	Download
M	tsan_mman.h	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	tsan_mman.cc	View	2 chunks	+26 lines, -0 lines	0 comments	Download
A	tsan_new_delete.cc	View	1 chunk	+70 lines, -0 lines	1 comment	Download
M	tsan_rtl.h	View	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 9

Expand All Messages | Collapse All Messages

dvyukov

https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc File tsan_new_delete.cc (right): https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc#newcode40 tsan_new_delete.cc:40: void *operator new(size_t size) throw(std::bad_alloc) { Can't we just ...

12 years, 4 months ago (2012-09-25 05:13:11 UTC) #2

kcc1

No, because we need to include operator <new> and we don't wont to polute the ...

12 years, 4 months ago (2012-09-25 05:20:33 UTC) #3

dvyukov

We need to include zillions of other include files for other things in tsan_interceptors.cc, but ...

12 years, 4 months ago (2012-09-25 05:23:55 UTC) #4

samsonov

It's difficult to use stuff like std::bad_alloc with extern "C" machinery we utilize for other ...

12 years, 4 months ago (2012-09-25 05:36:38 UTC) #5

dvyukov

On Mon, Sep 24, 2012 at 10:36 PM, Alexey Samsonov <samsonov@google.com>wrote: > It's difficult to ...

12 years, 4 months ago (2012-09-25 05:47:23 UTC) #6

samsonov

On Tue, Sep 25, 2012 at 9:47 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Mon, ...

12 years, 4 months ago (2012-09-25 07:42:42 UTC) #7

On Tue, Sep 25, 2012 at 9:47 AM, Dmitry Vyukov <dvyukov@google.com> wrote:

> On Mon, Sep 24, 2012 at 10:36 PM, Alexey Samsonov <samsonov@google.com>wrote:
>
>> It's difficult to use stuff like std::bad_alloc with extern "C" machinery
>> we utilize for other functions from pthread etc.
>
>
> What do you mean?
>

I mean it's more difficult to declare operators new/delete w/o headers, as
extern "C" functions.
(this is probably why you had to use _Znam etc).


>
>
>
>> I thought we may need to pull some stuff from tsan_interceptors.cc to
>> header anyway (just a bit later) - if we ever port
>> TSan to other platforms.
>>
>> On Tue, Sep 25, 2012 at 9:23 AM, Dmitry Vyukov <dvyukov@google.com>wrote:
>>
>>> We need to include zillions of other include files for other things in
tsan_interceptors.cc,
>>> but we just do not. We somehow live w/o the includes.
>>>
>>>
>>>
>>> On Mon, Sep 24, 2012 at 10:20 PM, Kostya Serebryany <kcc@google.com>wrote:
>>>
>>>> No, because we need to include operator <new> and we don't wont to
>>>> polute the large file with it.
>>>>
>>>>
>>>> On Tue, Sep 25, 2012 at 9:13 AM, <dvyukov@google.com> wrote:
>>>>
>>>>>
>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
>>>>>
new_delete.cc<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc>
>>>>> File tsan_new_delete.cc (right):
>>>>>
>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
>>>>>
new_delete.cc#newcode40<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc#newcode40>
>>>>> tsan_new_delete.cc:40: void *operator new(size_t size)
>>>>> throw(std::bad_alloc) {
>>>>> Can't we just put all these into tsan_interceptors.cc w/o the massive
>>>>> changes?
>>>>>
>>>>>
https://codereview.appspot.**com/6554077/<https://codereview.appspot.com/6554...
>>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> Alexey Samsonov, MSK
>>
>>
>


-- 
Alexey Samsonov, MSK

dvyukov

On Tue, Sep 25, 2012 at 12:42 AM, Alexey Samsonov <samsonov@google.com>wrote: > > > On ...

12 years, 4 months ago (2012-09-25 15:31:32 UTC) #8

On Tue, Sep 25, 2012 at 12:42 AM, Alexey Samsonov <samsonov@google.com>wrote:

>
>
> On Tue, Sep 25, 2012 at 9:47 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>
>> On Mon, Sep 24, 2012 at 10:36 PM, Alexey Samsonov
<samsonov@google.com>wrote:
>>
>>> It's difficult to use stuff like std::bad_alloc with extern "C"
>>> machinery we utilize for other functions from pthread etc.
>>
>>
>> What do you mean?
>>
>
> I mean it's more difficult to declare operators new/delete w/o headers, as
> extern "C" functions.
>


No, it is absolutely not.
You may just copy-paste that into tsan_interceptors.cc

$ cat test.cc
extern "C" int printf(char const*, ...);
extern "C" void* malloc(unsigned long);

void* operator new(unsigned long sz)
{
printf("new(%zu)\n", sz);
return malloc(sz);
}

namespace std
{
struct nothrow_t {};
}

void* operator new(unsigned long sz, std::nothrow_t const&)
{
printf("new_nothrow(%zu)\n", sz);
return malloc(sz);
}

int main()
{
extern void foo();
foo();
}

$ cat test1.cc
#include <new>

void foo()
{
new int;
new(std::nothrow) int;
}

$ g++ test.cc test1.cc -Wall && ./a.out
new(4)
new_nothrow(4)





> (this is probably why you had to use _Znam etc).
>
>
>>
>>
>>
>>> I thought we may need to pull some stuff from tsan_interceptors.cc to
>>> header anyway (just a bit later) - if we ever port
>>> TSan to other platforms.
>>>
>>> On Tue, Sep 25, 2012 at 9:23 AM, Dmitry Vyukov <dvyukov@google.com>wrote:
>>>
>>>> We need to include zillions of other include files for other things in
tsan_interceptors.cc,
>>>> but we just do not. We somehow live w/o the includes.
>>>>
>>>>
>>>>
>>>> On Mon, Sep 24, 2012 at 10:20 PM, Kostya Serebryany <kcc@google.com>wrote:
>>>>
>>>>> No, because we need to include operator <new> and we don't wont to
>>>>> polute the large file with it.
>>>>>
>>>>>
>>>>> On Tue, Sep 25, 2012 at 9:13 AM, <dvyukov@google.com> wrote:
>>>>>
>>>>>>
>>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
>>>>>>
new_delete.cc<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc>
>>>>>> File tsan_new_delete.cc (right):
>>>>>>
>>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
>>>>>>
new_delete.cc#newcode40<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc#newcode40>
>>>>>> tsan_new_delete.cc:40: void *operator new(size_t size)
>>>>>> throw(std::bad_alloc) {
>>>>>> Can't we just put all these into tsan_interceptors.cc w/o the massive
>>>>>> changes?
>>>>>>
>>>>>>
https://codereview.appspot.**com/6554077/<https://codereview.appspot.com/6554...
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Alexey Samsonov, MSK
>>>
>>>
>>
>
>
> --
> Alexey Samsonov, MSK
>
>

samsonov

12 years, 4 months ago (2012-09-27 09:52:48 UTC) #9

On 2012/09/25 15:31:32, dvyukov wrote:
> On Tue, Sep 25, 2012 at 12:42 AM, Alexey Samsonov <samsonov@google.com>wrote:
> 
> >
> >
> > On Tue, Sep 25, 2012 at 9:47 AM, Dmitry Vyukov <mailto:dvyukov@google.com>
wrote:
> >
> >> On Mon, Sep 24, 2012 at 10:36 PM, Alexey Samsonov
> <samsonov@google.com>wrote:
> >>
> >>> It's difficult to use stuff like std::bad_alloc with extern "C"
> >>> machinery we utilize for other functions from pthread etc.
> >>
> >>
> >> What do you mean?
> >>
> >
> > I mean it's more difficult to declare operators new/delete w/o headers, as
> > extern "C" functions.
> >
> 
> 
> No, it is absolutely not.
> You may just copy-paste that into tsan_interceptors.cc

Alright. r164764.


> 
> $ cat test.cc
> extern "C" int printf(char const*, ...);
> extern "C" void* malloc(unsigned long);
> 
> void* operator new(unsigned long sz)
> {
> printf("new(%zu)\n", sz);
> return malloc(sz);
> }
> 
> namespace std
> {
> struct nothrow_t {};
> }
> 
> void* operator new(unsigned long sz, std::nothrow_t const&)
> {
> printf("new_nothrow(%zu)\n", sz);
> return malloc(sz);
> }
> 
> int main()
> {
> extern void foo();
> foo();
> }
> 
> $ cat test1.cc
> #include <new>
> 
> void foo()
> {
> new int;
> new(std::nothrow) int;
> }
> 
> $ g++ test.cc test1.cc -Wall && ./a.out
> new(4)
> new_nothrow(4)
> 
> 
> 
> 
> 
> > (this is probably why you had to use _Znam etc).
> >
> >
> >>
> >>
> >>
> >>> I thought we may need to pull some stuff from tsan_interceptors.cc to
> >>> header anyway (just a bit later) - if we ever port
> >>> TSan to other platforms.
> >>>
> >>> On Tue, Sep 25, 2012 at 9:23 AM, Dmitry Vyukov <dvyukov@google.com>wrote:
> >>>
> >>>> We need to include zillions of other include files for other things in
> tsan_interceptors.cc,
> >>>> but we just do not. We somehow live w/o the includes.
> >>>>
> >>>>
> >>>>
> >>>> On Mon, Sep 24, 2012 at 10:20 PM, Kostya Serebryany
<kcc@google.com>wrote:
> >>>>
> >>>>> No, because we need to include operator <new> and we don't wont to
> >>>>> polute the large file with it.
> >>>>>
> >>>>>
> >>>>> On Tue, Sep 25, 2012 at 9:13 AM, <mailto:dvyukov@google.com> wrote:
> >>>>>
> >>>>>>
> >>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
> >>>>>>
>
new_delete.cc<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc>
> >>>>>> File tsan_new_delete.cc (right):
> >>>>>>
> >>>>>> https://codereview.appspot.**com/6554077/diff/4001/tsan_**
> >>>>>>
>
new_delete.cc#newcode40<https://codereview.appspot.com/6554077/diff/4001/tsan_new_delete.cc#newcode40>
> >>>>>> tsan_new_delete.cc:40: void *operator new(size_t size)
> >>>>>> throw(std::bad_alloc) {
> >>>>>> Can't we just put all these into tsan_interceptors.cc w/o the massive
> >>>>>> changes?
> >>>>>>
> >>>>>>
>
https://codereview.appspot.**com/6554077/%3Chttps://codereview.appspot.com/65...>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> Alexey Samsonov, MSK
> >>>
> >>>
> >>
> >
> >
> > --
> > Alexey Samsonov, MSK
> >
> >

Expand All Messages | Collapse All Messages