linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
@ 2025-04-06 16:34 Likhitha Korrapati
  2025-04-06 18:40 ` Athira Rajeev
  2025-04-07  5:39 ` Mukesh Kumar Chaurasiya
  0 siblings, 2 replies; 17+ messages in thread
From: Likhitha Korrapati @ 2025-04-06 16:34 UTC (permalink / raw)
  To: acme, jolsa, adrian.hunter, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev,
	Likhitha Korrapati

perf build break observed when using gcc 13-3 (FC39 ppc64le)
with the following error.

cpumap.c: In function 'perf_cpu_map__merge':
cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
  414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from cpumap.c:4:
/usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
  672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
      |              ^~~~~~
cc1: all warnings being treated as errors

Error happens to be only in gcc13-3 and not in latest gcc 14.
Even though git-bisect pointed bad commit as:
'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
issue is with tmp_len being "int". It holds number of cpus and making
it "unsigned int" fixes the issues.

After the fix:

  CC      util/pmu-flex.o
  CC      util/expr-flex.o
  LD      util/perf-util-in.o
  LD      perf-util-in.o
  AR      libperf-util.a
  LINK    perf
  GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so

Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
---
 tools/lib/perf/cpumap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
index 4454a5987570..c7c784e18225 100644
--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
 int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
 {
 	struct perf_cpu *tmp_cpus;
-	int tmp_len;
+	unsigned int tmp_len;
 	int i, j, k;
 	struct perf_cpu_map *merged;
 
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-06 16:34 [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c Likhitha Korrapati
@ 2025-04-06 18:40 ` Athira Rajeev
  2025-04-07 12:08   ` Venkat Rao Bagalkote
  2025-04-07  5:39 ` Mukesh Kumar Chaurasiya
  1 sibling, 1 reply; 17+ messages in thread
From: Athira Rajeev @ 2025-04-06 18:40 UTC (permalink / raw)
  To: Likhitha Korrapati, Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Madhavan Srinivasan



> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
> 
> perf build break observed when using gcc 13-3 (FC39 ppc64le)
> with the following error.
> 
> cpumap.c: In function 'perf_cpu_map__merge':
> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>  414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from cpumap.c:4:
> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>  672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>      |              ^~~~~~
> cc1: all warnings being treated as errors
> 
> Error happens to be only in gcc13-3 and not in latest gcc 14.
> Even though git-bisect pointed bad commit as:
> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
> issue is with tmp_len being "int". It holds number of cpus and making
> it "unsigned int" fixes the issues.
> 
> After the fix:
> 
>  CC      util/pmu-flex.o
>  CC      util/expr-flex.o
>  LD      util/perf-util-in.o
>  LD      perf-util-in.o
>  AR      libperf-util.a
>  LINK    perf
>  GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
> 
> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
Looks good to me

Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>

Thanks
Athira
> ---
> tools/lib/perf/cpumap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index 4454a5987570..c7c784e18225 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> {
> struct perf_cpu *tmp_cpus;
> - int tmp_len;
> + unsigned int tmp_len;
> int i, j, k;
> struct perf_cpu_map *merged;
> 
> -- 
> 2.43.5
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-06 16:34 [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c Likhitha Korrapati
  2025-04-06 18:40 ` Athira Rajeev
@ 2025-04-07  5:39 ` Mukesh Kumar Chaurasiya
  1 sibling, 0 replies; 17+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2025-04-07  5:39 UTC (permalink / raw)
  To: Likhitha Korrapati
  Cc: acme, jolsa, adrian.hunter, irogers, namhyung, linux-perf-users,
	linuxppc-dev, maddy, atrajeev

On Sun, Apr 06, 2025 at 10:04:12PM +0530, Likhitha Korrapati wrote:
> perf build break observed when using gcc 13-3 (FC39 ppc64le)
> with the following error.
> 
> cpumap.c: In function 'perf_cpu_map__merge':
> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from cpumap.c:4:
> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>       |              ^~~~~~
> cc1: all warnings being treated as errors
> 
> Error happens to be only in gcc13-3 and not in latest gcc 14.
> Even though git-bisect pointed bad commit as:
> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
> issue is with tmp_len being "int". It holds number of cpus and making
> it "unsigned int" fixes the issues.
> 
> After the fix:
> 
>   CC      util/pmu-flex.o
>   CC      util/expr-flex.o
>   LD      util/perf-util-in.o
>   LD      perf-util-in.o
>   AR      libperf-util.a
>   LINK    perf
>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
> 
> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
> ---
>  tools/lib/perf/cpumap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index 4454a5987570..c7c784e18225 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
>  int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>  {
>  	struct perf_cpu *tmp_cpus;
> -	int tmp_len;
> +	unsigned int tmp_len;
>  	int i, j, k;
>  	struct perf_cpu_map *merged;
>
LGTM

Reviewed-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>

> -- 
> 2.43.5
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-06 18:40 ` Athira Rajeev
@ 2025-04-07 12:08   ` Venkat Rao Bagalkote
  2025-04-14  1:38     ` Madhavan Srinivasan
  0 siblings, 1 reply; 17+ messages in thread
From: Venkat Rao Bagalkote @ 2025-04-07 12:08 UTC (permalink / raw)
  To: Athira Rajeev, Likhitha Korrapati, Arnaldo Carvalho de Melo,
	Namhyung Kim
  Cc: Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Madhavan Srinivasan


On 07/04/25 12:10 am, Athira Rajeev wrote:
>
>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
>>
>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
>> with the following error.
>>
>> cpumap.c: In function 'perf_cpu_map__merge':
>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> In file included from cpumap.c:4:
>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>>       |              ^~~~~~
>> cc1: all warnings being treated as errors
>>
>> Error happens to be only in gcc13-3 and not in latest gcc 14.
>> Even though git-bisect pointed bad commit as:
>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
>> issue is with tmp_len being "int". It holds number of cpus and making
>> it "unsigned int" fixes the issues.
>>
>> After the fix:
>>
>>   CC      util/pmu-flex.o
>>   CC      util/expr-flex.o
>>   LD      util/perf-util-in.o
>>   LD      perf-util-in.o
>>   AR      libperf-util.a
>>   LINK    perf
>>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
>>
>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
> Looks good to me
>
> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>

Tested this patch on perf-tools-next repo, and this patch fixes the issue.

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>


Regards,

Venkat.

>
> Thanks
> Athira
>> ---
>> tools/lib/perf/cpumap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
>> index 4454a5987570..c7c784e18225 100644
>> --- a/tools/lib/perf/cpumap.c
>> +++ b/tools/lib/perf/cpumap.c
>> @@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
>> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>> {
>> struct perf_cpu *tmp_cpus;
>> - int tmp_len;
>> + unsigned int tmp_len;
>> int i, j, k;
>> struct perf_cpu_map *merged;
>>
>> -- 
>> 2.43.5
>>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-07 12:08   ` Venkat Rao Bagalkote
@ 2025-04-14  1:38     ` Madhavan Srinivasan
  2025-04-25 14:49       ` Athira Rajeev
  0 siblings, 1 reply; 17+ messages in thread
From: Madhavan Srinivasan @ 2025-04-14  1:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, Likhitha Korrapati,
	linuxppc-dev, Venkat Rao Bagalkote, Athira Rajeev



On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
> 
> On 07/04/25 12:10 am, Athira Rajeev wrote:
>>
>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
>>>
>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
>>> with the following error.
>>>
>>> cpumap.c: In function 'perf_cpu_map__merge':
>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> In file included from cpumap.c:4:
>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>>>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>>>       |              ^~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
>>> Even though git-bisect pointed bad commit as:
>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
>>> issue is with tmp_len being "int". It holds number of cpus and making
>>> it "unsigned int" fixes the issues.
>>>
>>> After the fix:
>>>
>>>   CC      util/pmu-flex.o
>>>   CC      util/expr-flex.o
>>>   LD      util/perf-util-in.o
>>>   LD      perf-util-in.o
>>>   AR      libperf-util.a
>>>   LINK    perf
>>>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
>>>
>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
>> Looks good to me
>>
>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> Tested this patch on perf-tools-next repo, and this patch fixes the issue.
> 
> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> 
>

Arnaldo, Namhyung,

can you consider pulling this fix? since it is breaking the build in gcc13-3 or
if you have any comments do let us know.

Thanks
Maddy


 
> Regards,
> 
> Venkat.
> 
>>
>> Thanks
>> Athira
>>> ---
>>> tools/lib/perf/cpumap.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
>>> index 4454a5987570..c7c784e18225 100644
>>> --- a/tools/lib/perf/cpumap.c
>>> +++ b/tools/lib/perf/cpumap.c
>>> @@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
>>> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>>> {
>>> struct perf_cpu *tmp_cpus;
>>> - int tmp_len;
>>> + unsigned int tmp_len;
>>> int i, j, k;
>>> struct perf_cpu_map *merged;
>>>
>>> -- 
>>> 2.43.5
>>>
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-14  1:38     ` Madhavan Srinivasan
@ 2025-04-25 14:49       ` Athira Rajeev
  2025-04-25 17:46         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Athira Rajeev @ 2025-04-25 14:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Jiri Olsa, Adrian Hunter,
	Ian Rogers, open list:PERFORMANCE EVENTS SUBSYSTEM,
	Likhitha Korrapati, linuxppc-dev, Venkat Rao Bagalkote,
	Madhavan Srinivasan



> On 14 Apr 2025, at 7:08 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
> 
> 
> 
> On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
>> 
>> On 07/04/25 12:10 am, Athira Rajeev wrote:
>>> 
>>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
>>>> 
>>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
>>>> with the following error.
>>>> 
>>>> cpumap.c: In function 'perf_cpu_map__merge':
>>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>>>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>>>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> In file included from cpumap.c:4:
>>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>>>>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>>>>       |              ^~~~~~
>>>> cc1: all warnings being treated as errors
>>>> 
>>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
>>>> Even though git-bisect pointed bad commit as:
>>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
>>>> issue is with tmp_len being "int". It holds number of cpus and making
>>>> it "unsigned int" fixes the issues.
>>>> 
>>>> After the fix:
>>>> 
>>>>   CC      util/pmu-flex.o
>>>>   CC      util/expr-flex.o
>>>>   LD      util/perf-util-in.o
>>>>   LD      perf-util-in.o
>>>>   AR      libperf-util.a
>>>>   LINK    perf
>>>>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
>>>> 
>>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
>>> Looks good to me
>>> 
>>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>
>> 
>> Tested this patch on perf-tools-next repo, and this patch fixes the issue.
>> 
>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>> 
>> 
> 
> Arnaldo, Namhyung,
> 
> can you consider pulling this fix? since it is breaking the build in gcc13-3 or
> if you have any comments do let us know.
> 
> Thanks
> Maddy

Hi,

Can we get this pulled in if the change looks good ? This is breaking build on gcc-13-3 
Looking for feedback on this patch..

Thanks
Athira
> 
> 
> 
>> Regards,
>> 
>> Venkat.
>> 
>>> 
>>> Thanks
>>> Athira
>>>> ---
>>>> tools/lib/perf/cpumap.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
>>>> index 4454a5987570..c7c784e18225 100644
>>>> --- a/tools/lib/perf/cpumap.c
>>>> +++ b/tools/lib/perf/cpumap.c
>>>> @@ -398,7 +398,7 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
>>>> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>>>> {
>>>> struct perf_cpu *tmp_cpus;
>>>> - int tmp_len;
>>>> + unsigned int tmp_len;
>>>> int i, j, k;
>>>> struct perf_cpu_map *merged;
>>>> 
>>>> -- 
>>>> 2.43.5



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-25 14:49       ` Athira Rajeev
@ 2025-04-25 17:46         ` Arnaldo Carvalho de Melo
  2025-04-29  5:11           ` Likhitha Korrapati
                             ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-04-25 17:46 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, Likhitha Korrapati,
	linuxppc-dev, Venkat Rao Bagalkote, Madhavan Srinivasan

On Fri, Apr 25, 2025 at 08:19:02PM +0530, Athira Rajeev wrote:
> > On 14 Apr 2025, at 7:08 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
> > On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
> >> On 07/04/25 12:10 am, Athira Rajeev wrote:
> >>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:

> >>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
> >>>> with the following error.

> >>>> cpumap.c: In function 'perf_cpu_map__merge':
> >>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
> >>>>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> >>>>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> In file included from cpumap.c:4:
> >>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
> >>>>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
> >>>>       |              ^~~~~~
> >>>> cc1: all warnings being treated as errors

> >>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
> >>>> Even though git-bisect pointed bad commit as:
> >>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
> >>>> issue is with tmp_len being "int". It holds number of cpus and making
> >>>> it "unsigned int" fixes the issues.

> >>>> After the fix:

> >>>>   CC      util/pmu-flex.o
> >>>>   CC      util/expr-flex.o
> >>>>   LD      util/perf-util-in.o
> >>>>   LD      perf-util-in.o
> >>>>   AR      libperf-util.a
> >>>>   LINK    perf
> >>>>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so

> >>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
> >>> Looks good to me

> >>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>

> >> Tested this patch on perf-tools-next repo, and this patch fixes the issue.

> >> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>

> > Arnaldo, Namhyung,

> > can you consider pulling this fix? since it is breaking the build in gcc13-3 or
> > if you have any comments do let us know.

This isn't the only place in that file where this pattern exists:

⬢ [acme@toolbx perf-tools-next]$ grep malloc tools/lib/perf/cpumap.c 
	cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
⬢ [acme@toolbx perf-tools-next]$


struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus)
{
        RC_STRUCT(perf_cpu_map) *cpus;
        struct perf_cpu_map *result;

        if (nr_cpus == 0)
                return NULL;

        cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);


int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
{
        struct perf_cpu *tmp_cpus;
        int tmp_len;
        int i, j, k;
        struct perf_cpu_map *merged;

        if (perf_cpu_map__is_subset(*orig, other))
                return 0;
        if (perf_cpu_map__is_subset(other, *orig)) {
                perf_cpu_map__put(*orig);
                *orig = perf_cpu_map__get(other);
                return 0;
        }

        tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
        tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));


struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
                                             struct perf_cpu_map *other)
{
        struct perf_cpu *tmp_cpus;
        int tmp_len;
        int i, j, k;
        struct perf_cpu_map *merged = NULL;

