Re: [PATCH v12 01/40] selftests/dyndbg: Add kselftest script to verify dynamic-debug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests: test flakiness from unfiltered dmesg temporal slicing
- [Medium] selftests: test failure coupled to unrelated kernel/params.c pr_debug churn
--

commit 56bb89808ed29ee403a2b674106329de27cb9b06
Author: Jim Cromie <jim.cromie@gmail.com>

selftests/dyndbg: Add kselftest script to verify dynamic-debug

This commit introduces a dynamic-debug selftest script and a reusable helper
library for baseline-change-verify testing. It uses spatial control-file
slicing and temporal syslog slicing to verify valid syntax, error handling,
and basic queries against the built-in kernel/params.c file.

> diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> new file mode 100755
> index 0000000000000..c1475c7555429
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh

[ ... ]

> +function FT_basic_queries {
> +    v_echo "${GREEN}# BASIC_TESTS ${NC}"
> +    if [ $LACK_DD_BUILTIN -eq 1 ]; then
> +	echo "SKIP - test requires dynamic_debug built into kernel"
> +	return
> +    fi
> +    local f='kernel/params.c'
> +    ddcmd =_ # zero everything
> +
> +    ddcmd "file $f +mf" "$f"

[Severity: Medium]
By strictly validating the exact state of kernel/params.c and hashing the
normalized text, won't this test break if an unrelated upstream commit
legitimately adds or removes a pr_debug() in kernel/params.c? Does this
inadvertently couple the selftest to unrelated core kernel changes?

> +    ddcmd "file $f +l"  "$f"
> +    ddcmd "file $f -m"  "$f"
> +    ddcmd "file $f =_"  "$f"

[ ... ]

> diff --git a/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh b/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh
> new file mode 100644
> index 0000000000000..8c6e91f5d9c27
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh

[ ... ]

> +function verify_dmesg_slice {
> +    # Slices dmesg, computes its hash, and verifies it against the database.
> +    # $1 - unique test key (e.g. normal_513)
> +    # $2 - optional start marker (defaults to ${APP}_START_${label})
> +    # $3 - optional end marker (defaults to ${APP}_END_${label})
> +
> +    local label="$1"
> +    local app="${APP:-DYNDBG}"
> +    local start_marker="${2:-${app}_START_${label}_$$}"
> +    local end_marker="${3:-${app}_END_${label}_$$}"
> +
> +    # 1. Capture the log slice (exactly once!)
> +    local log_slice=$(dmesg | sed -n "/$start_marker/,/$end_marker/p" | \
> +	grep -E -v "$start_marker|$end_marker" | \
> +        sed -E -e 's/^(\[[^]]*\][[:space:]]*)+//' )

[Severity: Medium]
Since verify_dmesg_slice captures all kernel logs between test markers and
hashes the entire block without filtering for test-specific messages, could
concurrent unrelated kernel subsystems (like networking or RCU) emit printk
logs that alter the MD5 hash and cause false positive test failures?

> +
> +    # 2. Compute its fingerprint
> +    local fingerprint=$(echo "$log_slice" | tr -d '\r' | md5sum | cut -d' ' -f1)
> +
> +    # 3. Verify
> +    verify_fingerprint "$label" "$fingerprint" "Dmesg Log" "$log_slice"
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-dd-cmap-part2-clean-v12-0-3c8cc5725def@gmail.com?part=1