[PATCH v2] perf check: Add sanitizer feature and use to avoid test failure

Ian Rogers posted 1 patch 1 month, 1 week ago
tools/perf/builtin-check.c         | 49 ++++++++++++++++++++++++++++++
tools/perf/tests/shell/annotate.sh |  6 ++++
2 files changed, 55 insertions(+)
[PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Ian Rogers 1 month, 1 week ago
Sanitizer builds can break expectations for test disassembly,
particularly in the annotate test. Add features for the different
sanitizer options seen in the source tree. Use the added sanitizer
feature to skip the annotate test when sanitizers are in use.

Signed-off-by: Ian Rogers <irogers@google.com>
---
v2 build fix.
---
 tools/perf/builtin-check.c         | 49 ++++++++++++++++++++++++++++++
 tools/perf/tests/shell/annotate.sh |  6 ++++
 2 files changed, 55 insertions(+)

diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
index 0b76b6e42b78..c44444008d64 100644
--- a/tools/perf/builtin-check.c
+++ b/tools/perf/builtin-check.c
@@ -9,6 +9,49 @@
 #include <string.h>
 #include <subcmd/parse-options.h>
 
+#if defined(__has_feature)
+#define HAS_COMPILER_FEATURE(feature) __has_feature(feature)
+#else
+#define HAS_COMPILER_FEATURE(feature) 0
+#endif
+
+#if defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER) || \
+	HAS_COMPILER_FEATURE(address_sanitizer)
+#define HAVE_SANITIZER_ADDRESS 1
+#define HAVE_SANITIZER_LEAK 1
+#elif defined(LEAK_SANITIZER) || HAS_COMPILER_FEATURE(leak_sanitizer)
+#define HAVE_SANITIZER_ADDRESS 0
+#define HAVE_SANITIZER_LEAK 1
+#else
+#define HAVE_SANITIZER_ADDRESS 0
+#define HAVE_SANITIZER_LEAK 0
+#endif
+
+#if defined(MEMORY_SANITIZER) || HAS_COMPILER_FEATURE(memory_sanitizer)
+#define HAVE_SANITIZER_MEMORY 1
+#else
+#define HAVE_SANITIZER_MEMORY 0
+#endif
+
+#if defined(THREAD_SANITIZER) || HAS_COMPILER_FEATURE(thread_sanitizer)
+#define HAVE_SANITIZER_THREAD 1
+#else
+#define HAVE_SANITIZER_THREAD 0
+#endif
+
+#if defined(UNDEFINED_SANITIZER) || HAS_COMPILER_FEATURE(undefined_sanitizer)
+#define HAVE_SANITIZER_UNDEFINED 1
+#else
+#define HAVE_SANITIZER_UNDEFINED 0
+#endif
+
+#if HAVE_SANITIZER_ADDRESS || HAVE_SANITIZER_LEAK || HAVE_SANITIZER_MEMORY || \
+	HAVE_SANITIZER_THREAD || HAVE_SANITIZER_UNDEFINED
+#define HAVE_SANITIZER 1
+#else
+#define HAVE_SANITIZER 0
+#endif
+
 static const char * const check_subcommands[] = { "feature", NULL };
 static struct option check_options[] = {
 	OPT_BOOLEAN('q', "quiet", &quiet, "do not show any warnings or messages"),
@@ -47,6 +90,12 @@ struct feature_status supported_features[] = {
 	FEATURE_STATUS("libunwind", HAVE_LIBUNWIND_SUPPORT),
 	FEATURE_STATUS("lzma", HAVE_LZMA_SUPPORT),
 	FEATURE_STATUS("numa_num_possible_cpus", HAVE_LIBNUMA_SUPPORT),
+	FEATURE_STATUS("sanitizer", HAVE_SANITIZER),
+	FEATURE_STATUS("sanitizer_address", HAVE_SANITIZER_ADDRESS),
+	FEATURE_STATUS("sanitizer_leak", HAVE_SANITIZER_LEAK),
+	FEATURE_STATUS("sanitizer_memory", HAVE_SANITIZER_MEMORY),
+	FEATURE_STATUS("sanitizer_thread", HAVE_SANITIZER_THREAD),
+	FEATURE_STATUS("sanitizer_undefined", HAVE_SANITIZER_UNDEFINED),
 	FEATURE_STATUS("syscall_table", HAVE_SYSCALL_TABLE_SUPPORT),
 	FEATURE_STATUS("zlib", HAVE_ZLIB_SUPPORT),
 	FEATURE_STATUS("zstd", HAVE_ZSTD_SUPPORT),
diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/annotate.sh
index 1590a37363de..199f547e656d 100755
--- a/tools/perf/tests/shell/annotate.sh
+++ b/tools/perf/tests/shell/annotate.sh
@@ -4,6 +4,12 @@
 
 set -e
 
+if perf check feature -q sanitizer
+then
+  echo "Skip test with sanitizers due to differing assembly code"
+  exit 2
+fi
+
 shelldir=$(dirname "$0")
 
 # shellcheck source=lib/perf_has_symbol.sh
-- 
2.47.0.105.g07ac214952-goog
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Namhyung Kim 1 month ago
On Thu, Oct 17, 2024 at 10:56:27PM -0700, Ian Rogers wrote:
> Sanitizer builds can break expectations for test disassembly,
> particularly in the annotate test. Add features for the different
> sanitizer options seen in the source tree. Use the added sanitizer
> feature to skip the annotate test when sanitizers are in use.

Can you please split the perf check changes from the test change?
It's good to add an example output (of perf version --build-options)
with new sanitizer features.

Also it might be helpful if you share how the test fails.  I don't
think the disasm format is changed.  Then it probably missed to find
the target symbol in the first 250 lines for some reason.

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2 build fix.
> ---
>  tools/perf/builtin-check.c         | 49 ++++++++++++++++++++++++++++++
>  tools/perf/tests/shell/annotate.sh |  6 ++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
> index 0b76b6e42b78..c44444008d64 100644
> --- a/tools/perf/builtin-check.c
> +++ b/tools/perf/builtin-check.c
> @@ -9,6 +9,49 @@
>  #include <string.h>
>  #include <subcmd/parse-options.h>
>  
> +#if defined(__has_feature)
> +#define HAS_COMPILER_FEATURE(feature) __has_feature(feature)
> +#else
> +#define HAS_COMPILER_FEATURE(feature) 0
> +#endif
> +
> +#if defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER) || \
> +	HAS_COMPILER_FEATURE(address_sanitizer)
> +#define HAVE_SANITIZER_ADDRESS 1
> +#define HAVE_SANITIZER_LEAK 1
> +#elif defined(LEAK_SANITIZER) || HAS_COMPILER_FEATURE(leak_sanitizer)
> +#define HAVE_SANITIZER_ADDRESS 0
> +#define HAVE_SANITIZER_LEAK 1
> +#else
> +#define HAVE_SANITIZER_ADDRESS 0
> +#define HAVE_SANITIZER_LEAK 0
> +#endif
> +
> +#if defined(MEMORY_SANITIZER) || HAS_COMPILER_FEATURE(memory_sanitizer)
> +#define HAVE_SANITIZER_MEMORY 1
> +#else
> +#define HAVE_SANITIZER_MEMORY 0
> +#endif
> +
> +#if defined(THREAD_SANITIZER) || HAS_COMPILER_FEATURE(thread_sanitizer)
> +#define HAVE_SANITIZER_THREAD 1
> +#else
> +#define HAVE_SANITIZER_THREAD 0
> +#endif
> +
> +#if defined(UNDEFINED_SANITIZER) || HAS_COMPILER_FEATURE(undefined_sanitizer)
> +#define HAVE_SANITIZER_UNDEFINED 1
> +#else
> +#define HAVE_SANITIZER_UNDEFINED 0
> +#endif
> +
> +#if HAVE_SANITIZER_ADDRESS || HAVE_SANITIZER_LEAK || HAVE_SANITIZER_MEMORY || \
> +	HAVE_SANITIZER_THREAD || HAVE_SANITIZER_UNDEFINED
> +#define HAVE_SANITIZER 1
> +#else
> +#define HAVE_SANITIZER 0
> +#endif
> +
>  static const char * const check_subcommands[] = { "feature", NULL };
>  static struct option check_options[] = {
>  	OPT_BOOLEAN('q', "quiet", &quiet, "do not show any warnings or messages"),
> @@ -47,6 +90,12 @@ struct feature_status supported_features[] = {
>  	FEATURE_STATUS("libunwind", HAVE_LIBUNWIND_SUPPORT),
>  	FEATURE_STATUS("lzma", HAVE_LZMA_SUPPORT),
>  	FEATURE_STATUS("numa_num_possible_cpus", HAVE_LIBNUMA_SUPPORT),
> +	FEATURE_STATUS("sanitizer", HAVE_SANITIZER),
> +	FEATURE_STATUS("sanitizer_address", HAVE_SANITIZER_ADDRESS),
> +	FEATURE_STATUS("sanitizer_leak", HAVE_SANITIZER_LEAK),
> +	FEATURE_STATUS("sanitizer_memory", HAVE_SANITIZER_MEMORY),
> +	FEATURE_STATUS("sanitizer_thread", HAVE_SANITIZER_THREAD),
> +	FEATURE_STATUS("sanitizer_undefined", HAVE_SANITIZER_UNDEFINED),
>  	FEATURE_STATUS("syscall_table", HAVE_SYSCALL_TABLE_SUPPORT),
>  	FEATURE_STATUS("zlib", HAVE_ZLIB_SUPPORT),
>  	FEATURE_STATUS("zstd", HAVE_ZSTD_SUPPORT),
> diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/annotate.sh
> index 1590a37363de..199f547e656d 100755
> --- a/tools/perf/tests/shell/annotate.sh
> +++ b/tools/perf/tests/shell/annotate.sh
> @@ -4,6 +4,12 @@
>  
>  set -e
>  
> +if perf check feature -q sanitizer
> +then
> +  echo "Skip test with sanitizers due to differing assembly code"
> +  exit 2
> +fi
> +
>  shelldir=$(dirname "$0")
>  
>  # shellcheck source=lib/perf_has_symbol.sh
> -- 
> 2.47.0.105.g07ac214952-goog
>
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Ian Rogers 1 month ago
On Mon, Oct 21, 2024 at 10:41 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Oct 17, 2024 at 10:56:27PM -0700, Ian Rogers wrote:
> > Sanitizer builds can break expectations for test disassembly,
> > particularly in the annotate test. Add features for the different
> > sanitizer options seen in the source tree. Use the added sanitizer
> > feature to skip the annotate test when sanitizers are in use.
>
> Can you please split the perf check changes from the test change?
> It's good to add an example output (of perf version --build-options)
> with new sanitizer features.

Okay, will split/add in v3.

> Also it might be helpful if you share how the test fails.  I don't
> think the disasm format is changed.  Then it probably missed to find
> the target symbol in the first 250 lines for some reason.

Sure, the reproduction is trivial, just add -fsanitize=address, so I'm
surprised you're not already seeing it:
```
$ perf test annotate -v
--- start ---
test child forked, pid 444258
 68e8a0-68e96b l noploop
perf does have symbol 'noploop'
Basic perf annotate test
         : 0      0x68e8a0 <noploop>:
    0.00 :   68e8a0:       pushq   %rbp
    0.00 :   68e8a1:       movq    %rsp, %rbp
    0.00 :   68e8a4:       subq    $0x30, %rsp
    0.00 :   68e8a8:       movq    %fs:0x28, %rax
    0.00 :   68e8b1:       movq    %rax, -8(%rbp)
    0.00 :   68e8b5:       movl    %edi, -0xc(%rbp)
    0.00 :   68e8b8:       movq    %rsi, -0x18(%rbp)
    0.00 :   68e8bc:       movl    $1, -0x1c(%rbp)
    0.00 :   68e8c3:       cmpl    $0, -0xc(%rbp)
    0.00 :   68e8c7:       jle     0x68e8fe
    0.00 :   68e8cd:       movq    -0x18(%rbp), %rax
    0.00 :   68e8d1:       movq    %rax, -0x28(%rbp)
    0.00 :   68e8d5:       shrq    $3, %rax
    0.00 :   68e8d9:       cmpb    $0, 0x7fff8000(%rax)
    0.00 :   68e8e0:       je      0x68e8ef
    0.00 :   68e8e6:       movq    -0x28(%rbp), %rdi
    0.00 :   68e8ea:       callq   0x2adea0
    0.00 :   68e8ef:       movq    -0x28(%rbp), %rax
    0.00 :   68e8f3:       movq    (%rax), %rdi
    0.00 :   68e8f6:       callq   0x28fac0
    0.00 :   68e8fb:       movl    %eax, -0x1c(%rbp)
    0.00 :   68e8fe:       movl    $2, %edi
    0.00 :   68e903:       leaq    0x66(%rip), %rsi     # 68e970 <sighandler>
    0.00 :   68e90a:       callq   0x27e650
    0.00 :   68e90f:       movl    $0xe, %edi
    0.00 :   68e914:       leaq    0x55(%rip), %rsi     # 68e970 <sighandler>
    0.00 :   68e91b:       callq   0x27e650
    0.00 :   68e920:       movl    -0x1c(%rbp), %edi
    0.00 :   68e923:       callq   0x2086e0
    0.03 :   68e928:       movl    0x18aca72(%rip), %eax        #
1f3b3a0 <buildid_dir+0xa0>
    0.00 :   68e92e:       cmpl    $0, %eax
    0.00 :   68e931:       setne   %al
   50.02 :   68e934:       xorb    $0xff, %al
    0.00 :   68e936:       testb   $1, %al
   41.28 :   68e938:       jne     0x68e943
    0.00 :   68e93e:       jmp     0x68e948
    8.67 :   68e943:       jmp     0x68e928
    0.00 :   68e948:       movq    %fs:0x28, %rax
    0.00 :   68e951:       movq    -8(%rbp), %rcx
    0.00 :   68e955:       cmpq    %rcx, %rax
    0.00 :   68e958:       jne     0x68e966
    0.00 :   68e95e:       xorl    %eax, %eax
    0.00 :   68e960:       addq    $0x30, %rsp
    0.00 :   68e964:       popq    %rbp
    0.00 :   68e965:       retq
    0.00 :   68e966:       callq   0x2086f0
    0.00 :   2d6910:       endbr64
    0.00 :   2d6914:       pushq   %rbp
    0.00 :   2d6915:       pushq   %r15
    0.00 :   2d6917:       pushq   %r14
    0.00 :   2d6919:       pushq   %r13
    0.00 :   2d691b:       pushq   %r12
    0.00 :   2d691d:       pushq   %rbx
    0.00 :   2d691e:       subq    $0x48, %rsp
    0.00 :   2d6922:       movq    %rdx, 0x20(%rsp)
    0.00 :   2d6927:       movq    %r8, 0x30(%rsp)
    0.00 :   2d692c:       leal    -1(%r8), %eax
    0.00 :   2d6930:       cmpl    $2, %eax
    0.00 :   2d6933:       jae     0x2d6b2c
    0.00 :   2d6939:       movq    %rsi, %r9
    0.00 :   2d693c:       movq    %rdi, %r12
    0.00 :   2d693f:       cmpb    $0, 0x1c5f99c(%rip)  # 1f362e2
<cpu_bitmap+0x122>
    0.00 :   2d6946:       movl    $8, %eax
    0.00 :   2d694b:       movl    $1, %ebx
    0.00 :   2d6950:       cmoveq  %rax, %rbx
    0.00 :   2d6954:       cmpb    $0, 0x1c5f989(%rip)  # 1f362e4
<cpu_bitmap+0x124>
    0.00 :   2d695b:       movq    %rsi, 0x28(%rsp)
    0.00 :   2d6960:       je      0x2d697e
    0.00 :   2d6962:       leaq    0x8e9708(%rip), %rdi
    0.00 :   2d6969:       movq    %rcx, %rsi
    0.00 :   2d696c:       movq    %r12, %rdx
    0.00 :   2d696f:       movq    %r9, %rcx
    0.00 :   2d6972:       xorl    %eax, %eax
    0.00 :   2d6974:       callq   0x2c18c0
    0.00 :   2d6979:       movq    0x28(%rsp), %r9
    0.00 :   2d697e:       leaq    -1(%rbx), %rax
    0.00 :   2d6982:       andq    %r12, %rax
    0.00 :   2d6985:       leaq    (%rbx, %r12), %r13
    0.00 :   2d6989:       subq    %rax, %r13
    0.00 :   2d698c:       testq   %rax, %rax
    0.00 :   2d698f:       cmoveq  %r12, %r13
    0.00 :   2d6993:       leaq    8(%r13), %rax
    0.00 :   2d6997:       cmpq    %r9, %rax
    0.00 :   2d699a:       jbe     0x2d69ab
    0.00 :   2d699c:       addq    $0x48, %rsp
    0.00 :   2d69a0:       popq    %rbx
    0.00 :   2d69a1:       popq    %r12
    0.00 :   2d69a3:       popq    %r13
    0.00 :   2d69a5:       popq    %r14
    0.00 :   2d69a7:       popq    %r15
    0.00 :   2d69a9:       popq    %rbp
    0.00 :   2d69aa:       retq
    0.00 :   2d69ab:       movabsq $0x7fffffffbfff, %r15
    0.00 :   2d69b5:       leaq    0x40(%rsp), %rbp
    0.00 :   2d69ba:       movq    %r12, 0x38(%rsp)
    0.00 :   2d69bf:       jmp     0x2d69ea
    0.00 :   2d69c1:       nopw    %cs:(%rax, %rax)
    0.00 :   2d69d0:       movq    0x38(%rsp), %r12
    0.00 :   2d69d5:       movq    0x28(%rsp), %r9
    0.00 :   2d69da:       leaq    (%rbx, %r13), %rax
    0.00 :   2d69de:       addq    $8, %rax
    0.00 :   2d69e2:       addq    %rbx, %r13
    0.00 :   2d69e5:       cmpq    %r9, %rax
    0.00 :   2d69e8:       ja      0x2d699c
    0.00 :   2d69ea:       movq    (%r13), %r14
    0.00 :   2d69ee:       leaq    -0x4000(%r14), %rax
    0.00 :   2d69f5:       cmpq    %r15, %rax
   55.45 :   2d69f8:       ja      0x2d69da
    0.00 :   2d69fa:       movq    %r14, %rdi
   44.55 :   2d69fd:       callq   0x20df90
    0.00 :   2d6a02:       movq    %rax, 0x18(%rsp)
    0.00 :   2d6a07:       testq   %rax, %rax
    0.00 :   2d6a0a:       je      0x2d69d5
    0.00 :   2d6a0c:       cmpq    %r12, %rax
    0.00 :   2d6a0f:       je      0x2d69d5
    0.00 :   2d6a11:       movq    %rbp, %r12
    0.00 :   2d6a14:       movq    %rbp, %rdi
    0.00 :   2d6a17:       movq    %rax, %rsi
    0.00 :   2d6a1a:       callq   0x20e190
    0.00 :   2d6a1f:       movq    %rbp, %rdi
    0.00 :   2d6a22:       callq   0x20e1d0
    0.00 :   2d6a27:       cmpl    $2, %eax
    0.00 :   2d6a2a:       je      0x2d69d0
    0.00 :   2d6a2c:       movq    %rbp, %rdi
    0.00 :   2d6a2f:       callq   0x20e1d0
    0.00 :   2d6a34:       cmpl    $3, %eax
    0.00 :   2d6a37:       je      0x2d69d0
    0.00 :   2d6a39:       cmpb    $0, 0x1c5f8a3(%rip)  # 1f362e3
<cpu_bitmap+0x123>
    0.00 :   2d6a40:       jne     0x2d6aab
    0.00 :   2d6a42:       movq    %r13, %rdi
    0.00 :   2d6a45:       callq   0x2a9030
    0.00 :   2d6a4a:       testb   %al, %al
    0.00 :   2d6a4c:       je      0x2d6aab
    0.00 :   2d6a4e:       cmpb    $0, 0x1c5f88f(%rip)  # 1f362e4
<cpu_bitmap+0x124>
    0.00 :   2d6a55:       je      0x2d69d0
    0.00 :   2d6a5b:       movq    0x18(%rsp), %rax
    0.00 :   2d6a60:       movq    %rax, 8(%rsp)
    0.00 :   2d6a65:       movq    %rbp, %r12
    0.00 :   2d6a68:       movq    %rbp, %rdi
    0.00 :   2d6a6b:       callq   0x20e210
    0.00 :   2d6a70:       movq    8(%rsp), %rcx
    0.00 :   2d6a75:       addq    %rcx, %rax
    0.00 :   2d6a78:       movq    %rax, 0x10(%rsp)
    0.00 :   2d6a7d:       movq    %rbp, %rdi
    0.00 :   2d6a80:       callq   0x20e210
    0.00 :   2d6a85:       leaq    0x8e95ff(%rip), %rdi
    0.00 :   2d6a8c:       movq    %r13, %rsi
    0.00 :   2d6a8f:       movq    %r14, %rdx
    0.00 :   2d6a92:       movq    8(%rsp), %rcx
    0.00 :   2d6a97:       movq    0x10(%rsp), %r8
    0.00 :   2d6a9c:       movq    %rax, %r9
    0.00 :   2d6a9f:       xorl    %eax, %eax
    0.00 :   2d6aa1:       callq   0x2c18c0
    0.00 :   2d6aa6:       jmp     0x2d69d0
    0.00 :   2d6aab:       movq    %rbp, %r12
    0.00 :   2d6aae:       movq    %rbp, %rdi
    0.00 :   2d6ab1:       movq    0x30(%rsp), %rsi
    0.00 :   2d6ab6:       callq   0x20e1f0
    0.00 :   2d6abb:       cmpb    $0, 0x1c5f822(%rip)  # 1f362e4
<cpu_bitmap+0x124>
    0.00 :   2d6ac2:       je      0x2d6b0c
    0.00 :   2d6ac4:       movq    0x18(%rsp), %rax
    0.00 :   2d6ac9:       movq    %rax, 8(%rsp)
    0.00 :   2d6ace:       movq    %r12, %rdi
    0.00 :   2d6ad1:       callq   0x20e210
    0.00 :   2d6ad6:       movq    8(%rsp), %rcx
    0.00 :   2d6adb:       addq    %rcx, %rax
    0.00 :   2d6ade:       movq    %rax, 0x10(%rsp)
    0.00 :   2d6ae3:       movq    %r12, %rdi
    0.00 :   2d6ae6:       callq   0x20e210
    0.00 :   2d6aeb:       leaq    0x8e95dd(%rip), %rdi
    0.00 :   2d6af2:       movq    %r13, %rsi
    0.00 :   2d6af5:       movq    %r14, %rdx
    0.00 :   2d6af8:       movq    8(%rsp), %rcx
    0.00 :   2d6afd:       movq    0x10(%rsp), %r8
    0.00 :   2d6b02:       movq    %rax, %r9
    0.00 :   2d6b05:       xorl    %eax, %eax
    0.00 :   2d6b07:       callq   0x2c18c0
    0.00 :   2d6b0c:       cmpq    $0, 0x20(%rsp)
    0.00 :   2d6b12:       je      0x2d69d0
    0.00 :   2d6b18:       movq    0x20(%rsp), %rdi
    0.00 :   2d6b1d:       leaq    0x18(%rsp), %rsi
    0.00 :   2d6b22:       callq   0x2b45a0
    0.00 :   2d6b27:       jmp     0x2d69d0
    0.00 :   2d6b2c:       leaq    0x8e94cf(%rip), %rdi
    0.00 :   2d6b33:       leaq    0x8e94fe(%rip), %rdx
    0.00 :   2d6b3a:       movl    $0x128, %esi
    0.00 :   2d6b3f:       xorl    %ecx, %ecx
    0.00 :   2d6b41:       xorl    %r8d, %r8d
    0.00 :   2d6b44:       callq   0x2c77f0
    0.00 :   9200:       pushq   %r15
    0.00 :   9202:       pushq   %r14
    0.00 :   9204:       pushq   %r13
    0.00 :   9206:       pushq   %r12
    0.00 :   9208:       pushq   %rbp
    0.00 :   9209:       pushq   %rbx
    0.00 :   920a:       subq    $0x88, %rsp
    0.00 :   9211:       movq    %rdi, 8(%rsp)
    0.00 :   9216:       movl    8(%r9), %edi
    0.00 :   921a:       movq    %r8, 0x20(%rsp)
    0.00 :   921f:       movq    0xc0(%rsp), %rbp
    0.00 :   9227:       movq    %rdx, 0x28(%rsp)
    0.00 :   922c:       movq    0xd8(%rsp), %r8
    0.00 :   9234:       movq    %rdi, %r10
    0.00 :   9237:       movq    %rcx, 0x10(%rsp)
    0.00 :   923c:       movq    0xe8(%rsp), %r11
    0.00 :   9244:       movq    (%r9), %r13
    0.00 :   9247:       movl    %esi, %eax
    0.00 :   9249:       movl    0xe0(%rsp), %r9d
    0.00 :   9251:       movl    %esi, %r15d
    0.00 :   9254:       shrl    $6, %eax
    0.00 :   9257:       movq    %r8, %r12
    0.00 :   925a:       movl    %eax, 0x18(%rsp)
    0.00 :   925e:       andl    $2, %r9d
    0.00 :   9262:       jmp     0x92ff
    0.00 :   9267:       nopw    (%rax, %rax)
    0.00 :   9270:       movl    0x30c(%rbx), %edi
    0.00 :   9276:       testl   %edi, %edi
    0.00 :   9278:       je      0x92f2
    0.00 :   927a:       movq    0x70(%rbx), %rax
    0.00 :   927e:       movl    $0, 0x74(%rsp)
    0.00 :   9286:       movq    $0, 0x78(%rsp)
    0.00 :   928f:       movq    8(%rax), %rsi
    0.00 :   9293:       xorl    %eax, %eax
    0.00 :   9295:       testb   $0x20, 0x336(%rbx)
    0.00 :   929c:       je      0x92a4
    0.00 :   929e:       movq    (%rbx), %rax
    0.00 :   92a1:       addq    %rax, %rsi
    0.00 :   92a4:       movq    0x68(%rbx), %rdx
    0.00 :   92a8:       addq    8(%rdx), %rax
    0.00 :   92ac:       movq    0x318(%rbx), %rdx
    0.00 :   92b3:       movq    %rax, %r8
    0.00 :   92b6:       testq   %rdx, %rdx
    0.00 :   92b9:       je      0x93b0
    0.00 :   92bf:       movl    0x314(%rbx), %ecx
    0.00 :   92c5:       movl    0x18(%rsp), %eax
   13.66 :   92c9:       andl    0x310(%rbx), %eax
   16.28 :   92cf:       movq    (%rdx, %rax, 8), %rax
    0.00 :   92d3:       movl    %r15d, %edx
    0.00 :   92d6:       shrl    %cl, %edx
    0.00 :   92d8:       movl    %edx, %ecx
    0.00 :   92da:       movq    %rax, %rdx
Basic annotate [Failed: missing disasm output when specifying the target symbol]
---- end(-1) ----
 77: perf annotate basic tests                                       : FAILED!
```

Thanks,
Ian
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Namhyung Kim 1 month ago
On Tue, Oct 22, 2024 at 10:39:36AM -0700, Ian Rogers wrote:
> On Mon, Oct 21, 2024 at 10:41 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Thu, Oct 17, 2024 at 10:56:27PM -0700, Ian Rogers wrote:
> > > Sanitizer builds can break expectations for test disassembly,
> > > particularly in the annotate test. Add features for the different
> > > sanitizer options seen in the source tree. Use the added sanitizer
> > > feature to skip the annotate test when sanitizers are in use.
> >
> > Can you please split the perf check changes from the test change?
> > It's good to add an example output (of perf version --build-options)
> > with new sanitizer features.
> 
> Okay, will split/add in v3.

Thanks!

> 
> > Also it might be helpful if you share how the test fails.  I don't
> > think the disasm format is changed.  Then it probably missed to find
> > the target symbol in the first 250 lines for some reason.
> 
> Sure, the reproduction is trivial, just add -fsanitize=address, so I'm
> surprised you're not already seeing it:
> ```
> $ perf test annotate -v
> --- start ---
> test child forked, pid 444258
>  68e8a0-68e96b l noploop
> perf does have symbol 'noploop'
> Basic perf annotate test
>          : 0      0x68e8a0 <noploop>:
>     0.00 :   68e8a0:       pushq   %rbp
>     0.00 :   68e8a1:       movq    %rsp, %rbp
>     0.00 :   68e8a4:       subq    $0x30, %rsp
>     0.00 :   68e8a8:       movq    %fs:0x28, %rax
>     0.00 :   68e8b1:       movq    %rax, -8(%rbp)
>     0.00 :   68e8b5:       movl    %edi, -0xc(%rbp)
>     0.00 :   68e8b8:       movq    %rsi, -0x18(%rbp)
>     0.00 :   68e8bc:       movl    $1, -0x1c(%rbp)
>     0.00 :   68e8c3:       cmpl    $0, -0xc(%rbp)
>     0.00 :   68e8c7:       jle     0x68e8fe
>     0.00 :   68e8cd:       movq    -0x18(%rbp), %rax
>     0.00 :   68e8d1:       movq    %rax, -0x28(%rbp)
>     0.00 :   68e8d5:       shrq    $3, %rax
>     0.00 :   68e8d9:       cmpb    $0, 0x7fff8000(%rax)
>     0.00 :   68e8e0:       je      0x68e8ef
>     0.00 :   68e8e6:       movq    -0x28(%rbp), %rdi
>     0.00 :   68e8ea:       callq   0x2adea0
>     0.00 :   68e8ef:       movq    -0x28(%rbp), %rax
>     0.00 :   68e8f3:       movq    (%rax), %rdi
>     0.00 :   68e8f6:       callq   0x28fac0
>     0.00 :   68e8fb:       movl    %eax, -0x1c(%rbp)
>     0.00 :   68e8fe:       movl    $2, %edi
>     0.00 :   68e903:       leaq    0x66(%rip), %rsi     # 68e970 <sighandler>
>     0.00 :   68e90a:       callq   0x27e650
>     0.00 :   68e90f:       movl    $0xe, %edi
>     0.00 :   68e914:       leaq    0x55(%rip), %rsi     # 68e970 <sighandler>
>     0.00 :   68e91b:       callq   0x27e650
>     0.00 :   68e920:       movl    -0x1c(%rbp), %edi
>     0.00 :   68e923:       callq   0x2086e0
>     0.03 :   68e928:       movl    0x18aca72(%rip), %eax        #
> 1f3b3a0 <buildid_dir+0xa0>
>     0.00 :   68e92e:       cmpl    $0, %eax
>     0.00 :   68e931:       setne   %al
>    50.02 :   68e934:       xorb    $0xff, %al
>     0.00 :   68e936:       testb   $1, %al
>    41.28 :   68e938:       jne     0x68e943
>     0.00 :   68e93e:       jmp     0x68e948
>     8.67 :   68e943:       jmp     0x68e928
>     0.00 :   68e948:       movq    %fs:0x28, %rax
>     0.00 :   68e951:       movq    -8(%rbp), %rcx
>     0.00 :   68e955:       cmpq    %rcx, %rax
>     0.00 :   68e958:       jne     0x68e966
>     0.00 :   68e95e:       xorl    %eax, %eax
>     0.00 :   68e960:       addq    $0x30, %rsp
>     0.00 :   68e964:       popq    %rbp
>     0.00 :   68e965:       retq
>     0.00 :   68e966:       callq   0x2086f0
>     0.00 :   2d6910:       endbr64
>     0.00 :   2d6914:       pushq   %rbp
>     0.00 :   2d6915:       pushq   %r15
>     0.00 :   2d6917:       pushq   %r14
>     0.00 :   2d6919:       pushq   %r13
>     0.00 :   2d691b:       pushq   %r12
>     0.00 :   2d691d:       pushq   %rbx
>     0.00 :   2d691e:       subq    $0x48, %rsp
>     0.00 :   2d6922:       movq    %rdx, 0x20(%rsp)
>     0.00 :   2d6927:       movq    %r8, 0x30(%rsp)
>     0.00 :   2d692c:       leal    -1(%r8), %eax
>     0.00 :   2d6930:       cmpl    $2, %eax
>     0.00 :   2d6933:       jae     0x2d6b2c
>     0.00 :   2d6939:       movq    %rsi, %r9
>     0.00 :   2d693c:       movq    %rdi, %r12
>     0.00 :   2d693f:       cmpb    $0, 0x1c5f99c(%rip)  # 1f362e2
> <cpu_bitmap+0x122>
>     0.00 :   2d6946:       movl    $8, %eax
>     0.00 :   2d694b:       movl    $1, %ebx
>     0.00 :   2d6950:       cmoveq  %rax, %rbx
>     0.00 :   2d6954:       cmpb    $0, 0x1c5f989(%rip)  # 1f362e4
> <cpu_bitmap+0x124>
>     0.00 :   2d695b:       movq    %rsi, 0x28(%rsp)
>     0.00 :   2d6960:       je      0x2d697e
>     0.00 :   2d6962:       leaq    0x8e9708(%rip), %rdi
>     0.00 :   2d6969:       movq    %rcx, %rsi
>     0.00 :   2d696c:       movq    %r12, %rdx
>     0.00 :   2d696f:       movq    %r9, %rcx
>     0.00 :   2d6972:       xorl    %eax, %eax
>     0.00 :   2d6974:       callq   0x2c18c0
>     0.00 :   2d6979:       movq    0x28(%rsp), %r9
>     0.00 :   2d697e:       leaq    -1(%rbx), %rax
>     0.00 :   2d6982:       andq    %r12, %rax
>     0.00 :   2d6985:       leaq    (%rbx, %r12), %r13
>     0.00 :   2d6989:       subq    %rax, %r13
>     0.00 :   2d698c:       testq   %rax, %rax
>     0.00 :   2d698f:       cmoveq  %r12, %r13
>     0.00 :   2d6993:       leaq    8(%r13), %rax
>     0.00 :   2d6997:       cmpq    %r9, %rax
>     0.00 :   2d699a:       jbe     0x2d69ab
>     0.00 :   2d699c:       addq    $0x48, %rsp
>     0.00 :   2d69a0:       popq    %rbx
>     0.00 :   2d69a1:       popq    %r12
>     0.00 :   2d69a3:       popq    %r13
>     0.00 :   2d69a5:       popq    %r14
>     0.00 :   2d69a7:       popq    %r15
>     0.00 :   2d69a9:       popq    %rbp
>     0.00 :   2d69aa:       retq
>     0.00 :   2d69ab:       movabsq $0x7fffffffbfff, %r15
>     0.00 :   2d69b5:       leaq    0x40(%rsp), %rbp
>     0.00 :   2d69ba:       movq    %r12, 0x38(%rsp)
>     0.00 :   2d69bf:       jmp     0x2d69ea
>     0.00 :   2d69c1:       nopw    %cs:(%rax, %rax)
>     0.00 :   2d69d0:       movq    0x38(%rsp), %r12
>     0.00 :   2d69d5:       movq    0x28(%rsp), %r9
>     0.00 :   2d69da:       leaq    (%rbx, %r13), %rax
>     0.00 :   2d69de:       addq    $8, %rax
>     0.00 :   2d69e2:       addq    %rbx, %r13
>     0.00 :   2d69e5:       cmpq    %r9, %rax
>     0.00 :   2d69e8:       ja      0x2d699c
>     0.00 :   2d69ea:       movq    (%r13), %r14
>     0.00 :   2d69ee:       leaq    -0x4000(%r14), %rax
>     0.00 :   2d69f5:       cmpq    %r15, %rax
>    55.45 :   2d69f8:       ja      0x2d69da
>     0.00 :   2d69fa:       movq    %r14, %rdi
>    44.55 :   2d69fd:       callq   0x20df90
>     0.00 :   2d6a02:       movq    %rax, 0x18(%rsp)
>     0.00 :   2d6a07:       testq   %rax, %rax
>     0.00 :   2d6a0a:       je      0x2d69d5
>     0.00 :   2d6a0c:       cmpq    %r12, %rax
>     0.00 :   2d6a0f:       je      0x2d69d5
>     0.00 :   2d6a11:       movq    %rbp, %r12
>     0.00 :   2d6a14:       movq    %rbp, %rdi
>     0.00 :   2d6a17:       movq    %rax, %rsi
>     0.00 :   2d6a1a:       callq   0x20e190
>     0.00 :   2d6a1f:       movq    %rbp, %rdi
>     0.00 :   2d6a22:       callq   0x20e1d0
>     0.00 :   2d6a27:       cmpl    $2, %eax
>     0.00 :   2d6a2a:       je      0x2d69d0
>     0.00 :   2d6a2c:       movq    %rbp, %rdi
>     0.00 :   2d6a2f:       callq   0x20e1d0
>     0.00 :   2d6a34:       cmpl    $3, %eax
>     0.00 :   2d6a37:       je      0x2d69d0
>     0.00 :   2d6a39:       cmpb    $0, 0x1c5f8a3(%rip)  # 1f362e3
> <cpu_bitmap+0x123>
>     0.00 :   2d6a40:       jne     0x2d6aab
>     0.00 :   2d6a42:       movq    %r13, %rdi
>     0.00 :   2d6a45:       callq   0x2a9030
>     0.00 :   2d6a4a:       testb   %al, %al
>     0.00 :   2d6a4c:       je      0x2d6aab
>     0.00 :   2d6a4e:       cmpb    $0, 0x1c5f88f(%rip)  # 1f362e4
> <cpu_bitmap+0x124>
>     0.00 :   2d6a55:       je      0x2d69d0
>     0.00 :   2d6a5b:       movq    0x18(%rsp), %rax
>     0.00 :   2d6a60:       movq    %rax, 8(%rsp)
>     0.00 :   2d6a65:       movq    %rbp, %r12
>     0.00 :   2d6a68:       movq    %rbp, %rdi
>     0.00 :   2d6a6b:       callq   0x20e210
>     0.00 :   2d6a70:       movq    8(%rsp), %rcx
>     0.00 :   2d6a75:       addq    %rcx, %rax
>     0.00 :   2d6a78:       movq    %rax, 0x10(%rsp)
>     0.00 :   2d6a7d:       movq    %rbp, %rdi
>     0.00 :   2d6a80:       callq   0x20e210
>     0.00 :   2d6a85:       leaq    0x8e95ff(%rip), %rdi
>     0.00 :   2d6a8c:       movq    %r13, %rsi
>     0.00 :   2d6a8f:       movq    %r14, %rdx
>     0.00 :   2d6a92:       movq    8(%rsp), %rcx
>     0.00 :   2d6a97:       movq    0x10(%rsp), %r8
>     0.00 :   2d6a9c:       movq    %rax, %r9
>     0.00 :   2d6a9f:       xorl    %eax, %eax
>     0.00 :   2d6aa1:       callq   0x2c18c0
>     0.00 :   2d6aa6:       jmp     0x2d69d0
>     0.00 :   2d6aab:       movq    %rbp, %r12
>     0.00 :   2d6aae:       movq    %rbp, %rdi
>     0.00 :   2d6ab1:       movq    0x30(%rsp), %rsi
>     0.00 :   2d6ab6:       callq   0x20e1f0
>     0.00 :   2d6abb:       cmpb    $0, 0x1c5f822(%rip)  # 1f362e4
> <cpu_bitmap+0x124>
>     0.00 :   2d6ac2:       je      0x2d6b0c
>     0.00 :   2d6ac4:       movq    0x18(%rsp), %rax
>     0.00 :   2d6ac9:       movq    %rax, 8(%rsp)
>     0.00 :   2d6ace:       movq    %r12, %rdi
>     0.00 :   2d6ad1:       callq   0x20e210
>     0.00 :   2d6ad6:       movq    8(%rsp), %rcx
>     0.00 :   2d6adb:       addq    %rcx, %rax
>     0.00 :   2d6ade:       movq    %rax, 0x10(%rsp)
>     0.00 :   2d6ae3:       movq    %r12, %rdi
>     0.00 :   2d6ae6:       callq   0x20e210
>     0.00 :   2d6aeb:       leaq    0x8e95dd(%rip), %rdi
>     0.00 :   2d6af2:       movq    %r13, %rsi
>     0.00 :   2d6af5:       movq    %r14, %rdx
>     0.00 :   2d6af8:       movq    8(%rsp), %rcx
>     0.00 :   2d6afd:       movq    0x10(%rsp), %r8
>     0.00 :   2d6b02:       movq    %rax, %r9
>     0.00 :   2d6b05:       xorl    %eax, %eax
>     0.00 :   2d6b07:       callq   0x2c18c0
>     0.00 :   2d6b0c:       cmpq    $0, 0x20(%rsp)
>     0.00 :   2d6b12:       je      0x2d69d0
>     0.00 :   2d6b18:       movq    0x20(%rsp), %rdi
>     0.00 :   2d6b1d:       leaq    0x18(%rsp), %rsi
>     0.00 :   2d6b22:       callq   0x2b45a0
>     0.00 :   2d6b27:       jmp     0x2d69d0
>     0.00 :   2d6b2c:       leaq    0x8e94cf(%rip), %rdi
>     0.00 :   2d6b33:       leaq    0x8e94fe(%rip), %rdx
>     0.00 :   2d6b3a:       movl    $0x128, %esi
>     0.00 :   2d6b3f:       xorl    %ecx, %ecx
>     0.00 :   2d6b41:       xorl    %r8d, %r8d
>     0.00 :   2d6b44:       callq   0x2c77f0
>     0.00 :   9200:       pushq   %r15
>     0.00 :   9202:       pushq   %r14
>     0.00 :   9204:       pushq   %r13
>     0.00 :   9206:       pushq   %r12
>     0.00 :   9208:       pushq   %rbp
>     0.00 :   9209:       pushq   %rbx
>     0.00 :   920a:       subq    $0x88, %rsp
>     0.00 :   9211:       movq    %rdi, 8(%rsp)
>     0.00 :   9216:       movl    8(%r9), %edi
>     0.00 :   921a:       movq    %r8, 0x20(%rsp)
>     0.00 :   921f:       movq    0xc0(%rsp), %rbp
>     0.00 :   9227:       movq    %rdx, 0x28(%rsp)
>     0.00 :   922c:       movq    0xd8(%rsp), %r8
>     0.00 :   9234:       movq    %rdi, %r10
>     0.00 :   9237:       movq    %rcx, 0x10(%rsp)
>     0.00 :   923c:       movq    0xe8(%rsp), %r11
>     0.00 :   9244:       movq    (%r9), %r13
>     0.00 :   9247:       movl    %esi, %eax
>     0.00 :   9249:       movl    0xe0(%rsp), %r9d
>     0.00 :   9251:       movl    %esi, %r15d
>     0.00 :   9254:       shrl    $6, %eax
>     0.00 :   9257:       movq    %r8, %r12
>     0.00 :   925a:       movl    %eax, 0x18(%rsp)
>     0.00 :   925e:       andl    $2, %r9d
>     0.00 :   9262:       jmp     0x92ff
>     0.00 :   9267:       nopw    (%rax, %rax)
>     0.00 :   9270:       movl    0x30c(%rbx), %edi
>     0.00 :   9276:       testl   %edi, %edi
>     0.00 :   9278:       je      0x92f2
>     0.00 :   927a:       movq    0x70(%rbx), %rax
>     0.00 :   927e:       movl    $0, 0x74(%rsp)
>     0.00 :   9286:       movq    $0, 0x78(%rsp)
>     0.00 :   928f:       movq    8(%rax), %rsi
>     0.00 :   9293:       xorl    %eax, %eax
>     0.00 :   9295:       testb   $0x20, 0x336(%rbx)
>     0.00 :   929c:       je      0x92a4
>     0.00 :   929e:       movq    (%rbx), %rax
>     0.00 :   92a1:       addq    %rax, %rsi
>     0.00 :   92a4:       movq    0x68(%rbx), %rdx
>     0.00 :   92a8:       addq    8(%rdx), %rax
>     0.00 :   92ac:       movq    0x318(%rbx), %rdx
>     0.00 :   92b3:       movq    %rax, %r8
>     0.00 :   92b6:       testq   %rdx, %rdx
>     0.00 :   92b9:       je      0x93b0
>     0.00 :   92bf:       movl    0x314(%rbx), %ecx
>     0.00 :   92c5:       movl    0x18(%rsp), %eax
>    13.66 :   92c9:       andl    0x310(%rbx), %eax
>    16.28 :   92cf:       movq    (%rdx, %rax, 8), %rax
>     0.00 :   92d3:       movl    %r15d, %edx
>     0.00 :   92d6:       shrl    %cl, %edx
>     0.00 :   92d8:       movl    %edx, %ecx
>     0.00 :   92da:       movq    %rax, %rdx
> Basic annotate [Failed: missing disasm output when specifying the target symbol]

Hmm.. this is strange.  The error message says it failed when it
specified the target symbol (noploop) for perf annotate.

As it's the dominant symbol, it should have the same output for the
first function (noploop) whether it has target symbol or not and it
should match the disasm_regex.  I'm curious how it can fail here.

Thanks,
Namhyung


> ---- end(-1) ----
>  77: perf annotate basic tests                                       : FAILED!
> ```
> 
> Thanks,
> Ian
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Namhyung Kim 1 month ago
On Tue, Oct 22, 2024 at 11:18:18PM -0700, Namhyung Kim wrote:
> On Tue, Oct 22, 2024 at 10:39:36AM -0700, Ian Rogers wrote:
> > Sure, the reproduction is trivial, just add -fsanitize=address, so I'm
> > surprised you're not already seeing it:
> > ```
> > $ perf test annotate -v
> > --- start ---
> > test child forked, pid 444258
> >  68e8a0-68e96b l noploop
> > perf does have symbol 'noploop'
> > Basic perf annotate test
> >          : 0      0x68e8a0 <noploop>:
> >     0.00 :   68e8a0:       pushq   %rbp
> >     0.00 :   68e8a1:       movq    %rsp, %rbp
> >     0.00 :   68e8a4:       subq    $0x30, %rsp
[...]
> >     0.00 :   92d6:       shrl    %cl, %edx
> >     0.00 :   92d8:       movl    %edx, %ecx
> >     0.00 :   92da:       movq    %rax, %rdx
> > Basic annotate [Failed: missing disasm output when specifying the target symbol]
> 
> Hmm.. this is strange.  The error message says it failed when it
> specified the target symbol (noploop) for perf annotate.
> 
> As it's the dominant symbol, it should have the same output for the
> first function (noploop) whether it has target symbol or not and it
> should match the disasm_regex.  I'm curious how it can fail here.

Hmm.. ok.  For some reason, it wasn't failed when I add DEBUG=1.

Without DEBUG, I can see it now.

=================================================================
==1053492==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 33 byte(s) in 1 object(s) allocated from:
    #0 0x7f1ad78edd20 in strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:566
    #1 0x55eda19cb76f in perf_data__open (linux/tools/perf/perf+0x65276f) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #2 0x55eda18ffafa in __perf_session__new (linux/tools/perf/perf+0x586afa) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #3 0x55eda15485d3 in cmd_annotate (linux/tools/perf/perf+0x1cf5d3) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #4 0x55eda1695467 in run_builtin (linux/tools/perf/perf+0x31c467) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #5 0x55eda1695c0e in handle_internal_command (linux/tools/perf/perf+0x31cc0e) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #6 0x55eda153ba72 in main (linux/tools/perf/perf+0x1c2a72) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
    #7 0x7f1acda43b89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 33 byte(s) leaked in 1 allocation(s).
Unexpected signal in test_basic

No idea how it can leak the data->file.path (that's what I can find
where strdup is used in the function).

Thanks,
Namhyung
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Namhyung Kim 1 month ago
On Wed, Oct 23, 2024 at 03:19:38PM -0700, Namhyung Kim wrote:
> On Tue, Oct 22, 2024 at 11:18:18PM -0700, Namhyung Kim wrote:
> > On Tue, Oct 22, 2024 at 10:39:36AM -0700, Ian Rogers wrote:
> > > Sure, the reproduction is trivial, just add -fsanitize=address, so I'm
> > > surprised you're not already seeing it:
> > > ```
> > > $ perf test annotate -v
> > > --- start ---
> > > test child forked, pid 444258
> > >  68e8a0-68e96b l noploop
> > > perf does have symbol 'noploop'
> > > Basic perf annotate test
> > >          : 0      0x68e8a0 <noploop>:
> > >     0.00 :   68e8a0:       pushq   %rbp
> > >     0.00 :   68e8a1:       movq    %rsp, %rbp
> > >     0.00 :   68e8a4:       subq    $0x30, %rsp
> [...]
> > >     0.00 :   92d6:       shrl    %cl, %edx
> > >     0.00 :   92d8:       movl    %edx, %ecx
> > >     0.00 :   92da:       movq    %rax, %rdx
> > > Basic annotate [Failed: missing disasm output when specifying the target symbol]
> > 
> > Hmm.. this is strange.  The error message says it failed when it
> > specified the target symbol (noploop) for perf annotate.
> > 
> > As it's the dominant symbol, it should have the same output for the
> > first function (noploop) whether it has target symbol or not and it
> > should match the disasm_regex.  I'm curious how it can fail here.
> 
> Hmm.. ok.  For some reason, it wasn't failed when I add DEBUG=1.

Oh, now I'm seeing why.  We skip perf_session__delete() on !DEBUG build.
:(

> 
> Without DEBUG, I can see it now.
> 
> =================================================================
> ==1053492==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 33 byte(s) in 1 object(s) allocated from:
>     #0 0x7f1ad78edd20 in strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:566
>     #1 0x55eda19cb76f in perf_data__open (linux/tools/perf/perf+0x65276f) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #2 0x55eda18ffafa in __perf_session__new (linux/tools/perf/perf+0x586afa) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #3 0x55eda15485d3 in cmd_annotate (linux/tools/perf/perf+0x1cf5d3) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #4 0x55eda1695467 in run_builtin (linux/tools/perf/perf+0x31c467) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #5 0x55eda1695c0e in handle_internal_command (linux/tools/perf/perf+0x31cc0e) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #6 0x55eda153ba72 in main (linux/tools/perf/perf+0x1c2a72) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
>     #7 0x7f1acda43b89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
> 
> SUMMARY: AddressSanitizer: 33 byte(s) leaked in 1 allocation(s).
> Unexpected signal in test_basic
> 
> No idea how it can leak the data->file.path (that's what I can find
> where strdup is used in the function).

Maybe we need to revisit how much speed up it can give.

Thanks,
Namhyung
Re: [PATCH v2] perf check: Add sanitizer feature and use to avoid test failure
Posted by Ian Rogers 1 month ago
On Wed, Oct 23, 2024 at 4:24 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Oct 23, 2024 at 03:19:38PM -0700, Namhyung Kim wrote:
> > On Tue, Oct 22, 2024 at 11:18:18PM -0700, Namhyung Kim wrote:
> > > On Tue, Oct 22, 2024 at 10:39:36AM -0700, Ian Rogers wrote:
> > > > Sure, the reproduction is trivial, just add -fsanitize=address, so I'm
> > > > surprised you're not already seeing it:
> > > > ```
> > > > $ perf test annotate -v
> > > > --- start ---
> > > > test child forked, pid 444258
> > > >  68e8a0-68e96b l noploop
> > > > perf does have symbol 'noploop'
> > > > Basic perf annotate test
> > > >          : 0      0x68e8a0 <noploop>:
> > > >     0.00 :   68e8a0:       pushq   %rbp
> > > >     0.00 :   68e8a1:       movq    %rsp, %rbp
> > > >     0.00 :   68e8a4:       subq    $0x30, %rsp
> > [...]
> > > >     0.00 :   92d6:       shrl    %cl, %edx
> > > >     0.00 :   92d8:       movl    %edx, %ecx
> > > >     0.00 :   92da:       movq    %rax, %rdx
> > > > Basic annotate [Failed: missing disasm output when specifying the target symbol]
> > >
> > > Hmm.. this is strange.  The error message says it failed when it
> > > specified the target symbol (noploop) for perf annotate.
> > >
> > > As it's the dominant symbol, it should have the same output for the
> > > first function (noploop) whether it has target symbol or not and it
> > > should match the disasm_regex.  I'm curious how it can fail here.
> >
> > Hmm.. ok.  For some reason, it wasn't failed when I add DEBUG=1.
>
> Oh, now I'm seeing why.  We skip perf_session__delete() on !DEBUG build.
> :(
>
> >
> > Without DEBUG, I can see it now.
> >
> > =================================================================
> > ==1053492==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 33 byte(s) in 1 object(s) allocated from:
> >     #0 0x7f1ad78edd20 in strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:566
> >     #1 0x55eda19cb76f in perf_data__open (linux/tools/perf/perf+0x65276f) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #2 0x55eda18ffafa in __perf_session__new (linux/tools/perf/perf+0x586afa) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #3 0x55eda15485d3 in cmd_annotate (linux/tools/perf/perf+0x1cf5d3) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #4 0x55eda1695467 in run_builtin (linux/tools/perf/perf+0x31c467) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #5 0x55eda1695c0e in handle_internal_command (linux/tools/perf/perf+0x31cc0e) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #6 0x55eda153ba72 in main (linux/tools/perf/perf+0x1c2a72) (BuildId: 6fc1b7cdc123c7bd586ce55ea8b727875f42cda2)
> >     #7 0x7f1acda43b89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
> >
> > SUMMARY: AddressSanitizer: 33 byte(s) leaked in 1 allocation(s).
> > Unexpected signal in test_basic
> >
> > No idea how it can leak the data->file.path (that's what I can find
> > where strdup is used in the function).
>
> Maybe we need to revisit how much speed up it can give.

I think this is a different issue and not sanitizer related except for
the sanitizer catching the deliberate leak. My full build command line
is:
```
$ make -C tools/perf O=/tmp/perf DEBUG=1 EXTRA_CFLAGS="-O0 -g
-fno-omit-frame-pointer -fsanitize=address" CC=clang CXX=clang++
HOSTCC=clang
```
Of which the -O0 may be the thing that is making the difference and
breaking the test with sanitizers for me.

Thanks,
Ian