        if (perf_cpu_map__is_subset(other, orig))
                return perf_cpu_map__get(orig);
        if (perf_cpu_map__is_subset(orig, other))
                return perf_cpu_map__get(other);

        tmp_len = max(__perf_cpu_map__nr(orig), __perf_cpu_map__nr(other));
        tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));

I'm trying to figure out why its only in perf_cpu_map__merge() that this
triggers :-\

Maybe that max() call in perf_cpu_map__intersect() somehow makes the
compiler happy.

And in perf_cpu_map__alloc() all calls seems to validate it.

But wouldn't turning this into a calloc() be better?

Like:

diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
index 4454a5987570cfbc..99d21618a252ac0e 100644
--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
        }
 
        tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
-       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
+       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
        if (!tmp_cpus)
                return -ENOMEM;
 
⬢ [acme@toolbx perf-tools-next]$


And better, do the max size that the compiler is trying to help us
catch?

- Arnaldo

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-25 17:46         ` Arnaldo Carvalho de Melo
@ 2025-04-29  5:11           ` Likhitha Korrapati
  2025-05-02  7:44           ` Mukesh Kumar Chaurasiya
  2025-05-02  9:05           ` Likhitha Korrapati
  2 siblings, 0 replies; 17+ messages in thread
From: Likhitha Korrapati @ 2025-04-29  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Athira Rajeev
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

