DebugLeaks C++ expert issue?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

DebugLeaks C++ expert issue?

VTK - Dev mailing list

On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is complaining about vtkCommands being leaked because the two string literals in the below code have different addresses (the string address is used in a map type structure). Looking at DebugLeaks the only way this code would work is if they have the same address. But I checked in the debugger and they have different addresses.


//----------------------------------------------------------------
vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkCommand or subclass");
#endif
}

//----------------------------------------------------------------
void vtkCommand::UnRegister()
{
int refcount = this->GetReferenceCount()-1;
this->SetReferenceCount(refcount);
if (refcount <= 0)
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::DestructClass("vtkCommand or subclass");
#endif
delete this;
}
}

I can fix it by doing the following but wanted a c++ expert to weigh in, was the original code OK? Is this a compiler compliance issue? Is the proposed fix the right approach?

const char *cname = "vtkCommand or subclass";

//----------------------------------------------------------------
vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass(cname);
#endif
}

//----------------------------------------------------------------
void vtkCommand::UnRegister()
{
int refcount = this->GetReferenceCount()-1;
this->SetReferenceCount(refcount);
if (refcount <= 0)
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::DestructClass(cname);
#endif
delete this;
}
}


Thanks!
Ken


--
Ken Martin PhD
Distinguished Engineer
Kitware Inc.
101 East Weaver Street
Carrboro, North Carolina
27510 USA

This communication, including all attachments, contains confidential and legally privileged information, and it is intended only for the use of the addressee.  Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken in reliance on it is prohibited and may be unlawful. If you received this communication in error please notify us immediately and destroy the original message.  Thank you.

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: DebugLeaks C++ expert issue?

VTK - Dev mailing list
On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:

> On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> complaining about vtkCommands being leaked because the two string literals
> in the below code have different addresses (the string address is used in a
> map type structure). Looking at DebugLeaks the only way this code would
> work is if they have the same address. But I checked in the debugger and
> they have different addresses.
>
> <snip>
>
> I can fix it by doing the following but wanted a c++ expert to weigh in,
> was the original code OK? Is this a compiler compliance issue? Is the
> proposed fix the right approach?

I don't think the standard says anything about a string literal with the
same content as another literal having to share an address. I'd expect
that this is mainly optimization at the compiler or linker level. For
example, this:

    const char* foo = "foobar";
    const char* bar = "bar";

can be satisfied with this in the resulting binary:

    "foobar\0"
     ^  ^
     |  bar
     foo

but I'd expect the standard to say that doing `foo < bar` is always
undefined behavior since they're not pointers to the same object.

> const char *cname = "vtkCommand or subclass";
>
> //----------------------------------------------------------------
> vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> {
> #ifdef VTK_DEBUG_LEAKS
> vtkDebugLeaks::ConstructClass(cname);
> #endif
> }
>
> //----------------------------------------------------------------
> void vtkCommand::UnRegister()
> {
> int refcount = this->GetReferenceCount()-1;
> this->SetReferenceCount(refcount);
> if (refcount <= 0)
> {
> #ifdef VTK_DEBUG_LEAKS
> vtkDebugLeaks::DestructClass(cname);
> #endif
> delete this;
> }
> }

This looks reasonable to me. Maybe add `static`?

--Ben
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: DebugLeaks C++ expert issue?

VTK - Dev mailing list
Is it built with the /GF compiler flag?

https://docs.microsoft.com/en-us/cpp/build/reference/gf-eliminate-duplicate-strings?view=vs-2017


On Mon, Jan 7, 2019 at 11:59 AM Ben Boeckel via vtk-developers
<[hidden email]> wrote:

>
> On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:
> > On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> > complaining about vtkCommands being leaked because the two string literals
> > in the below code have different addresses (the string address is used in a
> > map type structure). Looking at DebugLeaks the only way this code would
> > work is if they have the same address. But I checked in the debugger and
> > they have different addresses.
> >
> > <snip>
> >
> > I can fix it by doing the following but wanted a c++ expert to weigh in,
> > was the original code OK? Is this a compiler compliance issue? Is the
> > proposed fix the right approach?
>
> I don't think the standard says anything about a string literal with the
> same content as another literal having to share an address. I'd expect
> that this is mainly optimization at the compiler or linker level. For
> example, this:
>
>     const char* foo = "foobar";
>     const char* bar = "bar";
>
> can be satisfied with this in the resulting binary:
>
>     "foobar\0"
>      ^  ^
>      |  bar
>      foo
>
> but I'd expect the standard to say that doing `foo < bar` is always
> undefined behavior since they're not pointers to the same object.
>
> > const char *cname = "vtkCommand or subclass";
> >
> > //----------------------------------------------------------------
> > vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> > {
> > #ifdef VTK_DEBUG_LEAKS
> > vtkDebugLeaks::ConstructClass(cname);
> > #endif
> > }
> >
> > //----------------------------------------------------------------
> > void vtkCommand::UnRegister()
> > {
> > int refcount = this->GetReferenceCount()-1;
> > this->SetReferenceCount(refcount);
> > if (refcount <= 0)
> > {
> > #ifdef VTK_DEBUG_LEAKS
> > vtkDebugLeaks::DestructClass(cname);
> > #endif
> > delete this;
> > }
> > }
>
> This looks reasonable to me. Maybe add `static`?
>
> --Ben
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> https://vtk.org/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: DebugLeaks C++ expert issue?

