[PATCH 0/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[PATCH 0/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[James Clark]

The first commit outputs the unwinding feature definition in perf
version so that the script can gate on it. And then skip the test if
it's not present.

I didn't add any fixes tags because it's modifying the perf version
output, requires both commits, and it's just for a test skip change so
I don't think backporting makes sense.

James Clark (2):
  perf version: Display availability of HAVE_DWARF_UNWIND_SUPPORT
  perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

 tools/perf/builtin-version.c                    | 1 +
 tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++
 2 files changed, 7 insertions(+)

-- 
2.34.1

[PATCH 1/2] perf version: Display availability of HAVE_DWARF_UNWIND_SUPPORT

[James Clark]

Even though unwinding depends on either HAVE_DWARF_SUPPORT or
HAVE_LIBUNWIND, scripts testing unwinding can't just look for the
existence of either of those flags. This is because
NO_LIBDW_DWARF_UNWIND=1 can disable unwinding with libdw, but libdw will
still be linked leaving HAVE_DWARF_SUPPORT turned on. Presumably because
it is used for things other than unwinding, so I don't think this needs
to be fixed.

HAVE_DWARF_UNWIND_SUPPORT already takes the combination of all those
things into account, and is used to gate the built in tests like "Test
dwarf unwind", so add it to the feature list output so that it can be
used by the script tests too.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-version.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index ac20c2b9bbc2..529e9ce8c46c 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -82,6 +82,7 @@ static void library_status(void)
 	STATUS(HAVE_LIBPFM, libpfm4);
 	STATUS(HAVE_LIBTRACEEVENT, libtraceevent);
 	STATUS(HAVE_BPF_SKEL, bpf_skeletons);
+	STATUS(HAVE_DWARF_UNWIND_SUPPORT, dwarf-unwind-support);
 }
 
 int cmd_version(int argc, const char **argv)
-- 
2.34.1

[PATCH 2/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[James Clark]

Even though this is a frame pointer unwind test, it's testing that a
frame pointer stack can be augmented correctly with a partial
Dwarf unwind. So add a feature check so that this test skips instead of
fails if Dwarf unwinding isn't present.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
index e342e6c8aa50..83b53591b1ea 100755
--- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
+++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
@@ -8,6 +8,12 @@ shelldir=$(dirname "$0")
 
 lscpu | grep -q "aarch64" || exit 2
 
+if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF
+then
+  echo "Skipping, no dwarf unwind support"
+  exit 2
+fi
+
 skip_test_missing_symbol leafloop
 
 PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
-- 
2.34.1

Re: [PATCH 2/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[Ian Rogers]

On Tue, Jan 23, 2024 at 8:39 AM James Clark <james.clark@arm.com> wrote:
>
> Even though this is a frame pointer unwind test, it's testing that a
> frame pointer stack can be augmented correctly with a partial
> Dwarf unwind. So add a feature check so that this test skips instead of
> fails if Dwarf unwinding isn't present.

Hi James,

Is there value in testing without the partial Dwarf unwind? Presumably
that is covered by the existing dwarf unwind test?
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/dwarf-unwind.c?h=perf-tools-next
If the issue is inlined functions I'm surprised addr2line isn't doing
the job properly. Is there an addr2line perf script issue here?

Thanks,
Ian

> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> index e342e6c8aa50..83b53591b1ea 100755
> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> @@ -8,6 +8,12 @@ shelldir=$(dirname "$0")
>
>  lscpu | grep -q "aarch64" || exit 2
>
> +if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF
> +then
> +  echo "Skipping, no dwarf unwind support"
> +  exit 2
> +fi
> +
>  skip_test_missing_symbol leafloop
>
>  PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> --
> 2.34.1
>

Re: [PATCH 2/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[James Clark]

On 23/01/2024 17:41, Ian Rogers wrote:
> On Tue, Jan 23, 2024 at 8:39 AM James Clark <james.clark@arm.com> wrote:
>>
>> Even though this is a frame pointer unwind test, it's testing that a
>> frame pointer stack can be augmented correctly with a partial
>> Dwarf unwind. So add a feature check so that this test skips instead of
>> fails if Dwarf unwinding isn't present.
> 
> Hi James,
> 
> Is there value in testing without the partial Dwarf unwind? Presumably

Yeah I think we could add a test for just --call-graph=fp, I don't think
there is one. But that would be separate to this test, and would be
redundant if the tests are run with a dwarf unwinder present because
this test already requires the frame pointer unwinder to be correct.

> that is covered by the existing dwarf unwind test?

There is no overlap, this test test is for --call-graph=fp, and the
dwarf test is for --call-graph=dwarf

> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/dwarf-unwind.c?h=perf-tools-next
> If the issue is inlined functions I'm surprised addr2line isn't doing
> the job properly. Is there an addr2line perf script issue here?
> 

The issue isn't inlined functions, it's when the leaf frame doesn't
insert a frame pointer. In that case we use the link register to see
what the parent function of the leaf frame was and insert it into the
frame pointer stack.

Dwarf is only used in this case to confirm if the link register was
valid at that instruction.

See commit b9f6fbb for more info. Long story short this test was only
added for that feature and it requires a dwarf unwinder to pass despite
being called test_arm_callgraph_fp

> Thanks,
> Ian
> 


>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>  tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
>> index e342e6c8aa50..83b53591b1ea 100755
>> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
>> +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
>> @@ -8,6 +8,12 @@ shelldir=$(dirname "$0")
>>
>>  lscpu | grep -q "aarch64" || exit 2
>>
>> +if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF
>> +then
>> +  echo "Skipping, no dwarf unwind support"
>> +  exit 2
>> +fi
>> +
>>  skip_test_missing_symbol leafloop
>>
>>  PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
>> --
>> 2.34.1
>>

Re: [PATCH 2/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[Ian Rogers]

On Wed, Jan 24, 2024 at 1:24 AM James Clark <james.clark@arm.com> wrote:
>
>
>
> On 23/01/2024 17:41, Ian Rogers wrote:
> > On Tue, Jan 23, 2024 at 8:39 AM James Clark <james.clark@arm.com> wrote:
> >>
> >> Even though this is a frame pointer unwind test, it's testing that a
> >> frame pointer stack can be augmented correctly with a partial
> >> Dwarf unwind. So add a feature check so that this test skips instead of
> >> fails if Dwarf unwinding isn't present.
> >
> > Hi James,
> >
> > Is there value in testing without the partial Dwarf unwind? Presumably
>
> Yeah I think we could add a test for just --call-graph=fp, I don't think
> there is one. But that would be separate to this test, and would be
> redundant if the tests are run with a dwarf unwinder present because
> this test already requires the frame pointer unwinder to be correct.
>
> > that is covered by the existing dwarf unwind test?
>
> There is no overlap, this test test is for --call-graph=fp, and the
> dwarf test is for --call-graph=dwarf
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/dwarf-unwind.c?h=perf-tools-next
> > If the issue is inlined functions I'm surprised addr2line isn't doing
> > the job properly. Is there an addr2line perf script issue here?
> >
>
> The issue isn't inlined functions, it's when the leaf frame doesn't
> insert a frame pointer. In that case we use the link register to see
> what the parent function of the leaf frame was and insert it into the
> frame pointer stack.
>
> Dwarf is only used in this case to confirm if the link register was
> valid at that instruction.
>
> See commit b9f6fbb for more info. Long story short this test was only
> added for that feature and it requires a dwarf unwinder to pass despite
> being called test_arm_callgraph_fp

Ok, not directly seeing b9f6fbb being dependent on
HAVE_DWARF_UNWIND_SUPPORT. I'll assume I'm ignorant and the fix here
is obviously a workaround.

For the series:
Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> > Thanks,
> > Ian
> >
>
>
> >> Signed-off-by: James Clark <james.clark@arm.com>
> >> ---
> >>  tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> >> index e342e6c8aa50..83b53591b1ea 100755
> >> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> >> +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> >> @@ -8,6 +8,12 @@ shelldir=$(dirname "$0")
> >>
> >>  lscpu | grep -q "aarch64" || exit 2
> >>
> >> +if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF
> >> +then
> >> +  echo "Skipping, no dwarf unwind support"
> >> +  exit 2
> >> +fi
> >> +
> >>  skip_test_missing_symbol leafloop
> >>
> >>  PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> >> --
> >> 2.34.1
> >>

Re: [PATCH 0/2] perf test: Skip test_arm_callgraph_fp.sh if unwinding isn't built in

[Namhyung Kim]

On Tue, 23 Jan 2024 16:39:00 +0000, James Clark wrote:
> The first commit outputs the unwinding feature definition in perf
> version so that the script can gate on it. And then skip the test if
> it's not present.
> 
> I didn't add any fixes tags because it's modifying the perf version
> output, requires both commits, and it's just for a test skip change so
> I don't think backporting makes sense.
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
-- 
Namhyung Kim <namhyung@kernel.org>