Hi Arnaldo,

On 4/25/25 23:16, Arnaldo Carvalho de Melo wrote:
> On Fri, Apr 25, 2025 at 08:19:02PM +0530, Athira Rajeev wrote:
>>> On 14 Apr 2025, at 7:08 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
>>> On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
>>>> On 07/04/25 12:10 am, Athira Rajeev wrote:
>>>>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
> 
>>>>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
>>>>>> with the following error.
> 
>>>>>> cpumap.c: In function 'perf_cpu_map__merge':
>>>>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>>>>>    414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>>>>>        |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>> In file included from cpumap.c:4:
>>>>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>>>>>>    672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>>>>>>        |              ^~~~~~
>>>>>> cc1: all warnings being treated as errors
> 
>>>>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
>>>>>> Even though git-bisect pointed bad commit as:
>>>>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
>>>>>> issue is with tmp_len being "int". It holds number of cpus and making
>>>>>> it "unsigned int" fixes the issues.
> 
>>>>>> After the fix:
> 
>>>>>>    CC      util/pmu-flex.o
>>>>>>    CC      util/expr-flex.o
>>>>>>    LD      util/perf-util-in.o
>>>>>>    LD      perf-util-in.o
>>>>>>    AR      libperf-util.a
>>>>>>    LINK    perf
>>>>>>    GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
> 
>>>>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
>>>>> Looks good to me
> 
>>>>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
>>>> Tested this patch on perf-tools-next repo, and this patch fixes the issue.
> 
>>>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> 
>>> Arnaldo, Namhyung,
> 
>>> can you consider pulling this fix? since it is breaking the build in gcc13-3 or
>>> if you have any comments do let us know.
> 
> This isn't the only place in that file where this pattern exists:
> 
> ⬢ [acme@toolbx perf-tools-next]$ grep malloc tools/lib/perf/cpumap.c
> 	cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus)
> {
>          RC_STRUCT(perf_cpu_map) *cpus;
>          struct perf_cpu_map *result;
> 
>          if (nr_cpus == 0)
>                  return NULL;
> 
>          cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 
> 
> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> {
>          struct perf_cpu *tmp_cpus;
>          int tmp_len;
>          int i, j, k;
>          struct perf_cpu_map *merged;
> 
>          if (perf_cpu_map__is_subset(*orig, other))
>                  return 0;
>          if (perf_cpu_map__is_subset(other, *orig)) {
>                  perf_cpu_map__put(*orig);
>                  *orig = perf_cpu_map__get(other);
>                  return 0;
>          }
> 
>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> 
> struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
>                                               struct perf_cpu_map *other)
> {
>          struct perf_cpu *tmp_cpus;
>          int tmp_len;
>          int i, j, k;
>          struct perf_cpu_map *merged = NULL;
> 
>          if (perf_cpu_map__is_subset(other, orig))
>                  return perf_cpu_map__get(orig);
>          if (perf_cpu_map__is_subset(orig, other))
>                  return perf_cpu_map__get(other);
> 
>          tmp_len = max(__perf_cpu_map__nr(orig), __perf_cpu_map__nr(other));
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> I'm trying to figure out why its only in perf_cpu_map__merge() that this
> triggers :-\
> 
> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> compiler happy.
> 
> And in perf_cpu_map__alloc() all calls seems to validate it.
> 
> But wouldn't turning this into a calloc() be better?

I have tried using calloc() instead of malloc() and the issue still 
exists even using calloc().

cpumap.c: In function ‘perf_cpu_map__merge’:
cpumap.c:414:20: error: argument 1 range [18446744071562067968, 
18446744073709551615] exceeds maximum object size 9223372036854775807 
[-Werror=alloc-size-larger-than=]

   414 |         tmp_cpus = calloc(tmp_len , sizeof(struct perf_cpu));

> 
> Like:
> 
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index 4454a5987570cfbc..99d21618a252ac0e 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>          }
>   
>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
>          if (!tmp_cpus)
>                  return -ENOMEM;
>   
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> And better, do the max size that the compiler is trying to help us
> catch?
> 
> - Arnaldo
I have tried using max and it worked. I am doing testing with this 
change and will be posting a V2.

Thanks
Likhitha

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-25 17:46         ` Arnaldo Carvalho de Melo
  2025-04-29  5:11           ` Likhitha Korrapati
@ 2025-05-02  7:44           ` Mukesh Kumar Chaurasiya
  2025-05-13 21:13             ` Arnaldo Carvalho de Melo
  2025-05-02  9:05           ` Likhitha Korrapati
  2 siblings, 1 reply; 17+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2025-05-02  7:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Athira Rajeev, Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, Likhitha Korrapati,
	linuxppc-dev, Venkat Rao Bagalkote, Madhavan Srinivasan

On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Apr 25, 2025 at 08:19:02PM +0530, Athira Rajeev wrote:
> > > On 14 Apr 2025, at 7:08 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
> > > On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
> > >> On 07/04/25 12:10 am, Athira Rajeev wrote:
> > >>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
> 
> > >>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
> > >>>> with the following error.
> 
> > >>>> cpumap.c: In function 'perf_cpu_map__merge':
> > >>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
> > >>>>   414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> > >>>>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >>>> In file included from cpumap.c:4:
> > >>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
> > >>>>   672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
> > >>>>       |              ^~~~~~
> > >>>> cc1: all warnings being treated as errors
> 
> > >>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
> > >>>> Even though git-bisect pointed bad commit as:
> > >>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
> > >>>> issue is with tmp_len being "int". It holds number of cpus and making
> > >>>> it "unsigned int" fixes the issues.
> 
> > >>>> After the fix:
> 
> > >>>>   CC      util/pmu-flex.o
> > >>>>   CC      util/expr-flex.o
> > >>>>   LD      util/perf-util-in.o
> > >>>>   LD      perf-util-in.o
> > >>>>   AR      libperf-util.a
> > >>>>   LINK    perf
> > >>>>   GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
> 
> > >>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
> > >>> Looks good to me
> 
> > >>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> > >> Tested this patch on perf-tools-next repo, and this patch fixes the issue.
> 
> > >> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> 
> > > Arnaldo, Namhyung,
> 
> > > can you consider pulling this fix? since it is breaking the build in gcc13-3 or
> > > if you have any comments do let us know.
> 
> This isn't the only place in that file where this pattern exists:
> 
> ⬢ [acme@toolbx perf-tools-next]$ grep malloc tools/lib/perf/cpumap.c 
> 	cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus)
> {
>         RC_STRUCT(perf_cpu_map) *cpus;
>         struct perf_cpu_map *result;
> 
>         if (nr_cpus == 0)
>                 return NULL;
> 
>         cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 
> 
> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> {
>         struct perf_cpu *tmp_cpus;
>         int tmp_len;
>         int i, j, k;
>         struct perf_cpu_map *merged;
> 
>         if (perf_cpu_map__is_subset(*orig, other))
>                 return 0;
>         if (perf_cpu_map__is_subset(other, *orig)) {
>                 perf_cpu_map__put(*orig);
>                 *orig = perf_cpu_map__get(other);
>                 return 0;
>         }
> 
>         tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> 
> struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
>                                              struct perf_cpu_map *other)
> {
>         struct perf_cpu *tmp_cpus;
>         int tmp_len;
>         int i, j, k;
>         struct perf_cpu_map *merged = NULL;
> 
>         if (perf_cpu_map__is_subset(other, orig))
>                 return perf_cpu_map__get(orig);
>         if (perf_cpu_map__is_subset(orig, other))
>                 return perf_cpu_map__get(other);
> 
>         tmp_len = max(__perf_cpu_map__nr(orig), __perf_cpu_map__nr(other));
>         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> I'm trying to figure out why its only in perf_cpu_map__merge() that this
> triggers :-\
> 
> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> compiler happy.
> 
> And in perf_cpu_map__alloc() all calls seems to validate it.
> 
> But wouldn't turning this into a calloc() be better?
> 
> Like:
> 
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index 4454a5987570cfbc..99d21618a252ac0e 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>         }
>  
>         tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
>         if (!tmp_cpus)
>                 return -ENOMEM;
>  
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> And better, do the max size that the compiler is trying to help us
> catch?
> 
> - Arnaldo
Isn't it better to use perf_cpu_map__nr. That should fix this problem.

One question I have, in perf_cpu_map__nr, the function is returning
1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?

Regards,
Mukesh

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-04-25 17:46         ` Arnaldo Carvalho de Melo
  2025-04-29  5:11           ` Likhitha Korrapati
  2025-05-02  7:44           ` Mukesh Kumar Chaurasiya
@ 2025-05-02  9:05           ` Likhitha Korrapati
  2 siblings, 0 replies; 17+ messages in thread
From: Likhitha Korrapati @ 2025-05-02  9:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Athira Rajeev
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

Hi Arnaldo,

On 4/25/25 23:16, Arnaldo Carvalho de Melo wrote:
> On Fri, Apr 25, 2025 at 08:19:02PM +0530, Athira Rajeev wrote:
>>> On 14 Apr 2025, at 7:08 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
>>> On 4/7/25 5:38 PM, Venkat Rao Bagalkote wrote:
>>>> On 07/04/25 12:10 am, Athira Rajeev wrote:
>>>>>> On 6 Apr 2025, at 10:04 PM, Likhitha Korrapati <likhitha@linux.ibm.com> wrote:
> 
>>>>>> perf build break observed when using gcc 13-3 (FC39 ppc64le)
>>>>>> with the following error.
> 
>>>>>> cpumap.c: In function 'perf_cpu_map__merge':
>>>>>> cpumap.c:414:20: error: argument 1 range [18446744069414584320, 18446744073709551614] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>>>>>    414 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>>>>>        |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>> In file included from cpumap.c:4:
>>>>>> /usr/include/stdlib.h:672:14: note: in a call to allocation function 'malloc' declared here
>>>>>>    672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
>>>>>>        |              ^~~~~~
>>>>>> cc1: all warnings being treated as errors
> 
>>>>>> Error happens to be only in gcc13-3 and not in latest gcc 14.
>>>>>> Even though git-bisect pointed bad commit as:
>>>>>> 'commit f5b07010c13c ("libperf: Don't remove -g when EXTRA_CFLAGS are used")',
>>>>>> issue is with tmp_len being "int". It holds number of cpus and making
>>>>>> it "unsigned int" fixes the issues.
> 
>>>>>> After the fix:
> 
>>>>>>    CC      util/pmu-flex.o
>>>>>>    CC      util/expr-flex.o
>>>>>>    LD      util/perf-util-in.o
>>>>>>    LD      perf-util-in.o
>>>>>>    AR      libperf-util.a
>>>>>>    LINK    perf
>>>>>>    GEN     python/perf.cpython-312-powerpc64le-linux-gnu.so
> 
>>>>>> Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
>>>>> Looks good to me
> 
>>>>> Reviewed-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
>>>> Tested this patch on perf-tools-next repo, and this patch fixes the issue.
> 
>>>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> 
>>> Arnaldo, Namhyung,
> 
>>> can you consider pulling this fix? since it is breaking the build in gcc13-3 or
>>> if you have any comments do let us know.
> 
> This isn't the only place in that file where this pattern exists:
> 
> ⬢ [acme@toolbx perf-tools-next]$ grep malloc tools/lib/perf/cpumap.c
> 	cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 	tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus)
> {
>          RC_STRUCT(perf_cpu_map) *cpus;
>          struct perf_cpu_map *result;
> 
>          if (nr_cpus == 0)
>                  return NULL;
> 
>          cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus);
> 
> 
> int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> {
>          struct perf_cpu *tmp_cpus;
>          int tmp_len;
>          int i, j, k;
>          struct perf_cpu_map *merged;
> 
>          if (perf_cpu_map__is_subset(*orig, other))
>                  return 0;
>          if (perf_cpu_map__is_subset(other, *orig)) {
>                  perf_cpu_map__put(*orig);
>                  *orig = perf_cpu_map__get(other);
>                  return 0;
>          }
> 
>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> 
> struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
>                                               struct perf_cpu_map *other)
> {
>          struct perf_cpu *tmp_cpus;
>          int tmp_len;
>          int i, j, k;
>          struct perf_cpu_map *merged = NULL;
> 
>          if (perf_cpu_map__is_subset(other, orig))
>                  return perf_cpu_map__get(orig);
>          if (perf_cpu_map__is_subset(orig, other))
>                  return perf_cpu_map__get(other);
> 
>          tmp_len = max(__perf_cpu_map__nr(orig), __perf_cpu_map__nr(other));
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> 
> I'm trying to figure out why its only in perf_cpu_map__merge() that this
> triggers :-\
> 
> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> compiler happy.
> 
> And in perf_cpu_map__alloc() all calls seems to validate it.
> 
> But wouldn't turning this into a calloc() be better?
> 
> Like:
> 
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index 4454a5987570cfbc..99d21618a252ac0e 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>          }
>   
>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
>          if (!tmp_cpus)
>                  return -ENOMEM;
>   
> ⬢ [acme@toolbx perf-tools-next]$
> 
> 
> And better, do the max size that the compiler is trying to help us
> catch?
> 
> - Arnaldo

I am not sure if using max() is right incase of perf_cpu_map_merge as 
this is a merge functionality. As this is summation we get a value 
greater than the max of orig and other cpus. And this might be correct 
in perf_cpu_map__intersect() but will cause issues in perf_cpu_map__marge().
Can you please eloborate if you meant this or there is something else I 
missed.

I tried the following:

int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
         return cpus ? __perf_cpu_map__nr(cpus) : 1;
}

static int __perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
         return RC_CHK_ACCESS(cpus)->nr;
}

This got introduced via commit 7d1b529f164d33ad4514b272bcec65036873d717 
where it assumes cpu map as non-null.

And we are checking this non-null in perf_cpu_map__merge()

         if (perf_cpu_map__is_subset(*orig, other))
                 return 0;
         if (perf_cpu_map__is_subset(other, *orig)) {
                 perf_cpu_map__put(*orig);
                 *orig = perf_cpu_map__get(other);
                 return 0;
         }

Using perf_cpu_map__nr instead of __perf_cpu_map__nr  and this works as 
it has a check.

--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, 
struct perf_cpu_map *other)
                 return 0;
         }

-       tmp_len = max(__perf_cpu_map__nr(*orig), __perf_cpu_map__nr(other));
+       tmp_len = perf_cpu_map__nr(*orig) +  perf_cpu_map__nr(other);
         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
         if (!tmp_cpus)
                 return -ENOMEM;
Regards,
Likhitha.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-02  7:44           ` Mukesh Kumar Chaurasiya
@ 2025-05-13 21:13             ` Arnaldo Carvalho de Melo
  2025-05-13 22:12               ` Ian Rogers
  2025-05-21 13:03               ` Likhitha Korrapati
  0 siblings, 2 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-13 21:13 UTC (permalink / raw)
  To: Mukesh Kumar Chaurasiya
  Cc: Athira Rajeev, Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, Likhitha Korrapati,
	linuxppc-dev, Venkat Rao Bagalkote, Madhavan Srinivasan

On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
> On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> > compiler happy.

> > And in perf_cpu_map__alloc() all calls seems to validate it.
 
> > Like:

> > +++ b/tools/lib/perf/cpumap.c
> > @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> >         }
> >  
> >         tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> > -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> > +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
> >         if (!tmp_cpus)
> >                 return -ENOMEM;

> > ⬢ [acme@toolbx perf-tools-next]$

> > And better, do the max size that the compiler is trying to help us
> > catch?

> Isn't it better to use perf_cpu_map__nr. That should fix this problem.

Maybe, have you tried it?
 
> One question I have, in perf_cpu_map__nr, the function is returning
> 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?

Indeed this better be documented, as by just looking at:

int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
        return cpus ? __perf_cpu_map__nr(cpus) : 1;
}

It really doesn't make much sense to say that a NULL map has one entry.

But the next functions are:

bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
{
        return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
}

bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
{
        if (!map)
                return true;

        return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
}

bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
{
        return map == NULL;
}

So it seems that a NULL cpu map means "any/all CPU) and a map with just
one entry would have as its content "-1" that would mean "any/all CPU".

Ian did work on trying to simplify/clarify this, so maybe he can chime
in :-)

- Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-13 21:13             ` Arnaldo Carvalho de Melo
@ 2025-05-13 22:12               ` Ian Rogers
  2025-05-13 22:36                 ` Ian Rogers
  2025-05-21 13:03               ` Likhitha Korrapati
  1 sibling, 1 reply; 17+ messages in thread
From: Ian Rogers @ 2025-05-13 22:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Mukesh Kumar Chaurasiya, Athira Rajeev, Namhyung Kim, Jiri Olsa,
	Adrian Hunter, open list:PERFORMANCE EVENTS SUBSYSTEM,
	Likhitha Korrapati, linuxppc-dev, Venkat Rao Bagalkote,
	Madhavan Srinivasan

On Tue, May 13, 2025 at 2:13 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
> > On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> > > compiler happy.
>
> > > And in perf_cpu_map__alloc() all calls seems to validate it.
>
> > > Like:
>
> > > +++ b/tools/lib/perf/cpumap.c
> > > @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> > >         }
> > >
> > >         tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> > > -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> > > +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
> > >         if (!tmp_cpus)
> > >                 return -ENOMEM;
>
> > > ⬢ [acme@toolbx perf-tools-next]$
>
> > > And better, do the max size that the compiler is trying to help us
> > > catch?
>
> > Isn't it better to use perf_cpu_map__nr. That should fix this problem.
>
> Maybe, have you tried it?
>
> > One question I have, in perf_cpu_map__nr, the function is returning
> > 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
>
> Indeed this better be documented, as by just looking at:
>
> int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> {
>         return cpus ? __perf_cpu_map__nr(cpus) : 1;
> }
>
> It really doesn't make much sense to say that a NULL map has one entry.
>
> But the next functions are:
>
> bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> {
>         return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
> }
>
> bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> {
>         if (!map)
>                 return true;
>
>         return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
> }
>
> bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
> {
>         return map == NULL;
> }
>
> So it seems that a NULL cpu map means "any/all CPU) and a map with just
> one entry would have as its content "-1" that would mean "any/all CPU".
>
> Ian did work on trying to simplify/clarify this, so maybe he can chime
> in :-)

So I've tried to improve the naming but not vary the implementation
greatly - initially I was in the code fixing reference count checking
issues. There is an important distinction between "all" meaning a
range of CPUs like 0-15 on a 16 core/hyperthread system, and "any"
meaning the special "-1" value. It is possible to have a perf_cpu_map
to both be "all" and "any", iterating an empty perf_cpu_map has
strangely also meant the "any" and so the code isn't specific but
relies on these odd properties.

Anyway, I'm not sure on the implication of this with
malloc/calloc/unsigned... It would seem reasonable to me for
__perf_cpu_map__nr to return an unsigned number and to propagate that
to fix the new GCC issue.

Thanks,
Ian


> - Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-13 22:12               ` Ian Rogers
@ 2025-05-13 22:36                 ` Ian Rogers
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Rogers @ 2025-05-13 22:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Mukesh Kumar Chaurasiya, Athira Rajeev, Namhyung Kim, Jiri Olsa,
	Adrian Hunter, open list:PERFORMANCE EVENTS SUBSYSTEM,
	Likhitha Korrapati, linuxppc-dev, Venkat Rao Bagalkote,
	Madhavan Srinivasan

On Tue, May 13, 2025 at 3:12 PM Ian Rogers <irogers@google.com> wrote:
>
> On Tue, May 13, 2025 at 2:13 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
> > > On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> > > > compiler happy.
> >
> > > > And in perf_cpu_map__alloc() all calls seems to validate it.
> >
> > > > Like:
> >
> > > > +++ b/tools/lib/perf/cpumap.c
> > > > @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> > > >         }
> > > >
> > > >         tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> > > > -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> > > > +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
> > > >         if (!tmp_cpus)
> > > >                 return -ENOMEM;
> >
> > > > ⬢ [acme@toolbx perf-tools-next]$
> >
> > > > And better, do the max size that the compiler is trying to help us
> > > > catch?
> >
> > > Isn't it better to use perf_cpu_map__nr. That should fix this problem.
> >
> > Maybe, have you tried it?
> >
> > > One question I have, in perf_cpu_map__nr, the function is returning
> > > 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
> >
> > Indeed this better be documented, as by just looking at:
> >
> > int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> > {
> >         return cpus ? __perf_cpu_map__nr(cpus) : 1;
> > }
> >
> > It really doesn't make much sense to say that a NULL map has one entry.
> >
> > But the next functions are:
> >
> > bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> > {
> >         return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
> > }
> >
> > bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> > {
> >         if (!map)
> >                 return true;
> >
> >         return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
> > }
> >
> > bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
> > {
> >         return map == NULL;
> > }
> >
> > So it seems that a NULL cpu map means "any/all CPU) and a map with just
> > one entry would have as its content "-1" that would mean "any/all CPU".
> >
> > Ian did work on trying to simplify/clarify this, so maybe he can chime
> > in :-)
>
> So I've tried to improve the naming but not vary the implementation
> greatly - initially I was in the code fixing reference count checking
> issues. There is an important distinction between "all" meaning a
> range of CPUs like 0-15 on a 16 core/hyperthread system, and "any"
> meaning the special "-1" value. It is possible to have a perf_cpu_map
> to both be "all" and "any", iterating an empty perf_cpu_map has
> strangely also meant the "any" and so the code isn't specific but
> relies on these odd properties.

I just remembered, there was also a discussion with Adrian IIRC. I
wanted "all" to mean all possible CPUs on a system, and "online" to be
either the same or a subset of that, in case CPUs were offline. There
was a different view that the distinction between "all" and "online"
wasn't useful. So there is some confusion in the code in this regard
and some errors may exist when CPU cores are offline. There's
definitely work to make the API better.

Thanks,
Ian

> Anyway, I'm not sure on the implication of this with
> malloc/calloc/unsigned... It would seem reasonable to me for
> __perf_cpu_map__nr to return an unsigned number and to propagate that
> to fix the new GCC issue.
>
> Thanks,
> Ian
>
>
> > - Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-13 21:13             ` Arnaldo Carvalho de Melo
  2025-05-13 22:12               ` Ian Rogers
@ 2025-05-21 13:03               ` Likhitha Korrapati
  2025-05-21 15:45                 ` Ian Rogers
  1 sibling, 1 reply; 17+ messages in thread
From: Likhitha Korrapati @ 2025-05-21 13:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Mukesh Kumar Chaurasiya
  Cc: Athira Rajeev, Namhyung Kim, Jiri Olsa, Adrian Hunter, Ian Rogers,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

Hi Arnaldo,

On 5/14/25 02:43, Arnaldo Carvalho de Melo wrote:
> On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
>> On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
>>> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
>>> compiler happy.
> 
>>> And in perf_cpu_map__alloc() all calls seems to validate it.
>   
>>> Like:
> 
>>> +++ b/tools/lib/perf/cpumap.c
>>> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>>>          }
>>>   
>>>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>>> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
>>>          if (!tmp_cpus)
>>>                  return -ENOMEM;
> 
>>> ⬢ [acme@toolbx perf-tools-next]$
> 
>>> And better, do the max size that the compiler is trying to help us
>>> catch?
> 
>> Isn't it better to use perf_cpu_map__nr. That should fix this problem.
> 
> Maybe, have you tried it?

I have tried this method and it works.

--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, 
struct perf_cpu_map *other)
                 return 0;
         }

-       tmp_len = max(__perf_cpu_map__nr(*orig), __perf_cpu_map__nr(other));
+       tmp_len = perf_cpu_map__nr(*orig) +  perf_cpu_map__nr(other);
         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
         if (!tmp_cpus)
                 return -ENOMEM;

I will send a V2 with this change if this looks good.

Thanks
Likhitha.

>   
>> One question I have, in perf_cpu_map__nr, the function is returning
>> 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
> 
> Indeed this better be documented, as by just looking at:
> 
> int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> {
>          return cpus ? __perf_cpu_map__nr(cpus) : 1;
> }
> 
> It really doesn't make much sense to say that a NULL map has one entry.
> 
> But the next functions are:
> 
> bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> {
>          return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
> }
> 
> bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> {
>          if (!map)
>                  return true;
> 
>          return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
> }
> 
> bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
> {
>          return map == NULL;
> }
> 
> So it seems that a NULL cpu map means "any/all CPU) and a map with just
> one entry would have as its content "-1" that would mean "any/all CPU".
> 
> Ian did work on trying to simplify/clarify this, so maybe he can chime
> in :-)
> 
> - Arnaldo


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-21 13:03               ` Likhitha Korrapati
@ 2025-05-21 15:45                 ` Ian Rogers
  2025-05-21 17:28                   ` Likhitha Korrapati
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Rogers @ 2025-05-21 15:45 UTC (permalink / raw)
  To: Likhitha Korrapati
  Cc: Arnaldo Carvalho de Melo, Mukesh Kumar Chaurasiya, Athira Rajeev,
	Namhyung Kim, Jiri Olsa, Adrian Hunter,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

On Wed, May 21, 2025 at 6:03 AM Likhitha Korrapati
<likhitha@linux.ibm.com> wrote:
>
> Hi Arnaldo,
>
> On 5/14/25 02:43, Arnaldo Carvalho de Melo wrote:
> > On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
> >> On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> >>> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> >>> compiler happy.
> >
> >>> And in perf_cpu_map__alloc() all calls seems to validate it.
> >
> >>> Like:
> >
> >>> +++ b/tools/lib/perf/cpumap.c
> >>> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> >>>          }
> >>>
> >>>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> >>> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> >>> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
> >>>          if (!tmp_cpus)
> >>>                  return -ENOMEM;
> >
> >>> ⬢ [acme@toolbx perf-tools-next]$
> >
> >>> And better, do the max size that the compiler is trying to help us
> >>> catch?
> >
> >> Isn't it better to use perf_cpu_map__nr. That should fix this problem.
> >
> > Maybe, have you tried it?
>
> I have tried this method and it works.
>
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig,
> struct perf_cpu_map *other)
>                  return 0;
>          }
>
> -       tmp_len = max(__perf_cpu_map__nr(*orig), __perf_cpu_map__nr(other));
> +       tmp_len = perf_cpu_map__nr(*orig) +  perf_cpu_map__nr(other);
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>          if (!tmp_cpus)
>                  return -ENOMEM;
>
> I will send a V2 with this change if this looks good.

