On Fri, Sep 8, 2017 at 12:05 AM, Milian Wolff [off-list ref] wrote:

On Thursday, September 7, 2017 4:58:53 PM CEST Namhyung Kim wrote:

quoted

Hi Milian,

Hey Namhyung!

quoted

quoted

quoted

quoted

quoted

@@ -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.

IIUC, ilist->symbol can point an existing symbol.  And all existing
symbols are freed before calling inline_node__delete().  I don't know
why valgrind or asan didn't catch anything.. maybe I'm missing
something.

-- 
Thanks,
Namhyung