Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5929)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 7 months ago by samsonov
Modified:
11 years, 6 months ago
Reviewers:
kcc1, dvyukov
Base URL:
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/tsan/rtl/
Visibility:
Public.

Patch Set 1 : z #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set 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
kcc1
LGTM
11 years, 7 months ago (2012-09-24 11:18:30 UTC) #1
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 ...
11 years, 7 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 ...
11 years, 7 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 ...
11 years, 7 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 ...
11 years, 7 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 ...
11 years, 7 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, ...
11 years, 7 months ago (2012-09-25 07:42:42 UTC) #7
dvyukov
On Tue, Sep 25, 2012 at 12:42 AM, Alexey Samsonov <samsonov@google.com>wrote: > > > On ...
11 years, 7 months ago (2012-09-25 15:31:32 UTC) #8
samsonov
11 years, 7 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
> >
> >
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b