How is this different from the existing code:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/cpumap.c?h=perf-tools-next#n423
```
        tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
        tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
        if (!tmp_cpus)
                return -ENOMEM;
```

Thanks,
Ian

> Thanks
> Likhitha.
>
> >
> >> One question I have, in perf_cpu_map__nr, the function is returning
> >> 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
> >
> > Indeed this better be documented, as by just looking at:
> >
> > int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> > {
> >          return cpus ? __perf_cpu_map__nr(cpus) : 1;
> > }
> >
> > It really doesn't make much sense to say that a NULL map has one entry.
> >
> > But the next functions are:
> >
> > bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> > {
> >          return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
> > }
> >
> > bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> > {
> >          if (!map)
> >                  return true;
> >
> >          return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
> > }
> >
> > bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
> > {
> >          return map == NULL;
> > }
> >
> > So it seems that a NULL cpu map means "any/all CPU) and a map with just
> > one entry would have as its content "-1" that would mean "any/all CPU".
> >
> > Ian did work on trying to simplify/clarify this, so maybe he can chime
> > in :-)
> >
> > - Arnaldo
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-21 15:45                 ` Ian Rogers
@ 2025-05-21 17:28                   ` Likhitha Korrapati
  2025-05-21 17:39                     ` Ian Rogers
  0 siblings, 1 reply; 17+ messages in thread
From: Likhitha Korrapati @ 2025-05-21 17:28 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Arnaldo Carvalho de Melo, Mukesh Kumar Chaurasiya, Athira Rajeev,
	Namhyung Kim, Jiri Olsa, Adrian Hunter,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

Hi Ian,

On 5/21/25 21:15, Ian Rogers wrote:
> On Wed, May 21, 2025 at 6:03 AM Likhitha Korrapati
> <likhitha@linux.ibm.com> wrote:
>>
>> Hi Arnaldo,
>>
>> On 5/14/25 02:43, Arnaldo Carvalho de Melo wrote:
>>> On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
>>>> On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
>>>>> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
>>>>> compiler happy.
>>>
>>>>> And in perf_cpu_map__alloc() all calls seems to validate it.
>>>
>>>>> Like:
>>>
>>>>> +++ b/tools/lib/perf/cpumap.c
>>>>> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
>>>>>           }
>>>>>
>>>>>           tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>>>>> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>>>> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
>>>>>           if (!tmp_cpus)
>>>>>                   return -ENOMEM;
>>>
>>>>> ⬢ [acme@toolbx perf-tools-next]$
>>>
>>>>> And better, do the max size that the compiler is trying to help us
>>>>> catch?
>>>
>>>> Isn't it better to use perf_cpu_map__nr. That should fix this problem.
>>>
>>> Maybe, have you tried it?
>>
>> I have tried this method and it works.
>>
>> --- a/tools/lib/perf/cpumap.c
>> +++ b/tools/lib/perf/cpumap.c
>> @@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig,
>> struct perf_cpu_map *other)
>>                   return 0;
>>           }
>>
>> -       tmp_len = max(__perf_cpu_map__nr(*orig), __perf_cpu_map__nr(other));
>> +       tmp_len = perf_cpu_map__nr(*orig) +  perf_cpu_map__nr(other);
>>           tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>>           if (!tmp_cpus)
>>                   return -ENOMEM;
>>
>> I will send a V2 with this change if this looks good.
> 
> How is this different from the existing code:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/cpumap.c?h=perf-tools-next#n423
> ```
>          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>          if (!tmp_cpus)
>                  return -ENOMEM;
> ```
> 
> Thanks,
> Ian

I gave the wrong diff. Here is the corrected diff.

--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, 
struct perf_cpu_map *other)
                 return 0;
         }

-       tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
+       tmp_len = perf_cpu_map__nr(*orig) + perf_cpu_map__nr(other);
         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
         if (!tmp_cpus)
                 return -ENOMEM;


I am using perf_cpu_map__nr instead of __perf_cpu_map__nr.

Thanks,
Likhitha.

> 
>> Thanks
>> Likhitha.
>>
>>>
>>>> One question I have, in perf_cpu_map__nr, the function is returning
>>>> 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
>>>
>>> Indeed this better be documented, as by just looking at:
>>>
>>> int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
>>> {
>>>           return cpus ? __perf_cpu_map__nr(cpus) : 1;
>>> }
>>>
>>> It really doesn't make much sense to say that a NULL map has one entry.
>>>
>>> But the next functions are:
>>>
>>> bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
>>> {
>>>           return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
>>> }
>>>
>>> bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
>>> {
>>>           if (!map)
>>>                   return true;
>>>
>>>           return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
>>> }
>>>
>>> bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
>>> {
>>>           return map == NULL;
>>> }
>>>
>>> So it seems that a NULL cpu map means "any/all CPU) and a map with just
>>> one entry would have as its content "-1" that would mean "any/all CPU".
>>>
>>> Ian did work on trying to simplify/clarify this, so maybe he can chime
>>> in :-)
>>>
>>> - Arnaldo
>>
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
  2025-05-21 17:28                   ` Likhitha Korrapati
