Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757515AbbKSDMn (ORCPT ); Wed, 18 Nov 2015 22:12:43 -0500 Received: from mail7.hitachi.co.jp ([133.145.228.42]:48545 "EHLO mail7.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751600AbbKSDMl (ORCPT ); Wed, 18 Nov 2015 22:12:41 -0500 From: =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= To: "'Namhyung Kim'" , Arnaldo Carvalho de Melo CC: Peter Zijlstra , "linux-kernel@vger.kernel.org" , Adrian Hunter , "Ingo Molnar" , Namhyung Kim , Jiri Olsa Subject: RE: [PATCH perf/core 01/13] perf probe: Fix to free temporal Dwarf_Frame Thread-Topic: [PATCH perf/core 01/13] perf probe: Fix to free temporal Dwarf_Frame Thread-Index: AQHRIczz0S3JbR0tl0yW+sBju31qa56hyLSAgAAPjYCAALJOQA== Date: Thu, 19 Nov 2015 03:12:37 +0000 Message-ID: <50399556C9727B4D88A595C8584AAB3752624A21@GSjpTKYDCembx32.service.hitachi.net> References: <20151118064009.30709.74354.stgit@localhost.localdomain> <20151118064011.30709.65674.stgit@localhost.localdomain> <20151118223639.GB22729@kernel.org> In-Reply-To: Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.198.220.53] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id tAJ3DRs3022829 Content-Length: 2335 Lines: 70 From: Namhyung Kim [mailto:namhyung@gmail.com] >On November 19, 2015 7:36:39 AM GMT+09:00, Arnaldo Carvalho de Melo wrote: >>Em Wed, Nov 18, 2015 at 03:40:12PM +0900, Masami Hiramatsu escreveu: >>> Since dwarf_cfi_addrframe returns malloc'd Dwarf_Frame >>> object, it has to be freed after used. >> >>Applied to perf/urgent >> >>> Signed-off-by: Masami Hiramatsu >>> --- >>> tools/perf/util/probe-finder.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/tools/perf/util/probe-finder.c >>b/tools/perf/util/probe-finder.c >>> index 63993d7..4d7d4f4 100644 >>> --- a/tools/perf/util/probe-finder.c >>> +++ b/tools/perf/util/probe-finder.c >>> @@ -683,21 +683,24 @@ static int call_probe_finder(Dwarf_Die *sc_die, >>struct probe_finder *pf) >>> ret = dwarf_getlocation_addr(&fb_attr, pf->addr, &pf->fb_ops, >>&nops, 1); >>> if (ret <= 0 || nops == 0) { >>> pf->fb_ops = NULL; >>> + ret = 0; >>> #if _ELFUTILS_PREREQ(0, 142) >>> } else if (nops == 1 && pf->fb_ops[0].atom == DW_OP_call_frame_cfa >>&& >>> pf->cfi != NULL) { >>> - Dwarf_Frame *frame; >>> + Dwarf_Frame *frame = NULL; >>> if (dwarf_cfi_addrframe(pf->cfi, pf->addr, &frame) != 0 || >>> dwarf_frame_cfa(frame, &pf->fb_ops, &nops) != 0) { > >What if dwarf_cfi_addrframe() succeeded but >dwarf_frame_cfa() failed? It seems that the frame >still can be leaked.. No, it is also caught by free(). Please see below, > >>> pr_warning("Failed to get call frame on 0x%jx\n", >>> (uintmax_t)pf->addr); >>> - return -ENOENT; >>> + ret = -ENOENT; I've replaced "return -ENOENT" with "ret = -ENOENT", so this fall down >>> } >>> + free(frame); and free the frame :) (and if frame == NULL, it is just ignored) Thank you! >>> #endif >>> } >>> >>> /* Call finder's callback handler */ >>> - ret = pf->callback(sc_die, pf); >>> + if (ret >= 0) >>> + ret = pf->callback(sc_die, pf); >>> >>> /* *pf->fb_ops will be cached in libdw. Don't free it. */ >>> pf->fb_ops = NULL; > >Hi Arnaldo and Masami, >-- >Sent from my Android device with K-9 Mail. Please excuse my brevity. ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?