linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
@ 2023-07-28 15:18 Georg Müller
  2023-07-28 15:46 ` Ian Rogers
  2023-07-29  0:38 ` Masami Hiramatsu
  0 siblings, 2 replies; 5+ messages in thread
From: Georg Müller @ 2023-07-28 15:18 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Georg Müller,
	Masami Hiramatsu (Google)
  Cc: Arnaldo Carvalho de Melo, linux-perf-users, linux-kernel

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
  2023-07-28 15:18 [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc Georg Müller
@ 2023-07-28 15:46 ` Ian Rogers
  2023-07-29  0:38 ` Masami Hiramatsu
  1 sibling, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2023-07-28 15:46 UTC (permalink / raw)
  To: Georg Müller
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Adrian Hunter, Masami Hiramatsu (Google),
	Arnaldo Carvalho de Melo, linux-perf-users, linux-kernel

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
  2023-07-28 15:18 [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc Georg Müller
  2023-07-28 15:46 ` Ian Rogers
@ 2023-07-29  0:38 ` Masami Hiramatsu
  2023-07-29 10:59   ` Georg Müller
  1 sibling, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2023-07-29  0:38 UTC (permalink / raw)
  To: Georg Müller
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Arnaldo Carvalho de Melo,
	linux-perf-users, linux-kernel

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
  2023-07-29  0:38 ` Masami Hiramatsu
@ 2023-07-29 10:59   ` Georg Müller
  2023-08-01  0:29     ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Georg Müller @ 2023-07-29 10:59 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Arnaldo Carvalho de Melo,
	linux-perf-users, linux-kernel


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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc
  2023-07-29 10:59   ` Georg Müller
@ 2023-08-01  0:29     ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2023-08-01  0:29 UTC (permalink / raw)
  To: Georg Müller
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Arnaldo Carvalho de Melo,
	linux-perf-users, linux-kernel

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-08-01  0:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-28 15:18 [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc Georg Müller
2023-07-28 15:46 ` Ian Rogers
2023-07-29  0:38 ` Masami Hiramatsu
2023-07-29 10:59   ` Georg Müller
2023-08-01  0:29     ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).