@ 2025-05-21 17:39                     ` Ian Rogers
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Rogers @ 2025-05-21 17:39 UTC (permalink / raw)
  To: Likhitha Korrapati
  Cc: Arnaldo Carvalho de Melo, Mukesh Kumar Chaurasiya, Athira Rajeev,
	Namhyung Kim, Jiri Olsa, Adrian Hunter,
	open list:PERFORMANCE EVENTS SUBSYSTEM, linuxppc-dev,
	Venkat Rao Bagalkote, Madhavan Srinivasan

On Wed, May 21, 2025 at 10:28 AM Likhitha Korrapati
<likhitha@linux.ibm.com> wrote:
>
> Hi Ian,
>
> On 5/21/25 21:15, Ian Rogers wrote:
> > On Wed, May 21, 2025 at 6:03 AM Likhitha Korrapati
> > <likhitha@linux.ibm.com> wrote:
> >>
> >> Hi Arnaldo,
> >>
> >> On 5/14/25 02:43, Arnaldo Carvalho de Melo wrote:
> >>> On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
> >>>> On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:
> >>>>> Maybe that max() call in perf_cpu_map__intersect() somehow makes the
> >>>>> compiler happy.
> >>>
> >>>>> And in perf_cpu_map__alloc() all calls seems to validate it.
> >>>
> >>>>> Like:
> >>>
> >>>>> +++ b/tools/lib/perf/cpumap.c
> >>>>> @@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
> >>>>>           }
> >>>>>
> >>>>>           tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> >>>>> -       tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> >>>>> +       tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu));
> >>>>>           if (!tmp_cpus)
> >>>>>                   return -ENOMEM;
> >>>
> >>>>> ⬢ [acme@toolbx perf-tools-next]$
> >>>
> >>>>> And better, do the max size that the compiler is trying to help us
> >>>>> catch?
> >>>
> >>>> Isn't it better to use perf_cpu_map__nr. That should fix this problem.
> >>>
> >>> Maybe, have you tried it?
> >>
> >> I have tried this method and it works.
> >>
> >> --- a/tools/lib/perf/cpumap.c
> >> +++ b/tools/lib/perf/cpumap.c
> >> @@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig,
> >> struct perf_cpu_map *other)
> >>                   return 0;
> >>           }
> >>
> >> -       tmp_len = max(__perf_cpu_map__nr(*orig), __perf_cpu_map__nr(other));
> >> +       tmp_len = perf_cpu_map__nr(*orig) +  perf_cpu_map__nr(other);
> >>           tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> >>           if (!tmp_cpus)
> >>                   return -ENOMEM;
> >>
> >> I will send a V2 with this change if this looks good.
> >
> > How is this different from the existing code:
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/cpumap.c?h=perf-tools-next#n423
> > ```
> >          tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> >          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> >          if (!tmp_cpus)
> >                  return -ENOMEM;
> > ```
> >
> > Thanks,
> > Ian
>
> I gave the wrong diff. Here is the corrected diff.
>
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -410,7 +410,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig,
> struct perf_cpu_map *other)
>                  return 0;
>          }
>
> -       tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
> +       tmp_len = perf_cpu_map__nr(*orig) + perf_cpu_map__nr(other);
>          tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
>          if (!tmp_cpus)
>                  return -ENOMEM;
>
>
> I am using perf_cpu_map__nr instead of __perf_cpu_map__nr.

Ok, why is that a fix? The function declarations are near identical
and perf_cpu_map__nr is implemented in terms of __perf_cpu_map__nr:
```
static int __perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
        return RC_CHK_ACCESS(cpus)->nr;
}
int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
        return cpus ? __perf_cpu_map__nr(cpus) : 1;
}
```
My guess is that being static allows all of the code to be analyzed in
the compilation unit and thereby create the warning/error, your change
is just defeating the analysis. The analysis could easily kick in
again for Link Time Optimization. I'd prefer making these `__nr`
functions return `unsigned int` or size_t over changes like this.

Thanks,
Ian

> Thanks,
> Likhitha.
>
> >
> >> Thanks
> >> Likhitha.
> >>
> >>>
> >>>> One question I have, in perf_cpu_map__nr, the function is returning
> >>>> 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
> >>>
> >>> Indeed this better be documented, as by just looking at:
> >>>
> >>> int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> >>> {
> >>>           return cpus ? __perf_cpu_map__nr(cpus) : 1;
> >>> }
> >>>
> >>> It really doesn't make much sense to say that a NULL map has one entry.
> >>>
> >>> But the next functions are:
> >>>
> >>> bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> >>> {
> >>>           return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
> >>> }
> >>>
> >>> bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
> >>> {
> >>>           if (!map)
> >>>                   return true;
> >>>
> >>>           return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
> >>> }
> >>>
> >>> bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
> >>> {
> >>>           return map == NULL;
> >>> }
> >>>
> >>> So it seems that a NULL cpu map means "any/all CPU) and a map with just
> >>> one entry would have as its content "-1" that would mean "any/all CPU".
> >>>
> >>> Ian did work on trying to simplify/clarify this, so maybe he can chime
> >>> in :-)
> >>>
> >>> - Arnaldo
> >>
> >
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-05-21 17:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-06 16:34 [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c Likhitha Korrapati
2025-04-06 18:40 ` Athira Rajeev
2025-04-07 12:08   ` Venkat Rao Bagalkote
2025-04-14  1:38     ` Madhavan Srinivasan
2025-04-25 14:49       ` Athira Rajeev
2025-04-25 17:46         ` Arnaldo Carvalho de Melo
2025-04-29  5:11           ` Likhitha Korrapati
2025-05-02  7:44           ` Mukesh Kumar Chaurasiya
2025-05-13 21:13             ` Arnaldo Carvalho de Melo
2025-05-13 22:12               ` Ian Rogers
2025-05-13 22:36                 ` Ian Rogers
2025-05-21 13:03               ` Likhitha Korrapati
2025-05-21 15:45                 ` Ian Rogers
2025-05-21 17:28                   ` Likhitha Korrapati
2025-05-21 17:39                     ` Ian Rogers
2025-05-02  9:05           ` Likhitha Korrapati
2025-04-07  5:39 ` Mukesh Kumar Chaurasiya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).