Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755415AbdIGPQ5 (ORCPT ); Thu, 7 Sep 2017 11:16:57 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:37249 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753853AbdIGPQ4 (ORCPT ); Thu, 7 Sep 2017 11:16:56 -0400 X-Google-Smtp-Source: ADKCNb5UVc+SGyC5OFZhz8QJyee5oVuB611AxLvf71Vxt2kvVMlsZbGZwzqobW7oG8iGfII098XSZ1qP3BNdm6deqAc= MIME-Version: 1.0 In-Reply-To: <3394979.u4vPUxEDmB@milian-kdab2> References: <20170806212446.24925-1-milian.wolff@kdab.com> <1935427.QE1zWV8lNS@agathebauer> <20170907145853.GA5764@danjae.aot.lge.com> <3394979.u4vPUxEDmB@milian-kdab2> From: Namhyung Kim Date: Fri, 8 Sep 2017 00:16:34 +0900 X-Google-Sender-Auth: 7tO2RVT8-IM0kwopf1ugOyX2gmk Message-ID: Subject: Re: [PATCH v2 03/14] perf report: create real callchain entries for inlined frames To: Milian Wolff Cc: Arnaldo Carvalho de Melo , Jin Yao , "linux-kernel@vger.kernel.org" , linux-perf-users , Arnaldo Carvalho de Melo , David Ahern , Peter Zijlstra , kernel-team@lge.com Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1790 Lines: 48 On Fri, Sep 8, 2017 at 12:05 AM, Milian Wolff wrote: > 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. 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