Re: [PATCH v2 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:
- [High] The beautifier script regex update drops all legacy clone flags (0-31), breaking flag decoding for standard `clone()` syscalls.
- [High] Processing clone flags with indices >= 32 in `strarray__scnprintf_flags()` causes Undefined Behavior on 32-bit platforms, leading to incorrect flag decoding.
--

commit 47293f130dab2e91781452bd39a472811835f8a9
Author: Namhyung Kim <namhyung@kernel.org>

perf trace: Update beautifier script for clone flags

This commit updates the clone_flags beautifier script to parse the new bit
shift definitions in sched.h. It changes the regex to match the 1ULL bit
shift pattern instead of the previous hexadecimal values.

>   $ cat tools/perf/trace/beauty/generated/clone_flags_array.c
>   static const char *clone_flags[] = {
>         [32 + 1] = "CLEAR_SIGHAND",
>         [33 + 1] = "INTO_CGROUP",
>         [34 + 1] = "AUTOREAP",
>         [35 + 1] = "NNP",
>         [36 + 1] = "PIDFD_AUTOKILL",
>         [37 + 1] = "EMPTY_MNTNS",
>   };

When the array has entries up to index 38, does this cause an issue on 32-bit
builds of perf?

Looking at strarray__scnprintf_flags(), it computes the flag bit using:

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

On 32-bit platforms, unsigned long is 32 bits. Shifting by 32 or more invokes
undefined behavior. On architectures like x86, the shift amount is masked
(e.g., modulo 32), so 1UL << 32 evaluates as 1UL << 0.

Could this cause perf trace to erroneously report clone3 flags like
CLEAR_SIGHAND whenever unrelated lower bits are set?

> diff --git a/tools/perf/trace/beauty/clone.sh b/tools/perf/trace/beauty/clone.sh
> index 18b6c0d756937..f807372519fe2 100755
> --- a/tools/perf/trace/beauty/clone.sh
> +++ b/tools/perf/trace/beauty/clone.sh

[ ... ]

>  printf "static const char *clone_flags[] = {\n"
> -regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+CLONE_([^_]+[[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> +regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+CLONE_([^_]+[[:alnum:]_]+)[[:space:]]+\(1ULL[[:space:]]*<<[[:space:]]*([[:digit:]]+)\)[[:space:]]*.*'

Does this new regex miss the legacy clone flags?

In include/uapi/linux/sched.h, legacy flags like CLONE_VM, CLONE_FS, and
CLONE_THREAD are still defined using hexadecimal notation.

Since the updated regex strictly requires the (1ULL << digit) format,
it appears all lower 32 flags are excluded from the generated
clone_flags_array.c.

Could this prevent perf trace from translating standard clone flags into
human-readable strings?

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