lkml.org 
[lkml]   [2017]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 03/14] perf report: create real callchain entries for inlined frames
Date
On Thursday, September 7, 2017 4:58:53 PM CEST Namhyung Kim wrote:
> Hi Milian,

Hey Namhyung!

> > > > > @@ -511,10 +563,63 @@ void inline_node__delete(struct inline_node
> > > > > *node)
> > > > >
> > > > > list_for_each_entry_safe(ilist, tmp, &node->val, list) {
> > > > >
> > > > > list_del_init(&ilist->list);
> > > > >
> > > > > - zfree(&ilist->filename);
> > > > > - zfree(&ilist->funcname);
> > > > > + zfree(&ilist->srcline);
> > > > > + // only the inlined symbols are owned by the list
> > > > > + if (ilist->symbol && ilist->symbol->inlined)
> > > > > + symbol__delete(ilist->symbol);
> > > >
> > > > Existing symbols are released at this moment.
> > >
> > > Thanks for the review, I'll try to look into these issues once I have
> > > more
> > > time again.
> >
> > OK, so I just dug into this part of the patch again. I don't think it's
> > actually a problem after all:
> >
> > When an inline node reuses the real symbol, that symbol won't have its
> > `inlined` member set to true. Thus these symbols will never get deleted by
> > inline_node__delete.
>
> But ilist->symbol is a dangling pointer so accessing ->inlined would
> be a problem, no?

Sorry, but I can't follow. Why would it be a dangling pointer? Note, again,
that I've tested this with both valgrind and ASAN and neither reports any
issues about this code.

> > If you have suggestions on how to make this clearer, I'm
> > all ears. For now, I'll add a comment to where we alias/reuse the symbol.
> >
> > I'll try to split up the patch now to make it somehow easier to review.
>
> Thanks for doing this, but I'm afraid I don't have time to review
> before going to OSS-NA.

No problem, enjoy!
--
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2017-09-07 17:06    [W:0.103 / U:1.604 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site