linux-kernel - Re: [PATCH] libperf: Don't remove -g when EXTRA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAP-5=fVHGryhBtt9SANOdyw3JijvefgqAHgZuiFBEuPmSdoTxA@mail.gmail.com>
Date: Wed, 19 Mar 2025 08:43:04 -0700
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: linux-perf-users@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...ux.intel.com>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] libperf: Don't remove -g when EXTRA_CFLAGS are used

On Wed, Mar 19, 2025 at 4:40 AM James Clark <james.clark@...aro.org> wrote:
>
> When using EXTRA_CFLAGS, for example "EXTRA_CFLAGS=-DREFCNT_CHECKING=1",
> this construct stops setting -g which you'd expect would not be affected
> by adding extra flags. Additionally, EXTRA_CFLAGS should be the last
> thing to be appended so that it can be used to undo any defaults. And no
> condition is required, just += appends to any existing CFLAGS and also
> appends or doesn't append EXTRA_CFLAGS if they are or aren't set.
>
> It's not clear why DEBUG=1 is required for -g in Perf when in libperf
> it's always on, but I don't think we need to change that behavior now
> because someone may be depending on it.
>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
>  tools/lib/perf/Makefile | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
> index 3a9b2140aa04..478fe57bf8ce 100644
> --- a/tools/lib/perf/Makefile
> +++ b/tools/lib/perf/Makefile
> @@ -54,13 +54,6 @@ endif
>
>  TEST_ARGS := $(if $(V),-v)
>
> -# Set compile option CFLAGS
> -ifdef EXTRA_CFLAGS
> -  CFLAGS := $(EXTRA_CFLAGS)
> -else
> -  CFLAGS := -g -Wall
> -endif

I suspect this was cargo culted from:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/bpf/Makefile?h=perf-tools-next#n81
I'm not clear why it was ever that way, but perhaps a wider cleanup is
necessary than just this one patch.

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

Thanks,
Ian

>  INCLUDES = \
>  -I$(srctree)/tools/lib/perf/include \
>  -I$(srctree)/tools/lib/ \
> @@ -70,11 +63,12 @@ INCLUDES = \
>  -I$(srctree)/tools/include/uapi
>
>  # Append required CFLAGS
> -override CFLAGS += $(EXTRA_WARNINGS)
> -override CFLAGS += -Werror -Wall
> +override CFLAGS += -g -Werror -Wall
>  override CFLAGS += -fPIC
>  override CFLAGS += $(INCLUDES)
>  override CFLAGS += -fvisibility=hidden
> +override CFLAGS += $(EXTRA_WARNINGS)
> +override CFLAGS += $(EXTRA_CFLAGS)
>
>  all:
>
> --
> 2.34.1
>