Re: [PATCH v3 5/7] perf test: Enhance annotate test coverage and isolate config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch adds a test for a non-existent 'libasm' disassembler backend and feature, resulting in permanently skipped dead code due to AI hallucination.
--

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

perf test: Enhance annotate test coverage and isolate config

This commit updates the annotate.sh test to verify different disassembler 
backends like objdump, llvm, and capstone. It also isolates the test script 
from host environment pollution by exporting PERF_CONFIG=/dev/null.

> diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/annotate.sh
> index 689de58e92384..eae6a46a247b4 100755
> --- a/tools/perf/tests/shell/annotate.sh
> +++ b/tools/perf/tests/shell/annotate.sh
[ ... ]
> @@ -106,8 +108,66 @@ test_basic() {
[ ... ]
> +# Restore perfdata to a regular format for disassembler tests
> +perf record -o "${perfdata}" ${testprog} > /dev/null 2>&1
> +
> +if [ "${err}" -eq 0 ]
> +then
> +  test_disassembler "objdump" ""
> +  test_disassembler "llvm" "libLLVM"
> +  test_disassembler "capstone" "libcapstone"
> +  test_disassembler "libasm" "libasm"

[Severity: Low]
Is "libasm" a supported feature in perf?

It does not appear to be defined in the supported_features array or as a valid
disassembler backend in perf. Because of this, the check in test_disassembler():

    if ! perf check feature "${feature}" > /dev/null 2>&1
    then
      echo "Skip test for ${disassembler} (feature ${feature} not supported)"
      return 0
    fi

will always fail, causing this subtest to silently print the skip message and
return 0 on every execution. 

Should this test case be removed?

> +fi
> +
>  cleanup
>  exit $err

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609182108.975586-1-irogers@google.com?part=5