To ensure the stack frames are on the stack tail calls optimizations
need to be inhibited. If your compiler supports an attribute use it,
otherwise use an asm volatile barrier.
The barrier fix was suggested here:
https://lore.kernel.org/lkml/[email protected]/
Tested with an optimized clang build and by forcing the asm barrier
route with an optimized clang build.
A GCC bug tracking a proper disable_tail_calls is:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831
Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
attribute")
v2. is a rebase. The original fix patch generated quite a lot of
discussion over the right place for the fix:
https://lore.kernel.org/lkml/[email protected]/
The patch reflects my preference of it being near the use, so that
future code cleanups don't break this somewhat special usage.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index a288035eb362..c756284b3b13 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -20,6 +20,23 @@
/* For bsearch. We try to unwind functions in shared object. */
#include <stdlib.h>
+/*
+ * The test will assert frames are on the stack but tail call optimizations lose
+ * the frame of the caller. Clang can disable this optimization on a called
+ * function but GCC currently (11/2020) lacks this attribute. The barrier is
+ * used to inhibit tail calls in these cases.
+ */
+#ifdef __has_attribute
+#if __has_attribute(disable_tail_calls)
+#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
+#define NO_TAIL_CALL_BARRIER
+#endif
+#endif
+#ifndef NO_TAIL_CALL_ATTRIBUTE
+#define NO_TAIL_CALL_ATTRIBUTE
+#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
+#endif
+
static int mmap_handler(struct perf_tool *tool __maybe_unused,
union perf_event *event,
struct perf_sample *sample,
@@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
return strcmp((const char *) symbol, funcs[idx]);
}
-noinline int test_dwarf_unwind__thread(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
{
struct perf_sample sample;
unsigned long cnt = 0;
@@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
static int global_unwind_retval = -INT_MAX;
-noinline int test_dwarf_unwind__compare(void *p1, void *p2)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
{
/* Any possible value should be 'thread' */
struct thread *thread = *(struct thread **)p1;
@@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
return p1 - p2;
}
-noinline int test_dwarf_unwind__krava_3(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
{
struct thread *array[2] = {thread, thread};
void *fp = &bsearch;
@@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
return global_unwind_retval;
}
-noinline int test_dwarf_unwind__krava_2(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
{
- return test_dwarf_unwind__krava_3(thread);
+ int ret;
+
+ ret = test_dwarf_unwind__krava_3(thread);
+ NO_TAIL_CALL_BARRIER;
+ return ret;
}
-noinline int test_dwarf_unwind__krava_1(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
{
- return test_dwarf_unwind__krava_2(thread);
+ int ret;
+
+ ret = test_dwarf_unwind__krava_2(thread);
+ NO_TAIL_CALL_BARRIER;
+ return ret;
}
int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
--
2.33.0.464.g1972c5931b-goog
On Wed, Sep 22, 2021 at 10:38:12AM -0700, Ian Rogers wrote:
> To ensure the stack frames are on the stack tail calls optimizations
> need to be inhibited. If your compiler supports an attribute use it,
> otherwise use an asm volatile barrier.
>
> The barrier fix was suggested here:
> https://lore.kernel.org/lkml/[email protected]/
> Tested with an optimized clang build and by forcing the asm barrier
> route with an optimized clang build.
>
> A GCC bug tracking a proper disable_tail_calls is:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831
>
> Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
> attribute")
>
> v2. is a rebase. The original fix patch generated quite a lot of
> discussion over the right place for the fix:
> https://lore.kernel.org/lkml/[email protected]/
> The patch reflects my preference of it being near the use, so that
> future code cleanups don't break this somewhat special usage.
Acked-by: Jiri Olsa <[email protected]>
thanks,
jirka
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index a288035eb362..c756284b3b13 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -20,6 +20,23 @@
> /* For bsearch. We try to unwind functions in shared object. */
> #include <stdlib.h>
>
> +/*
> + * The test will assert frames are on the stack but tail call optimizations lose
> + * the frame of the caller. Clang can disable this optimization on a called
> + * function but GCC currently (11/2020) lacks this attribute. The barrier is
> + * used to inhibit tail calls in these cases.
> + */
> +#ifdef __has_attribute
> +#if __has_attribute(disable_tail_calls)
> +#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
> +#define NO_TAIL_CALL_BARRIER
> +#endif
> +#endif
> +#ifndef NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
> +#endif
> +
> static int mmap_handler(struct perf_tool *tool __maybe_unused,
> union perf_event *event,
> struct perf_sample *sample,
> @@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> return strcmp((const char *) symbol, funcs[idx]);
> }
>
> -noinline int test_dwarf_unwind__thread(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
> {
> struct perf_sample sample;
> unsigned long cnt = 0;
> @@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
>
> static int global_unwind_retval = -INT_MAX;
>
> -noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> {
> /* Any possible value should be 'thread' */
> struct thread *thread = *(struct thread **)p1;
> @@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> return p1 - p2;
> }
>
> -noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> {
> struct thread *array[2] = {thread, thread};
> void *fp = &bsearch;
> @@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> return global_unwind_retval;
> }
>
> -noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> {
> - return test_dwarf_unwind__krava_3(thread);
> + int ret;
> +
> + ret = test_dwarf_unwind__krava_3(thread);
> + NO_TAIL_CALL_BARRIER;
> + return ret;
> }
>
> -noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> {
> - return test_dwarf_unwind__krava_2(thread);
> + int ret;
> +
> + ret = test_dwarf_unwind__krava_2(thread);
> + NO_TAIL_CALL_BARRIER;
> + return ret;
> }
>
> int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
> --
> 2.33.0.464.g1972c5931b-goog
>
Em Thu, Sep 23, 2021 at 12:00:11AM +0200, Jiri Olsa escreveu:
> On Wed, Sep 22, 2021 at 10:38:12AM -0700, Ian Rogers wrote:
> > To ensure the stack frames are on the stack tail calls optimizations
> > need to be inhibited. If your compiler supports an attribute use it,
> > otherwise use an asm volatile barrier.
> >
> > The barrier fix was suggested here:
> > https://lore.kernel.org/lkml/[email protected]/
> > Tested with an optimized clang build and by forcing the asm barrier
> > route with an optimized clang build.
> >
> > A GCC bug tracking a proper disable_tail_calls is:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831
> >
> > Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
> > attribute")
> >
> > v2. is a rebase. The original fix patch generated quite a lot of
> > discussion over the right place for the fix:
> > https://lore.kernel.org/lkml/[email protected]/
> > The patch reflects my preference of it being near the use, so that
> > future code cleanups don't break this somewhat special usage.
>
> Acked-by: Jiri Olsa <[email protected]>
Thanks, applied.
- Arnaldo
> thanks,
> jirka
>
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
> > 1 file changed, 32 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> > index a288035eb362..c756284b3b13 100644
> > --- a/tools/perf/tests/dwarf-unwind.c
> > +++ b/tools/perf/tests/dwarf-unwind.c
> > @@ -20,6 +20,23 @@
> > /* For bsearch. We try to unwind functions in shared object. */
> > #include <stdlib.h>
> >
> > +/*
> > + * The test will assert frames are on the stack but tail call optimizations lose
> > + * the frame of the caller. Clang can disable this optimization on a called
> > + * function but GCC currently (11/2020) lacks this attribute. The barrier is
> > + * used to inhibit tail calls in these cases.
> > + */
> > +#ifdef __has_attribute
> > +#if __has_attribute(disable_tail_calls)
> > +#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
> > +#define NO_TAIL_CALL_BARRIER
> > +#endif
> > +#endif
> > +#ifndef NO_TAIL_CALL_ATTRIBUTE
> > +#define NO_TAIL_CALL_ATTRIBUTE
> > +#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
> > +#endif
> > +
> > static int mmap_handler(struct perf_tool *tool __maybe_unused,
> > union perf_event *event,
> > struct perf_sample *sample,
> > @@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> > return strcmp((const char *) symbol, funcs[idx]);
> > }
> >
> > -noinline int test_dwarf_unwind__thread(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
> > {
> > struct perf_sample sample;
> > unsigned long cnt = 0;
> > @@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
> >
> > static int global_unwind_retval = -INT_MAX;
> >
> > -noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> > {
> > /* Any possible value should be 'thread' */
> > struct thread *thread = *(struct thread **)p1;
> > @@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> > return p1 - p2;
> > }
> >
> > -noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> > {
> > struct thread *array[2] = {thread, thread};
> > void *fp = &bsearch;
> > @@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> > return global_unwind_retval;
> > }
> >
> > -noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> > {
> > - return test_dwarf_unwind__krava_3(thread);
> > + int ret;
> > +
> > + ret = test_dwarf_unwind__krava_3(thread);
> > + NO_TAIL_CALL_BARRIER;
> > + return ret;
> > }
> >
> > -noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> > {
> > - return test_dwarf_unwind__krava_2(thread);
> > + int ret;
> > +
> > + ret = test_dwarf_unwind__krava_2(thread);
> > + NO_TAIL_CALL_BARRIER;
> > + return ret;
> > }
> >
> > int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
> > --
> > 2.33.0.464.g1972c5931b-goog
> >
--
- Arnaldo