2023-07-28 15:51:54

by Georg Müller

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

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 <[email protected]>
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



2023-07-28 16:42:21

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc

On Fri, Jul 28, 2023 at 8:19 AM Georg Müller <[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/r/CAP-5=fUP6UuLgRty3t2=fQsQi3k4hDMz415vWdp1x88QMvZ8ug@mail.gmail.com/

Acked-by: Ian Rogers <[email protected]>

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
>

2023-07-29 02:56:16

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc

On Fri, 28 Jul 2023 17:18:12 +0200
Georg Müller <[email protected]> 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) <[email protected]>

>
> Fixes: 56cbeacf1435 ("perf probe: Add test for regression introduced by switch to die_get_decl_file()")
> Signed-off-by: Georg Müller <[email protected]>
> 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) <[email protected]>

2023-07-29 12:23:55

by Georg Müller

[permalink] [raw]
Subject: Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc


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.

2023-08-01 01:03:19

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] perf probe: skip test_uprobe_from_different_cu if there is no gcc

On Sat, 29 Jul 2023 12:59:50 +0200
Georg Müller <[email protected]> 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) <[email protected]>