Re: [PATCH v3 7/7] perf trace: Update beautifier script for clone flags

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Adding 64-bit flag masks introduces an undefined behavior shift overflow on 32-bit platforms, causing corrupt perf trace output.
- [Medium] Missing empty input handling in `xargs` generates a buggy C array entry when compiled against older kernel headers.
--

commit 0277447cb377648d9d63a42e778d841a1fdaac29
Author: Namhyung Kim <namhyung@kernel.org>
perf trace: Update beautifier script for clone flags

This commit updates the clone beautifier script to parse newer clone flags
that use the 1ULL shift pattern. It extracts 64-bit clone3 flags up to bit
37 to generate the clone_flags array.

[ ... ]
>       [ilog2(0x80000000) + 1] = "IO",
>       [ilog2(0x00000080) + 1] = "NEWTIME",
>       [32 + 1] = "CLEAR_SIGHAND",
>       [33 + 1] = "INTO_CGROUP",
>       [34 + 1] = "AUTOREAP",
>       [35 + 1] = "NNP",
>       [36 + 1] = "PIDFD_AUTOKILL",
>       [37 + 1] = "EMPTY_MNTNS",
> };

Does this cause undefined behavior on 32-bit platforms?

The generated array now includes entries for bits up to 37. When perf trace
consumes this array in strarray__scnprintf_flags(), it computes the bit
mask using an unsigned long shift:

    unsigned long bit = 1UL << (i - 1);

On 32-bit architectures, unsigned long is 32 bits. Shifting by 32 or more
is undefined behavior and often wraps the shift count. This could cause
standard 32-bit clone syscalls to falsely match the new 64-bit flag names
if the corresponding wrapped lower bit is set, corrupting the tool's output.

> diff --git a/tools/perf/trace/beauty/clone.sh b/tools/perf/trace/beauty/clone.sh
> index 18b6c0d756937..98cb1f8d4a6ff 100755
> --- a/tools/perf/trace/beauty/clone.sh
> +++ b/tools/perf/trace/beauty/clone.sh
> @@ -14,4 +14,8 @@ regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+CLONE_([^_]+[[:alnum:]_]+)[[:
>  grep -E $regex ${linux_sched} | \
>  	sed -r "s/$regex/\2 \1/g"	| \
>  	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n"
> +regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+CLONE_([^_]+[[:alnum:]_]+)[[:space:]]+\(1ULL[[:space:]]*<<[[:space:]]*([[:digit:]]+)\)[[:space:]]*.*'
> +grep -E $regex ${linux_sched} | \
> +	sed -r "s/$regex/\2 \1/g"	| \
> +	xargs printf "\t[%s + 1] = \"%s\",\n"

Should this use xargs -r? 

If the script is run against older kernel headers that lack the new clone3
flags using the 1ULL shift pattern, grep will yield no output. However,
xargs will still execute printf once with empty arguments.

This results in a generated entry of:

    [ + 1] = "",

which compiles as index 1 (mapping to bit 0) and could cause perf trace
to print confusing empty suffixes for syscalls that set bit 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510202346.49853-1-namhyung@kernel.org?part=7