Elvis Stansvik
Den mån 7 jan. 2019 kl 20:41 skrev David Cole via vtk-developers
<[hidden email]>:
>
> Is it built with the /GF compiler flag?
>
> https://docs.microsoft.com/en-us/cpp/build/reference/gf-eliminate-duplicate-strings?view=vs-2017

(Also note that flag is in effect when /O1 or /O2 is used)

>
>
> On Mon, Jan 7, 2019 at 11:59 AM Ben Boeckel via vtk-developers
> <[hidden email]> wrote:
> >
> > On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:
> > > On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> > > complaining about vtkCommands being leaked because the two string literals
> > > in the below code have different addresses (the string address is used in a
> > > map type structure). Looking at DebugLeaks the only way this code would
> > > work is if they have the same address. But I checked in the debugger and
> > > they have different addresses.
> > >
> > > <snip>
> > >
> > > I can fix it by doing the following but wanted a c++ expert to weigh in,
> > > was the original code OK? Is this a compiler compliance issue? Is the
> > > proposed fix the right approach?
> >
> > I don't think the standard says anything about a string literal with the
> > same content as another literal having to share an address. I'd expect
> > that this is mainly optimization at the compiler or linker level. For
> > example, this:
> >
> >     const char* foo = "foobar";
> >     const char* bar = "bar";
> >
> > can be satisfied with this in the resulting binary:
> >
> >     "foobar\0"
> >      ^  ^
> >      |  bar
> >      foo
> >
> > but I'd expect the standard to say that doing `foo < bar` is always
> > undefined behavior since they're not pointers to the same object.
> >
> > > const char *cname = "vtkCommand or subclass";
> > >
> > > //----------------------------------------------------------------
> > > vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::ConstructClass(cname);
> > > #endif
> > > }
> > >
> > > //----------------------------------------------------------------
> > > void vtkCommand::UnRegister()
> > > {
> > > int refcount = this->GetReferenceCount()-1;
> > > this->SetReferenceCount(refcount);
> > > if (refcount <= 0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::DestructClass(cname);
> > > #endif
> > > delete this;
> > > }
> > > }
> >
> > This looks reasonable to me. Maybe add `static`?
> >
> > --Ben
> > _______________________________________________
> > Powered by www.kitware.com
> >
> > Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
> >
> > Search the list archives at: http://markmail.org/search/?q=vtk-developers
> >
> > Follow this link to subscribe/unsubscribe:
> > https://vtk.org/mailman/listinfo/vtk-developers
> >
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> https://vtk.org/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: DebugLeaks C++ expert issue?

VTK - Dev mailing list
    whether successive evaluations of a string-literal yield the same or a different object is unspecified  


On Mon, Jan 7, 2019 at 9:52 PM Elvis Stansvik <[hidden email]> wrote:
Den mån 7 jan. 2019 kl 20:41 skrev David Cole via vtk-developers
<[hidden email]>:
>
> Is it built with the /GF compiler flag?
>
> https://docs.microsoft.com/en-us/cpp/build/reference/gf-eliminate-duplicate-strings?view=vs-2017

(Also note that flag is in effect when /O1 or /O2 is used)

