Re: [PATCH stable 6.1 1/3] perf build: Conditionally define NDEBUG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Defining NDEBUG removes the side-effecting `bfd_check_format()` call inside `assert()`, leaving the `bfd` object uninitialized.
- [Medium] Compiling out assert() removes critical runtime validation, leading to deadlocks and NULL pointer dereferences.
- [Low] Defining NDEBUG causes variables only used in assert() to trigger unused-but-set build failures.
--

commit 34a3ad3c8a3c7ed93c69e4b84492292fae1cb400
Author: Ian Rogers <irogers@google.com>

perf build: Conditionally define NDEBUG

When a build is done without DEBUG=1 then define NDEBUG. This will
compile out asserts and other debug code.

Link: https://lore.kernel.org/r/20230330183827.1412303-1-irogers@google.com

> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 898226ea8cadc..45708084fd3d6 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -228,6 +228,7 @@ ifndef DEBUG
>  endif
>  
>  ifeq ($(DEBUG),0)
> +CORE_CFLAGS += -DNDEBUG=1

[Severity: Medium]
Will this cause a regression in tools/perf/util/annotate.c? The function
bfd_check_format() is called directly inside an assert() macro there. 

Since this function has side effects (identifying the binary format and
initializing the architecture and machine state), compiling it out will leave
the bfd structure uninitialized. 

Subsequent calls like bfd_get_arch(bfdf) operate on uninitialized bfdf state,
leading to NULL pointer dereferences and crashes during perf annotate.

[Severity: Low]
Will this also cause build regressions due to unused variables? Variables such
as old in tools/perf/util/block-range.c are initialized and assigned values but
are only read inside assert() macros. 

Compiling out assert() leaves these variables assigned but never read, which
triggers unused-but-set build failures because the perf build uses -Werror.

[Severity: Medium]
Does compiling out assert() remove required runtime validation elsewhere? The
codebase currently relies on assert() for handling expected runtime failures.

For example, in bench_sched_pipe() (tools/perf/bench/sched-pipe.c),
assert(pid >= 0) checks for fork() failures. Without it, a fork failure causes
the parent to execute the child's logic, reading from an empty pipe and
deadlocking indefinitely.

Similarly, in tools/perf/builtin-timechart.c (assert(cursor != NULL)) and
tools/perf/builtin-top.c (evlist__id2evsel), allocation and lookup failures
proceed without aborting, leading to immediate NULL pointer dereferences on
subsequent lines.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519185154.2987285-1-florian.fainelli@broadcom.com?part=1