Re: [PATCH] perf test: Add a new test for perf annotate

[Namhyung Kim]

On Wed, Apr 24, 2024 at 2:02 PM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> On Wed, Apr 24, 2024 at 11:09:48AM -0700, Ian Rogers wrote:
> > On Tue, Apr 23, 2024 at 5:12 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> > >
> > > Add a basic perf annotate test
> > >
> > >   $ ./perf test annotate -vv
> > >    76: perf annotate basic tests:
> > >   --- start ---
> > >   test child forked, pid 846989
> > >    fbcd0-fbd55 l noploop
> > >   perf does have symbol 'noploop'
> > >   Basic perf annotate test
> > >            : 0     0xfbcd0 <noploop>:
> > >       0.00 :   fbcd0:       pushq   %rbp
> > >       0.00 :   fbcd1:       movq    %rsp, %rbp
> > >       0.00 :   fbcd4:       pushq   %r12
> > >       0.00 :   fbcd6:       pushq   %rbx
> > >       0.00 :   fbcd7:       movl    $1, %ebx
> > >       0.00 :   fbcdc:       subq    $0x10, %rsp
> > >       0.00 :   fbce0:       movq    %fs:0x28, %rax
> > >       0.00 :   fbce9:       movq    %rax, -0x18(%rbp)
> > >       0.00 :   fbced:       xorl    %eax, %eax
> > >       0.00 :   fbcef:       testl   %edi, %edi
> > >       0.00 :   fbcf1:       jle     0xfbd04
> > >       0.00 :   fbcf3:       movq    (%rsi), %rdi
> > >       0.00 :   fbcf6:       movl    $0xa, %edx
> > >       0.00 :   fbcfb:       xorl    %esi, %esi
> > >       0.00 :   fbcfd:       callq   0x41920
> > >       0.00 :   fbd02:       movl    %eax, %ebx
> > >       0.00 :   fbd04:       leaq    -0x7b(%rip), %r12   # fbc90 <sighandler>
> > >       0.00 :   fbd0b:       movl    $2, %edi
> > >       0.00 :   fbd10:       movq    %r12, %rsi
> > >       0.00 :   fbd13:       callq   0x40a00
> > >       0.00 :   fbd18:       movl    $0xe, %edi
> > >       0.00 :   fbd1d:       movq    %r12, %rsi
> > >       0.00 :   fbd20:       callq   0x40a00
> > >       0.00 :   fbd25:       movl    %ebx, %edi
> > >       0.00 :   fbd27:       callq   0x407c0
> > >       0.10 :   fbd2c:       movl    0x89785e(%rip), %eax        # 993590 <done>
> > >       0.00 :   fbd32:       testl   %eax, %eax
> > >      99.90 :   fbd34:       je      0xfbd2c
> > >       0.00 :   fbd36:       movq    -0x18(%rbp), %rax
> > >       0.00 :   fbd3a:       subq    %fs:0x28, %rax
> > >       0.00 :   fbd43:       jne     0xfbd50
> > >       0.00 :   fbd45:       addq    $0x10, %rsp
> > >       0.00 :   fbd49:       xorl    %eax, %eax
> > >       0.00 :   fbd4b:       popq    %rbx
> > >       0.00 :   fbd4c:       popq    %r12
> > >       0.00 :   fbd4e:       popq    %rbp
> > >       0.00 :   fbd4f:       retq
> > >       0.00 :   fbd50:       callq   0x407e0
> > >       0.00 :   fbcd0:       pushq   %rbp
> > >       0.00 :   fbcd1:       movq    %rsp, %rbp
> > >       0.00 :   fbcd4:       pushq   %r12
> > >       0.00 :   fbcd0:  push   %rbp
> > >       0.00 :   fbcd1:  mov    %rsp,%rbp
> > >       0.00 :   fbcd4:  push   %r12
> > >   Basic annotate test [Success]
> > >   ---- end(0) ----
> > >    76: perf annotate basic tests                                       : Ok
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> >
> > Looks good, thanks for this!
> >
> > Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Thanks! Applied, with the following changes to improve the error
> reporting, please holler if you disagree:

LGTM!

Thanks,
Namhyung

>
> diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/annotate.sh
> index 7820d13eebaef535..1db1e8113d9943a6 100755
> --- a/tools/perf/tests/shell/annotate.sh
> +++ b/tools/perf/tests/shell/annotate.sh
> @@ -36,7 +36,7 @@ test_basic() {
>    echo "Basic perf annotate test"
>    if ! perf record -o "${perfdata}" ${testprog} 2> /dev/null
>    then
> -    echo "Basic annotate [Failed record]"
> +    echo "Basic annotate [Failed: perf record]"
>      err=1
>      return
>    fi
> @@ -44,7 +44,7 @@ test_basic() {
>    # check if it has the target symbol
>    if ! perf annotate -i "${perfdata}" 2> /dev/null | grep "${testsym}"
>    then
> -    echo "Basic annotate [Failed missing symbol]"
> +    echo "Basic annotate [Failed: missing target symbol]"
>      err=1
>      return
>    fi
> @@ -52,7 +52,7 @@ test_basic() {
>    # check if it has the disassembly lines
>    if ! perf annotate -i "${perfdata}" 2> /dev/null | grep "${disasm_regex}"
>    then
> -    echo "Basic annotate [Failed missing disasm output]"
> +    echo "Basic annotate [Failed: missing disasm output from default disassembler]"
>      err=1
>      return
>    fi
> @@ -61,7 +61,7 @@ test_basic() {
>    if ! perf annotate -i "${perfdata}" "${testsym}" 2> /dev/null | \
>           grep -m 3 "${disasm_regex}"
>    then
> -    echo "Basic annotate [Failed missing disasm output]"
> +    echo "Basic annotate [Failed: missing disasm output when specifying the target symbol]"
>      err=1
>      return
>    fi
> @@ -70,7 +70,7 @@ test_basic() {
>    if ! perf annotate -i "${perfdata}" --objdump=objdump 2> /dev/null | \
>           grep -m 3 "${disasm_regex}"
>    then
> -    echo "Basic annotate [Failed missing disasm output from objdump]"
> +    echo "Basic annotate [Failed: missing disasm output from non default disassembler (using --objdump)]"
>      err=1
>      return
>    fi