[PATCH] perf: Always feature test reallocarray

[PATCH] perf: Always feature test reallocarray

[James Clark]

This is also used in util/comm.c now, so instead of selectively doing
the feature test, always do it. If it's ever used anywhere else it's
less likely to cause another build failure.

This doesn't remove the need to manually include libc_compat.h, and
missing that will still cause an error for glibc < 2.26. There isn't a
way to fix that without poisoning reallocarray like libbpf did, but that
has other downsides like making memory debugging tools less useful. So
for Perf keep it like this and we'll have to fix up any missed includes.

Fixes the following build error:

  util/comm.c:152:31: error: implicit declaration of function
                      'reallocarray' [-Wimplicit-function-declaration]
  152 |                         tmp = reallocarray(comm_strs->strs,
      |                               ^~~~~~~~~~~~

Fixes: 13ca628716c6 ("perf comm: Add reference count checking to 'struct comm_str'")
Reported-by: Ali Utku Selen <ali.utku.selen@arm.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/Makefile.config | 10 ++++------
 tools/perf/util/comm.c     |  2 ++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a148ca9efca9..ed3ba13f1018 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -497,13 +497,14 @@ ifeq ($(feature-setns), 1)
   $(call detected,CONFIG_SETNS)
 endif
 
+ifeq ($(feature-reallocarray), 0)
+      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
+endif
+
 ifdef CORESIGHT
   $(call feature_check,libopencsd)
   ifeq ($(feature-libopencsd), 1)
     CFLAGS += -DHAVE_CSTRACE_SUPPORT $(LIBOPENCSD_CFLAGS)
-    ifeq ($(feature-reallocarray), 0)
-      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
-    endif
     LDFLAGS += $(LIBOPENCSD_LDFLAGS)
     EXTLIBS += $(OPENCSDLIBS)
     $(call detected,CONFIG_LIBOPENCSD)
@@ -1103,9 +1104,6 @@ ifndef NO_AUXTRACE
   ifndef NO_AUXTRACE
     $(call detected,CONFIG_AUXTRACE)
     CFLAGS += -DHAVE_AUXTRACE_SUPPORT
-    ifeq ($(feature-reallocarray), 0)
-      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
-    endif
   endif
 endif
 
diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
index 49b79cf0c5cc..8aa456d7c2cd 100644
--- a/tools/perf/util/comm.c
+++ b/tools/perf/util/comm.c
@@ -5,6 +5,8 @@
 #include <internal/rc_check.h>
 #include <linux/refcount.h>
 #include <linux/zalloc.h>
+#include <tools/libc_compat.h> // reallocarray
+
 #include "rwsem.h"
 
 DECLARE_RC_STRUCT(comm_str) {
-- 
2.34.1

Re: [PATCH] perf: Always feature test reallocarray

[Ian Rogers]

On Wed, Jan 29, 2025 at 7:44 AM James Clark <james.clark@linaro.org> wrote:
>
> This is also used in util/comm.c now, so instead of selectively doing
> the feature test, always do it. If it's ever used anywhere else it's
> less likely to cause another build failure.
>
> This doesn't remove the need to manually include libc_compat.h, and
> missing that will still cause an error for glibc < 2.26. There isn't a
> way to fix that without poisoning reallocarray like libbpf did, but that
> has other downsides like making memory debugging tools less useful. So
> for Perf keep it like this and we'll have to fix up any missed includes.
>
> Fixes the following build error:
>
>   util/comm.c:152:31: error: implicit declaration of function
>                       'reallocarray' [-Wimplicit-function-declaration]
>   152 |                         tmp = reallocarray(comm_strs->strs,
>       |                               ^~~~~~~~~~~~
>
> Fixes: 13ca628716c6 ("perf comm: Add reference count checking to 'struct comm_str'")
> Reported-by: Ali Utku Selen <ali.utku.selen@arm.com>
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/Makefile.config | 10 ++++------
>  tools/perf/util/comm.c     |  2 ++
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a148ca9efca9..ed3ba13f1018 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -497,13 +497,14 @@ ifeq ($(feature-setns), 1)
>    $(call detected,CONFIG_SETNS)
>  endif
>
> +ifeq ($(feature-reallocarray), 0)
> +      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> +endif
> +
>  ifdef CORESIGHT
>    $(call feature_check,libopencsd)
>    ifeq ($(feature-libopencsd), 1)
>      CFLAGS += -DHAVE_CSTRACE_SUPPORT $(LIBOPENCSD_CFLAGS)
> -    ifeq ($(feature-reallocarray), 0)
> -      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> -    endif
>      LDFLAGS += $(LIBOPENCSD_LDFLAGS)
>      EXTLIBS += $(OPENCSDLIBS)
>      $(call detected,CONFIG_LIBOPENCSD)
> @@ -1103,9 +1104,6 @@ ifndef NO_AUXTRACE
>    ifndef NO_AUXTRACE
>      $(call detected,CONFIG_AUXTRACE)
>      CFLAGS += -DHAVE_AUXTRACE_SUPPORT
> -    ifeq ($(feature-reallocarray), 0)
> -      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> -    endif
>    endif
>  endif
>
> diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
> index 49b79cf0c5cc..8aa456d7c2cd 100644
> --- a/tools/perf/util/comm.c
> +++ b/tools/perf/util/comm.c
> @@ -5,6 +5,8 @@
>  #include <internal/rc_check.h>
>  #include <linux/refcount.h>
>  #include <linux/zalloc.h>
> +#include <tools/libc_compat.h> // reallocarray
> +
>  #include "rwsem.h"
>
>  DECLARE_RC_STRUCT(comm_str) {
> --
> 2.34.1
>

Re: [PATCH] perf: Always feature test reallocarray

[Namhyung Kim]

On Wed, 29 Jan 2025 15:44:05 +0000, James Clark wrote:
> This is also used in util/comm.c now, so instead of selectively doing
> the feature test, always do it. If it's ever used anywhere else it's
> less likely to cause another build failure.
> 
> This doesn't remove the need to manually include libc_compat.h, and
> missing that will still cause an error for glibc < 2.26. There isn't a
> way to fix that without poisoning reallocarray like libbpf did, but that
> has other downsides like making memory debugging tools less useful. So
> for Perf keep it like this and we'll have to fix up any missed includes.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung