From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C25DC7EE2F for ; Mon, 12 Jun 2023 21:23:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233103AbjFLVXK (ORCPT ); Mon, 12 Jun 2023 17:23:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39458 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233166AbjFLVW5 (ORCPT ); Mon, 12 Jun 2023 17:22:57 -0400 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A21C959C9; Mon, 12 Jun 2023 14:18:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686604713; x=1718140713; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=dhxgXYpIXG5Uh2B//r4CHAr2esb78E4fQ9A4WP2agR0=; b=MZgXvNRvxgPz5oz7OIrKzPG/+8zaTvJxs2mRJugML3OsYYwbetXmxrjp 3n4cgYZKZQxt/lP2h74oJSKT06j0tUGCKsQf4Z0jf1gZgnNQDMZBmZZQf DLEAtjKYO71eSKAxEwDfMDfxQ37pWn5My725TL8kryqWzzUYAS9BVb1ty oijz7T+XbhjeFbUIju3tBfKhBDySRYTHiszc4HbkWUMMiAZXkKMg3kp0j hS4DCB9GWIZekI/py5RSoSEXrKYSYeZlc8eb8cwaXdywPCA1kRe6CDhO4 GHVSzxPW5Pr2uEsNLkKPgTjykmHVq9bfpeOUs3CD479Hg9dWOd6owQoeE g==; X-IronPort-AV: E=McAfee;i="6600,9927,10739"; a="355659382" X-IronPort-AV: E=Sophos;i="6.00,236,1681196400"; d="scan'208";a="355659382" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2023 14:16:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10739"; a="801171734" X-IronPort-AV: E=Sophos;i="6.00,236,1681196400"; d="scan'208";a="801171734" Received: from tassilo.jf.intel.com (HELO tassilo) ([10.54.38.190]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2023 14:16:35 -0700 Date: Mon, 12 Jun 2023 14:16:34 -0700 From: Andi Kleen To: Arnaldo Carvalho de Melo Cc: Ian Rogers , John Garry , Will Deacon , James Clark , Mike Leach , Leo Yan , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Adrian Hunter , Suzuki K Poulose , "Naveen N. Rao" , Kan Liang , German Gomez , Ali Saidi , Jing Zhang , Athira Rajeev , Miguel Ojeda , ye xingchen , Liam Howlett , Dmitrii Dolgov <9erthalion6@gmail.com>, Yang Jihong , K Prateek Nayak , Changbin Du , Ravi Bangoria , Sean Christopherson , "Steinar H. Gunderson" , Yuan Can , Brian Robbins , liuwenyu , Ivan Babrou , Fangrui Song , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, coresight@lists.linaro.org Subject: Re: [PATCH v2 26/26] perf hist: Fix srcline memory leak Message-ID: References: <20230608232823.4027869-1-irogers@google.com> <20230608232823.4027869-27-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On Mon, Jun 12, 2023 at 02:23:41PM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Jun 12, 2023 at 07:46:14AM -0700, Ian Rogers escreveu: > > On Mon, Jun 12, 2023 at 7:16 AM Arnaldo Carvalho de Melo > > wrote: > > > > > > Em Mon, Jun 12, 2023 at 11:13:59AM -0300, Arnaldo Carvalho de Melo escreveu: > > > > Em Thu, Jun 08, 2023 at 04:28:23PM -0700, Ian Rogers escreveu: > > > > > srcline isn't freed if it is SRCLINE_UNKNOWN. Avoid strduping in this > > > > > case as such strdups are redundant and leak memory. > > > > > > > > The patch is ok as its what the rest of the code is doing, i.e. strcmp() > > > > to check if a srcline is the unknown one, but how about the following > > > > patch on top of yours? > > > > > > [acme@quaco perf-tools-next]$ strings ~/bin/perf | grep '??:0' > > > ??:0 > > > SRCLINE_UNKNOWN ((char *) "??:0") > > > [acme@quaco perf-tools-next]$ > > > > Agreed, the strcmps make me nervous as they won't distinguish heap > > from a global meaning we could end up with things like pointers to > > freed memory. The comparison with the global is always going to be > > same imo. > > > > Acked-by: Ian Rogers > > Thanks, applied and added your Acked-by. Actually was there another patch that turned it into a explicit global? At least in my tree it isn't: util/srcline.h 28:#define SRCLINE_UNKNOWN ((char *) "??:0") Without any explicit global it's a bit dangerous because you rely on the linker doing string deduplication. While it normally does that there might be odd corner cases where it doesn't. -Andi From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 389AFC7EE2F for ; Mon, 12 Jun 2023 21:17:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=L8proWPD6UULs/YQmEMglxIMShJWnu7Qp+i8QCXTcwQ=; b=mzsc9QDgyAqHGp dtFTL+4h2JjbBCPQgReHnsVugheP51EtGzdVqAA/wzPUpnctMz/o/4fb/tVBwVdZt/CJYdi8geAi6 8hMV5V0WCXZuuRWvOU+vT043r93V0DcvwEiNg2OGi8hT0kDd9ZlnuKsN0/+HRTtLSovxvCwo1qbra n7JY5iinmNBlZWAROp/6K/pn6INQ/QbH0vLQ3LIA/VLbwZEz/aH/4PXGms2asnUNRmhuAs2FLhNAL 2p6SuUhbg4/CbksXXwMjMj54djcLqsUX2gV36IKQguWKcc7UwLe+zKjS6LSvTFdr8R5ZHQzQg+IrV EBT2ND+8iuW5lguV8QXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q8ouI-005TS2-2w; Mon, 12 Jun 2023 21:16:58 +0000 Received: from mga11.intel.com ([192.55.52.93]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q8ouG-005TQW-0p for linux-arm-kernel@lists.infradead.org; Mon, 12 Jun 2023 21:16:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686604616; x=1718140616; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=dhxgXYpIXG5Uh2B//r4CHAr2esb78E4fQ9A4WP2agR0=; b=k1NfozAyMNDPPz7lL2iQHavmOHfTJrjXacam4qa8h8u6llIRuL6EYFXD KWkV132Yi3oVoPOOGW5PTEG6/PjGjNPNcirbmoXYhrdnhaknK8Rsjevao PWW5vJRTYw8ecTGBFa2MdZaTB6uVe1UK52kdjOpk4SE4fNqiuBddC3auw WUBXV4aQIBBrCI/amtVyFy1CtcVvppYSf8683BXUHUz6XO+S97xjYBGOl u7BGFC5yO+C+BVj+jN8wAM0xFYyKcKTKNSCaPq4Eag9EYge1MGYrqpIMm 0t2IRE+54Hrupk6qQeLYbqVG4RKtwaY5Rem1DuM/uIxKx+G7IGqC7g65n A==; X-IronPort-AV: E=McAfee;i="6600,9927,10739"; a="355659388" X-IronPort-AV: E=Sophos;i="6.00,236,1681196400"; d="scan'208";a="355659388" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2023 14:16:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10739"; a="801171734" X-IronPort-AV: E=Sophos;i="6.00,236,1681196400"; d="scan'208";a="801171734" Received: from tassilo.jf.intel.com (HELO tassilo) ([10.54.38.190]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2023 14:16:35 -0700 Date: Mon, 12 Jun 2023 14:16:34 -0700 From: Andi Kleen To: Arnaldo Carvalho de Melo Cc: Ian Rogers , John Garry , Will Deacon , James Clark , Mike Leach , Leo Yan , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Adrian Hunter , Suzuki K Poulose , "Naveen N. Rao" , Kan Liang , German Gomez , Ali Saidi , Jing Zhang , Athira Rajeev , Miguel Ojeda , ye xingchen , Liam Howlett , Dmitrii Dolgov <9erthalion6@gmail.com>, Yang Jihong , K Prateek Nayak , Changbin Du , Ravi Bangoria , Sean Christopherson , "Steinar H. Gunderson" , Yuan Can , Brian Robbins , liuwenyu , Ivan Babrou , Fangrui Song , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, coresight@lists.linaro.org Subject: Re: [PATCH v2 26/26] perf hist: Fix srcline memory leak Message-ID: References: <20230608232823.4027869-1-irogers@google.com> <20230608232823.4027869-27-irogers@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230612_141656_345907_3A9C417F X-CRM114-Status: GOOD ( 22.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org T24gTW9uLCBKdW4gMTIsIDIwMjMgYXQgMDI6MjM6NDFQTSAtMDMwMCwgQXJuYWxkbyBDYXJ2YWxo byBkZSBNZWxvIHdyb3RlOgo+IEVtIE1vbiwgSnVuIDEyLCAyMDIzIGF0IDA3OjQ2OjE0QU0gLTA3 MDAsIElhbiBSb2dlcnMgZXNjcmV2ZXU6Cj4gPiBPbiBNb24sIEp1biAxMiwgMjAyMyBhdCA3OjE2 4oCvQU0gQXJuYWxkbyBDYXJ2YWxobyBkZSBNZWxvCj4gPiA8YWNtZUBrZXJuZWwub3JnPiB3cm90 ZToKPiA+ID4KPiA+ID4gRW0gTW9uLCBKdW4gMTIsIDIwMjMgYXQgMTE6MTM6NTlBTSAtMDMwMCwg QXJuYWxkbyBDYXJ2YWxobyBkZSBNZWxvIGVzY3JldmV1Ogo+ID4gPiA+IEVtIFRodSwgSnVuIDA4 LCAyMDIzIGF0IDA0OjI4OjIzUE0gLTA3MDAsIElhbiBSb2dlcnMgZXNjcmV2ZXU6Cj4gPiA+ID4g PiBzcmNsaW5lIGlzbid0IGZyZWVkIGlmIGl0IGlzIFNSQ0xJTkVfVU5LTk9XTi4gQXZvaWQgc3Ry ZHVwaW5nIGluIHRoaXMKPiA+ID4gPiA+IGNhc2UgYXMgc3VjaCBzdHJkdXBzIGFyZSByZWR1bmRh bnQgYW5kIGxlYWsgbWVtb3J5Lgo+ID4gPiA+Cj4gPiA+ID4gVGhlIHBhdGNoIGlzIG9rIGFzIGl0 cyB3aGF0IHRoZSByZXN0IG9mIHRoZSBjb2RlIGlzIGRvaW5nLCBpLmUuIHN0cmNtcCgpCj4gPiA+ ID4gdG8gY2hlY2sgaWYgYSBzcmNsaW5lIGlzIHRoZSB1bmtub3duIG9uZSwgYnV0IGhvdyBhYm91 dCB0aGUgZm9sbG93aW5nCj4gPiA+ID4gcGF0Y2ggb24gdG9wIG9mIHlvdXJzPwo+ID4gPgo+ID4g PiBbYWNtZUBxdWFjbyBwZXJmLXRvb2xzLW5leHRdJCBzdHJpbmdzIH4vYmluL3BlcmYgfCBncmVw ICc/PzowJwo+ID4gPiA/PzowCj4gPiA+IFNSQ0xJTkVfVU5LTk9XTiAoKGNoYXIgKikgIj8/OjAi KQo+ID4gPiBbYWNtZUBxdWFjbyBwZXJmLXRvb2xzLW5leHRdJAo+ID4gCj4gPiBBZ3JlZWQsIHRo ZSBzdHJjbXBzIG1ha2UgbWUgbmVydm91cyBhcyB0aGV5IHdvbid0IGRpc3Rpbmd1aXNoIGhlYXAK PiA+IGZyb20gYSBnbG9iYWwgbWVhbmluZyB3ZSBjb3VsZCBlbmQgdXAgd2l0aCB0aGluZ3MgbGlr ZSBwb2ludGVycyB0bwo+ID4gZnJlZWQgbWVtb3J5LiBUaGUgY29tcGFyaXNvbiB3aXRoIHRoZSBn bG9iYWwgaXMgYWx3YXlzIGdvaW5nIHRvIGJlCj4gPiBzYW1lIGltby4KPiA+IAo+ID4gQWNrZWQt Ynk6IElhbiBSb2dlcnMgPGlyb2dlcnNAZ29vZ2xlLmNvbT4KPiAKPiBUaGFua3MsIGFwcGxpZWQg YW5kIGFkZGVkIHlvdXIgQWNrZWQtYnkuCgpBY3R1YWxseSB3YXMgdGhlcmUgYW5vdGhlciBwYXRj aCB0aGF0IHR1cm5lZCBpdCBpbnRvIGEgZXhwbGljaXQgZ2xvYmFsPyAKCkF0IGxlYXN0IGluIG15 IHRyZWUgaXQgaXNuJ3Q6Cgp1dGlsL3NyY2xpbmUuaAoyODojZGVmaW5lIFNSQ0xJTkVfVU5LTk9X TiAgKChjaGFyICopICI/PzowIikKCldpdGhvdXQgYW55IGV4cGxpY2l0IGdsb2JhbCBpdCdzIGEg Yml0IGRhbmdlcm91cyBiZWNhdXNlIHlvdSByZWx5IG9uIHRoZQpsaW5rZXIgZG9pbmcgc3RyaW5n IGRlZHVwbGljYXRpb24uICBXaGlsZSBpdCBub3JtYWxseSBkb2VzIHRoYXQgdGhlcmUgbWlnaHQg YmUKb2RkIGNvcm5lciBjYXNlcyB3aGVyZSBpdCBkb2Vzbid0LgoKLUFuZGkKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFp bGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlz dHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK