Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Mon, 4 Dec 2023 14:44:54 -0800 (PST)
From:   Ilkka Koskinen <ilkka@os.amperecomputing.com>
To:     Ian Rogers <irogers@google.com>
cc:     John Garry <john.g.garry@oracle.com>,
        Will Deacon <will@kernel.org>,
        James Clark <james.clark@arm.com>,
        Mike Leach <mike.leach@linaro.org>,
        Leo Yan <leo.yan@linaro.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Ilkka Koskinen <ilkka@os.amperecomputing.com>,
        Kan Liang <kan.liang@linux.intel.com>,
        Kajol Jain <kjain@linux.ibm.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] perf metrics: Avoid segv if default metricgroup
 isn't set
In-Reply-To: <20231204182330.654255-1-irogers@google.com>
Message-ID: <bea4ad79-297e-9737-8af1-2286192d26f4@os.amperecomputing.com>
References: <20231204182330.654255-1-irogers@google.com>
Content-Type: text/plain; charset=US-ASCII; format=flowed
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?K7HtSOay6ufLi8ZoKldM/bPHuFLucsugpcSfmV2M6EP+l0sm7mUqYPRPIBXi?=
 =?us-ascii?Q?ER7E/Zg8oF9EABRG4cjFOsYkQE2CwIkWgN/4UfN5GMpGOAyhxLnqs+bjrq4A?=
 =?us-ascii?Q?5q/i/9baoSVh5PG0dXQVmnvZ5MZGmUdMBoedR8hCk1Hf9CbbQWBE9zGTJv0z?=
 =?us-ascii?Q?dTeVXDDmP6A32ntyVSbCBPZAdL7qsMYsNEj6g/0uWyxOhHViW1gBMhnr1+rI?=
 =?us-ascii?Q?mUJvfHlO7xkUL+UKNfwGLk1YCCacr4UPFi/ybp2Uocf4653jj8e8niYRQiqE?=
 =?us-ascii?Q?G9dH7tlZKw4IMMnQmD9OYkM4keKWfbLy/a+hE9Ei8BAWklO2x0PWr5DVL5tL?=
 =?us-ascii?Q?HBhGaW6As+mu1FRph8gVst8lsxP+8+1Kf7fzAktHHclNFBc3fZxWScJTMf7d?=
 =?us-ascii?Q?Dk8MPURh15uZHwWTPXmD6OujcvSY0LuqN9wiSJBttISs8XblsIyNqamdZ0EJ?=
 =?us-ascii?Q?pdQW7VL3Og4V2fYs6QBpPjFKPUtPBv0mwX8Rtv+ih21tus0uUzM8ccCw4tRG?=
 =?us-ascii?Q?PCY1zSo3z6k9Ys4Jb3BO2gwZzj3nBsoBvQMRcLJ2TeSJldfGZFF/FWiGKXmP?=
 =?us-ascii?Q?5mSU9S06M36Kx22B1PMhmT8G853Cr+NfQtpvriDXFbElih1snz04iVMsy2cs?=
 =?us-ascii?Q?fyEVbv/0rz0+/I1WicJsKJjFmZhg+MtRas7NZSiTKd3pmu2CypOqgZLP2VVk?=
 =?us-ascii?Q?VoIr2zq+2h4whrUxmfoMUZ+DspcMKLWJJ1NChH+FgjISFM2NOCR3p1f7/e43?=
 =?us-ascii?Q?MzxAOH0GnoUR2otOwlNPLqFgH6xjymL83ccP+1MonwYTO/lGjsXLgiCGxEVV?=
 =?us-ascii?Q?e7RxuIoyqLgol1aB3E47x3JsACQwjaOCHDnjFOCWdCVMhr0Mig+v9/ZxyQwR?=
 =?us-ascii?Q?O4IX8PlqeZsD7j2xJR/rpFmlvhOyWvNlQjLH1QWXDPPQbdinfCPJLF6SjCQH?=
 =?us-ascii?Q?X2my4LoGMlSzZPt3rVTt15LBRFu66ZgdoNbCGntG7ryYnV93+9swmfMY6ChB?=
 =?us-ascii?Q?NmBut6pRmQTNxt1Qkhv0LFLy2cBLqUF5e27J+LNJPw4vS1DYtk0Cg/bHqLC6?=
 =?us-ascii?Q?CxRYTl4/rEDYYcLIaZpSA8GGpbSh1osej5BrJ+fDDJNbNhSmjeYYayNZ3gUT?=
 =?us-ascii?Q?/bgpjy+3m6neICtKB7s9VPONbRATgWcg2shqiqvzt/TbaAoB2yAtfUGc1Leo?=
 =?us-ascii?Q?NSCqdG04ocuPNKh/9jPiIgPpD4bKeeAdneazDlAgFz7Sx9kC+9q5XUUvaJ+o?=
 =?us-ascii?Q?tbW9L0xBD0gPuHNE7ntnC7GZ4S7+VdECziIGjkRVrv7og+PZFSUDSEnpLsQF?=
 =?us-ascii?Q?bYtX29MDP1P4j3dR07xGMiK3JJ3tFslWvCNj/s5qJSUnzIt9YYEXPZksjHg7?=
 =?us-ascii?Q?8GpHyNvgLo+wNTDZb6mO0LafySViuXahLpDxc+eeK7WWmHWliVllD/HU8562?=
 =?us-ascii?Q?YEFevZfq+WXKFuGFtkfGLYgSyClvUxYzBW7zRUfpIxu0RQ1SNwLKx01Yh4+h?=
 =?us-ascii?Q?/vLVd/oORDBYaoXGFOcyISzqc1rDzqEyTYW8OyBNn9MI7gjEq4quksdTCibB?=
 =?us-ascii?Q?G/fDd3xqLMFXpFtLfp9vNClh9tEd6w6xom1QQU6C0vdaZZEy8TNwgJilVZ3D?=
 =?us-ascii?Q?7QoFmipYQIuU4uHIqtmWeZQ=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 916ec6fa-26e3-4e0c-3f7f-08dbf51aaf8a
X-MS-Exchange-CrossTenant-AuthSource: DM5PR0102MB3590.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Dec 2023 22:45:16.8382
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Ph4yiv1lpWugDtHbFC2cIf97z9yqrG0mY6K86dR9COnlPz+4uLZv0ICotFxdmo0QODfx+evmE6p/ChQW4ni7Onb29cA0XiK8AAXLvWHuMmVQSmHpAsr0NN88QLyC6sjd
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR01MB8116
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Mon, 04 Dec 2023 14:45:34 -0800 (PST)


On Mon, 4 Dec 2023, Ian Rogers wrote:
> A metric is default by having "Default" within its groups. The default
> metricgroup name needn't be set and this can result in segv in
> default_metricgroup_cmp and perf_stat__print_shadow_stats_metricgroup
> that assume it has a value when there is a Default metric group. To
> avoid the segv initialize the value to "".
>
> Fixes: 1c0e47956a8e ("perf metrics: Sort the Default metricgroup")
> Signed-off-by: Ian Rogers <irogers@google.com>

Thanks! I was going to look for the bug but got pulled to other 
tasks. The patch looks good to me and I tested it successfully on 
AmpereOne.

   Reviewed-and-tested-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>


Cheers, Ilkka

> ---
> tools/perf/util/metricgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 0484736d9fe4..ca3e0404f187 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -225,7 +225,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
>
> 	m->pmu = pm->pmu ?: "cpu";
> 	m->metric_name = pm->metric_name;
> -	m->default_metricgroup_name = pm->default_metricgroup_name;
> +	m->default_metricgroup_name = pm->default_metricgroup_name ?: "";
> 	m->modifier = NULL;
> 	if (modifier) {
> 		m->modifier = strdup(modifier);
> -- 
> 2.43.0.rc2.451.g8631bc7472-goog
>
>