>
>
> On Mon, Jan 7, 2019 at 11:59 AM Ben Boeckel via vtk-developers
> <[hidden email]> wrote:
> >
> > On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:
> > > On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> > > complaining about vtkCommands being leaked because the two string literals
> > > in the below code have different addresses (the string address is used in a
> > > map type structure). Looking at DebugLeaks the only way this code would
> > > work is if they have the same address. But I checked in the debugger and
> > > they have different addresses.
> > >
> > > <snip>
> > >
> > > I can fix it by doing the following but wanted a c++ expert to weigh in,
> > > was the original code OK? Is this a compiler compliance issue? Is the
> > > proposed fix the right approach?
> >
> > I don't think the standard says anything about a string literal with the
> > same content as another literal having to share an address. I'd expect
> > that this is mainly optimization at the compiler or linker level. For
> > example, this:
> >
> >     const char* foo = "foobar";
> >     const char* bar = "bar";
> >
> > can be satisfied with this in the resulting binary:
> >
> >     "foobar\0"
> >      ^  ^
> >      |  bar
> >      foo
> >
> > but I'd expect the standard to say that doing `foo < bar` is always
> > undefined behavior since they're not pointers to the same object.
> >
> > > const char *cname = "vtkCommand or subclass";
> > >
> > > //----------------------------------------------------------------
> > > vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::ConstructClass(cname);
> > > #endif
> > > }
> > >
> > > //----------------------------------------------------------------
> > > void vtkCommand::UnRegister()
> > > {
> > > int refcount = this->GetReferenceCount()-1;
> > > this->SetReferenceCount(refcount);
> > > if (refcount <= 0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::DestructClass(cname);
> > > #endif
> > > delete this;
> > > }
> > > }
> >
> > This looks reasonable to me. Maybe add `static`?
> >
> > --Ben
> > _______________________________________________
> > Powered by www.kitware.com
> >
> > Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
> >
> > Search the list archives at: http://markmail.org/search/?q=vtk-developers
> >
> > Follow this link to subscribe/unsubscribe:
> > https://vtk.org/mailman/listinfo/vtk-developers
> >
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> https://vtk.org/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: DebugLeaks C++ expert issue?

VTK - Dev mailing list
Yes, the code has been there for ages so it seems that 99.9% of the time the two string literals share storage. I'm not sure if it is a cmake change or a vs2017 change (I updated both) that caused the issue to pop up. It is a debug build with DEBUG_LEAKS turned on. But forcing them to share the same address definitely solved the problem and seems that safe approach from a standards point of view.

On Tue, Jan 8, 2019 at 3:25 AM Julien Finet <[hidden email]> wrote:
    whether successive evaluations of a string-literal yield the same or a different object is unspecified  


On Mon, Jan 7, 2019 at 9:52 PM Elvis Stansvik <[hidden email]> wrote:
Den mån 7 jan. 2019 kl 20:41 skrev David Cole via vtk-developers
<[hidden email]>:
>
> Is it built with the /GF compiler flag?
>
> https://docs.microsoft.com/en-us/cpp/build/reference/gf-eliminate-duplicate-strings?view=vs-2017

(Also note that flag is in effect when /O1 or /O2 is used)

>
>
> On Mon, Jan 7, 2019 at 11:59 AM Ben Boeckel via vtk-developers
> <[hidden email]> wrote:
> >
> > On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:
> > > On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> > > complaining about vtkCommands being leaked because the two string literals
> > > in the below code have different addresses (the string address is used in a
> > > map type structure). Looking at DebugLeaks the only way this code would
> > > work is if they have the same address. But I checked in the debugger and
> > > they have different addresses.
> > >
> > > <snip>
> > >
> > > I can fix it by doing the following but wanted a c++ expert to weigh in,
> > > was the original code OK? Is this a compiler compliance issue? Is the
> > > proposed fix the right approach?
> >
> > I don't think the standard says anything about a string literal with the
> > same content as another literal having to share an address. I'd expect
> > that this is mainly optimization at the compiler or linker level. For
> > example, this:
> >
> >     const char* foo = "foobar";
> >     const char* bar = "bar";
> >
> > can be satisfied with this in the resulting binary:
> >
> >     "foobar\0"
> >      ^  ^
> >      |  bar
> >      foo
> >
> > but I'd expect the standard to say that doing `foo < bar` is always
> > undefined behavior since they're not pointers to the same object.
> >
> > > const char *cname = "vtkCommand or subclass";
> > >
> > > //----------------------------------------------------------------
> > > vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::ConstructClass(cname);
> > > #endif
> > > }
> > >
> > > //----------------------------------------------------------------
> > > void vtkCommand::UnRegister()
> > > {
> > > int refcount = this->GetReferenceCount()-1;
> > > this->SetReferenceCount(refcount);
> > > if (refcount <= 0)
> > > {
> > > #ifdef VTK_DEBUG_LEAKS
> > > vtkDebugLeaks::DestructClass(cname);
> > > #endif
> > > delete this;
> > > }
> > > }
> >
> > This looks reasonable to me. Maybe add `static`?
> >
> > --Ben
> > _______________________________________________
> > Powered by www.kitware.com
> >
> > Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
> >
> > Search the list archives at: http://markmail.org/search/?q=vtk-developers
> >
> > Follow this link to subscribe/unsubscribe:
> > https://vtk.org/mailman/listinfo/vtk-developers
> >
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> https://vtk.org/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers



--
Ken Martin PhD
Distinguished Engineer
Kitware Inc.
101 East Weaver Street
Carrboro, North Carolina
27510 USA

This communication, including all attachments, contains confidential and legally privileged information, and it is intended only for the use of the addressee.  Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken in reliance on it is prohibited and may be unlawful. If you received this communication in error please notify us immediately and destroy the original message.  Thank you.

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers