[PATCH v3 1/2] perf check: Add sanitizer features

[PATCH v3 1/2] perf check: Add sanitizer features

[Ian Rogers]

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.

An example for an asan build:
```
$ perf version --build-options
perf version 6.12.rc3.g939b0caafd01
                   aio: [ on  ]  # HAVE_AIO_SUPPORT
                   bpf: [ on  ]  # HAVE_LIBBPF_SUPPORT
         bpf_skeletons: [ on  ]  # HAVE_BPF_SKEL
            debuginfod: [ on  ]  # HAVE_DEBUGINFOD_SUPPORT
                 dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
    dwarf_getlocations: [ on  ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
          dwarf-unwind: [ on  ]  # HAVE_DWARF_UNWIND_SUPPORT
              auxtrace: [ on  ]  # HAVE_AUXTRACE_SUPPORT
              libaudit: [ OFF ]  # HAVE_LIBAUDIT_SUPPORT
                libbfd: [ OFF ]  # HAVE_LIBBFD_SUPPORT
           libcapstone: [ on  ]  # HAVE_LIBCAPSTONE_SUPPORT
             libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
    libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
                libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
               libnuma: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
            libopencsd: [ OFF ]  # HAVE_CSTRACE_SUPPORT
               libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
               libpfm4: [ on  ]  # HAVE_LIBPFM
             libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
              libslang: [ on  ]  # HAVE_SLANG_SUPPORT
         libtraceevent: [ on  ]  # HAVE_LIBTRACEEVENT
             libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
                  lzma: [ on  ]  # HAVE_LZMA_SUPPORT
numa_num_possible_cpus: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
             sanitizer: [ on  ]  # HAVE_SANITIZER
     sanitizer_address: [ on  ]  # HAVE_SANITIZER_ADDRESS
        sanitizer_leak: [ on  ]  # HAVE_SANITIZER_LEAK
      sanitizer_memory: [ OFF ]  # HAVE_SANITIZER_MEMORY
      sanitizer_thread: [ OFF ]  # HAVE_SANITIZER_THREAD
   sanitizer_undefined: [ OFF ]  # HAVE_SANITIZER_UNDEFINED
         syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
                  zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
                  zstd: [ on  ]  # HAVE_ZSTD_SUPPORT
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
v3 split out annotate test check
v2 build fix.
---
 tools/perf/builtin-check.c | 49 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 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),
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v3 2/2] perf test: Skip annotate test for sanitizer builds

[Ian Rogers]

Symbols vary and the test breaks.

Closes: https://lore.kernel.org/lkml/CAP-5=fU04PAN4T=7KuHA4h+po=oTy+6Nbee-Gvx9hCsEf2Lh0w@mail.gmail.com/
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/annotate.sh | 6 ++++++
 1 file changed, 6 insertions(+)

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.163.g1226f6d8fa-goog

Re: [PATCH v3 2/2] perf test: Skip annotate test for sanitizer builds

[Namhyung Kim]

On Tue, Oct 22, 2024 at 10:48:38AM -0700, Ian Rogers wrote:
> Symbols vary and the test breaks.
> 
> Closes: https://lore.kernel.org/lkml/CAP-5=fU04PAN4T=7KuHA4h+po=oTy+6Nbee-Gvx9hCsEf2Lh0w@mail.gmail.com/
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/annotate.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> 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"

I don't think it's because of different assembly code.
It should be the return value from ASAN leak detector.

Maybe we can enable it using "ASAN_OPTIONS=detect_leaks=0"?
Probably with a comment that mentions we don't call
perf_session__delete() in perf annotate for a performance reason.

Thanks,
Namhyung


> +  exit 2
> +fi
> +
>  shelldir=$(dirname "$0")
>  
>  # shellcheck source=lib/perf_has_symbol.sh
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH v3 2/2] perf test: Skip annotate test for sanitizer builds

[Ian Rogers]

On Wed, Oct 23, 2024 at 4:31 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Tue, Oct 22, 2024 at 10:48:38AM -0700, Ian Rogers wrote:
> > Symbols vary and the test breaks.
> >
> > Closes: https://lore.kernel.org/lkml/CAP-5=fU04PAN4T=7KuHA4h+po=oTy+6Nbee-Gvx9hCsEf2Lh0w@mail.gmail.com/
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/shell/annotate.sh | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > 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"
>
> I don't think it's because of different assembly code.
> It should be the return value from ASAN leak detector.
>
> Maybe we can enable it using "ASAN_OPTIONS=detect_leaks=0"?
> Probably with a comment that mentions we don't call
> perf_session__delete() in perf annotate for a performance reason.

So doing things like not deleting things for performance reasons is
okay but should be guarded by an "#ifndef NDEBUG" or worse case
"#ifndef LEAK_SANITIZER", not always be on. The comment about a
performance optimization belongs there. We should be trying to avoid
disabling sanitizers in tests as otherwise we're missing the benefits
of sanitizers. I'll see if doing this can to avoid the test skipping.
It'd be nice if other people tested with sanitizers. I think we still
want the features for if we get other issues in the future.

Thanks,
Ian

Re: [PATCH v3 1/2] perf check: Add sanitizer features

[Ian Rogers]

On Tue, Oct 22, 2024 at 10:48 AM Ian Rogers <irogers@google.com> 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.
>
> An example for an asan build:
> ```
> $ perf version --build-options
> perf version 6.12.rc3.g939b0caafd01
>                    aio: [ on  ]  # HAVE_AIO_SUPPORT
>                    bpf: [ on  ]  # HAVE_LIBBPF_SUPPORT
>          bpf_skeletons: [ on  ]  # HAVE_BPF_SKEL
>             debuginfod: [ on  ]  # HAVE_DEBUGINFOD_SUPPORT
>                  dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
>     dwarf_getlocations: [ on  ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
>           dwarf-unwind: [ on  ]  # HAVE_DWARF_UNWIND_SUPPORT
>               auxtrace: [ on  ]  # HAVE_AUXTRACE_SUPPORT
>               libaudit: [ OFF ]  # HAVE_LIBAUDIT_SUPPORT
>                 libbfd: [ OFF ]  # HAVE_LIBBFD_SUPPORT
>            libcapstone: [ on  ]  # HAVE_LIBCAPSTONE_SUPPORT
>              libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
>     libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
>                 libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
>                libnuma: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
>             libopencsd: [ OFF ]  # HAVE_CSTRACE_SUPPORT
>                libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
>                libpfm4: [ on  ]  # HAVE_LIBPFM
>              libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
>               libslang: [ on  ]  # HAVE_SLANG_SUPPORT
>          libtraceevent: [ on  ]  # HAVE_LIBTRACEEVENT
>              libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
>                   lzma: [ on  ]  # HAVE_LZMA_SUPPORT
> numa_num_possible_cpus: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
>              sanitizer: [ on  ]  # HAVE_SANITIZER
>      sanitizer_address: [ on  ]  # HAVE_SANITIZER_ADDRESS
>         sanitizer_leak: [ on  ]  # HAVE_SANITIZER_LEAK
>       sanitizer_memory: [ OFF ]  # HAVE_SANITIZER_MEMORY
>       sanitizer_thread: [ OFF ]  # HAVE_SANITIZER_THREAD
>    sanitizer_undefined: [ OFF ]  # HAVE_SANITIZER_UNDEFINED
>          syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
>                   zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
>                   zstd: [ on  ]  # HAVE_ZSTD_SUPPORT
> ```
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v3 split out annotate test check
> v2 build fix.

Working around the asan issue for the annotate test is no longer
needed for me. Should we carry this first patch so that tests can have
better feature checks on sanitizers? I can resend the patch again on
its own, if this is useful.

Thanks,
Ian


> ---
>  tools/perf/builtin-check.c | 49 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 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),
> --
> 2.47.0.163.g1226f6d8fa-goog
>