[PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc

Georg Müller posted 1 patch 2 years, 6 months ago
tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
Posted by Georg Müller 2 years, 6 months ago
Without gcc, the test will fail.

On cleanup, ignore probe removal errors. Otherwise, in case of an error
adding the probe, the temporary directory is not removed.

Fixes: 56cbeacf1435 ("perf probe: Add test for regression introduced by switch to die_get_decl_file()")
Signed-off-by: Georg Müller <georgmueller@gmx.net>
Link: https://lore.kernel.org/r/CAP-5=fUP6UuLgRty3t2=fQsQi3k4hDMz415vWdp1x88QMvZ8ug@mail.gmail.com/
---
 tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
index 00d2e0e2e0c2..319f36ebb9a4 100755
--- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
+++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
@@ -4,6 +4,12 @@

 set -e

+# skip if there's no gcc
+if ! [ -x "$(command -v gcc)" ]; then
+        echo "failed: no gcc compiler"
+        exit 2
+fi
+
 temp_dir=$(mktemp -d /tmp/perf-uprobe-different-cu-sh.XXXXXXXXXX)

 cleanup()
@@ -11,7 +17,7 @@ cleanup()
 	trap - EXIT TERM INT
 	if [[ "${temp_dir}" =~ ^/tmp/perf-uprobe-different-cu-sh.*$ ]]; then
 		echo "--- Cleaning up ---"
-		perf probe -x ${temp_dir}/testfile -d foo
+		perf probe -x ${temp_dir}/testfile -d foo || true
 		rm -f "${temp_dir}/"*
 		rmdir "${temp_dir}"
 	fi
--
2.41.0
Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
Posted by Masami Hiramatsu (Google) 2 years, 6 months ago
On Fri, 28 Jul 2023 17:18:12 +0200
Georg Müller <georgmueller@gmx.net> wrote:

> Without gcc, the test will fail.
> 
> On cleanup, ignore probe removal errors. Otherwise, in case of an error
> adding the probe, the temporary directory is not removed.

Interesting, so clang will not generate DWARF or perf probe is not able to
handle clang generated DWARF?

Anyway, this looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

> 
> Fixes: 56cbeacf1435 ("perf probe: Add test for regression introduced by switch to die_get_decl_file()")
> Signed-off-by: Georg Müller <georgmueller@gmx.net>
> Link: https://lore.kernel.org/r/CAP-5=fUP6UuLgRty3t2=fQsQi3k4hDMz415vWdp1x88QMvZ8ug@mail.gmail.com/
> ---
>  tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> index 00d2e0e2e0c2..319f36ebb9a4 100755
> --- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> +++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> @@ -4,6 +4,12 @@
> 
>  set -e
> 
> +# skip if there's no gcc
> +if ! [ -x "$(command -v gcc)" ]; then
> +        echo "failed: no gcc compiler"
> +        exit 2
> +fi
> +
>  temp_dir=$(mktemp -d /tmp/perf-uprobe-different-cu-sh.XXXXXXXXXX)
> 
>  cleanup()
> @@ -11,7 +17,7 @@ cleanup()
>  	trap - EXIT TERM INT
>  	if [[ "${temp_dir}" =~ ^/tmp/perf-uprobe-different-cu-sh.*$ ]]; then
>  		echo "--- Cleaning up ---"
> -		perf probe -x ${temp_dir}/testfile -d foo
> +		perf probe -x ${temp_dir}/testfile -d foo || true
>  		rm -f "${temp_dir}/"*
>  		rmdir "${temp_dir}"
>  	fi
> --
> 2.41.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
Posted by Georg Müller 2 years, 6 months ago
Am 29.07.23 um 02:38 schrieb Masami Hiramatsu (Google):
>
> Interesting, so clang will not generate DWARF or perf probe is not able to
> handle clang generated DWARF?
>

clang does not accept mixed -flto and non-lto CUs and the problem is not
reproducible by this sample code using clang if using -flto for all CUs.
There might be (bigger?) examples where the same issue is triggered by
clang and bigger examples (like systemd on fedora) where I ran into the
bug, but this small example only shows the problem when using gcc and
mixing -flto and non-lto CUs.
Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
Posted by Masami Hiramatsu (Google) 2 years, 6 months ago
On Sat, 29 Jul 2023 12:59:50 +0200
Georg Müller <georgmueller@gmx.net> wrote:

> 
> Am 29.07.23 um 02:38 schrieb Masami Hiramatsu (Google):
> >
> > Interesting, so clang will not generate DWARF or perf probe is not able to
> > handle clang generated DWARF?
> >
> 
> clang does not accept mixed -flto and non-lto CUs and the problem is not
> reproducible by this sample code using clang if using -flto for all CUs.
> There might be (bigger?) examples where the same issue is triggered by
> clang and bigger examples (like systemd on fedora) where I ran into the
> bug, but this small example only shows the problem when using gcc and
> mixing -flto and non-lto CUs.

Thanks for the explanation! So the problem will be in the compiler side
(and maybe fixed when it is updated.)

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
Posted by Ian Rogers 2 years, 6 months ago
On Fri, Jul 28, 2023 at 8:19 AM Georg Müller <georgmueller@gmx.net> wrote:
>
> Without gcc, the test will fail.
>
> On cleanup, ignore probe removal errors. Otherwise, in case of an error
> adding the probe, the temporary directory is not removed.
>
> Fixes: 56cbeacf1435 ("perf probe: Add test for regression introduced by switch to die_get_decl_file()")
> Signed-off-by: Georg Müller <georgmueller@gmx.net>
> Link: https://lore.kernel.org/r/CAP-5=fUP6UuLgRty3t2=fQsQi3k4hDMz415vWdp1x88QMvZ8ug@mail.gmail.com/

Acked-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> index 00d2e0e2e0c2..319f36ebb9a4 100755
> --- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> +++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> @@ -4,6 +4,12 @@
>
>  set -e
>
> +# skip if there's no gcc
> +if ! [ -x "$(command -v gcc)" ]; then
> +        echo "failed: no gcc compiler"
> +        exit 2
> +fi
> +
>  temp_dir=$(mktemp -d /tmp/perf-uprobe-different-cu-sh.XXXXXXXXXX)
>
>  cleanup()
> @@ -11,7 +17,7 @@ cleanup()
>         trap - EXIT TERM INT
>         if [[ "${temp_dir}" =~ ^/tmp/perf-uprobe-different-cu-sh.*$ ]]; then
>                 echo "--- Cleaning up ---"
> -               perf probe -x ${temp_dir}/testfile -d foo
> +               perf probe -x ${temp_dir}/testfile -d foo || true
>                 rm -f "${temp_dir}/"*
>                 rmdir "${temp_dir}"
>         fi
> --
> 2.41.0
>