[10/12] s390: avoid __builtin_return_address(n) on clang

Message ID 20190408212648.2407234-10-arnd@arndb.de
State New
Headers show
Series
  • [01/12] s390: remove -fno-strength-reduce flag
Related show

Commit Message

Arnd Bergmann April 8, 2019, 9:26 p.m.
llvm on s390 has problems with __builtin_return_address(n), with n>0,
this results in a somewhat cryptic error message:

fatal error: error in backend: Unsupported stack frame traversal count

To work around it, use the direct return address directly. This
is probably not ideal here, but gets things to compile and should
only lead to inferior reporting, not to misbehavior of the generated
code.

Link: https://bugs.llvm.org/show_bug.cgi?id=41424
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/s390/include/asm/ftrace.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.20.0

Comments

Martin Schwidefsky April 10, 2019, 4:03 p.m. | #1
On Mon,  8 Apr 2019 23:26:23 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> llvm on s390 has problems with __builtin_return_address(n), with n>0,

> this results in a somewhat cryptic error message:

> 

> fatal error: error in backend: Unsupported stack frame traversal count

> 

> To work around it, use the direct return address directly. This

> is probably not ideal here, but gets things to compile and should

> only lead to inferior reporting, not to misbehavior of the generated

> code.

> 

> Link: https://bugs.llvm.org/show_bug.cgi?id=41424

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  arch/s390/include/asm/ftrace.h | 5 +++++

>  1 file changed, 5 insertions(+)

> 

> diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h

> index 5a3c95b11952..7923c63946fb 100644

> --- a/arch/s390/include/asm/ftrace.h

> +++ b/arch/s390/include/asm/ftrace.h

> @@ -13,7 +13,12 @@

> 

>  #ifndef __ASSEMBLY__

> 

> +#ifdef CONFIG_CC_IS_CLANG

> +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */

> +#define ftrace_return_address(n) __builtin_return_address(0)

> +#else

>  #define ftrace_return_address(n) __builtin_return_address(n)

> +#endif

> 

>  void _mcount(void);

>  void ftrace_caller(void);


I can say I like this one. If the compiler can not do __builtin_return_address(n)
it feels wrong to just use __builtin_return_address(0).

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.
Steven Rostedt April 10, 2019, 4:14 p.m. | #2
On Wed, 10 Apr 2019 18:03:57 +0200
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> > --- a/arch/s390/include/asm/ftrace.h

> > +++ b/arch/s390/include/asm/ftrace.h

> > @@ -13,7 +13,12 @@

> > 

> >  #ifndef __ASSEMBLY__

> > 

> > +#ifdef CONFIG_CC_IS_CLANG

> > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */

> > +#define ftrace_return_address(n) __builtin_return_address(0)

> > +#else

> >  #define ftrace_return_address(n) __builtin_return_address(n)

> > +#endif

> > 

> >  void _mcount(void);

> >  void ftrace_caller(void);  

> 

> I can say I like this one. If the compiler can not do __builtin_return_address(n)

> it feels wrong to just use __builtin_return_address(0).


I agree. The proper return value is 0UL, see include/linux/ftrace.h

/* Archs may use other ways for ADDR1 and beyond */
#ifndef ftrace_return_address
# ifdef CONFIG_FRAME_POINTER
#  define ftrace_return_address(n) __builtin_return_address(n)
# else
#  define ftrace_return_address(n) 0UL
# endif
#endif

This is why we treat zero differently:

#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))


-- Steve
Arnd Bergmann April 10, 2019, 7:07 p.m. | #3
On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

>

> > > --- a/arch/s390/include/asm/ftrace.h

> > > +++ b/arch/s390/include/asm/ftrace.h

> > > @@ -13,7 +13,12 @@

> > >

> > >  #ifndef __ASSEMBLY__

> > >

> > > +#ifdef CONFIG_CC_IS_CLANG

> > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */

> > > +#define ftrace_return_address(n) __builtin_return_address(0)

> > > +#else

> > >  #define ftrace_return_address(n) __builtin_return_address(n)

> > > +#endif

> > >

> > >  void _mcount(void);

> > >  void ftrace_caller(void);

> >

> > I can say I like this one. If the compiler can not do __builtin_return_address(n)

> > it feels wrong to just use __builtin_return_address(0).

>

> I agree. The proper return value is 0UL, see include/linux/ftrace.h

>

> /* Archs may use other ways for ADDR1 and beyond */

> #ifndef ftrace_return_address

> # ifdef CONFIG_FRAME_POINTER

> #  define ftrace_return_address(n) __builtin_return_address(n)

> # else

> #  define ftrace_return_address(n) 0UL

> # endif

> #endif

>

> This is why we treat zero differently:

>

> #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)

> #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))

> #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))

> #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))

> #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))

> #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))

> #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))


Right, got it.

Martin, do you want me to send a replacement patch, or can you
commit the patch with

#ifdef CONFIG_CC_IS_CLANG
/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
#define ftrace_return_address(n) 0UL
#else
#define ftrace_return_address(n) __builtin_return_address(n)
#endif

instead?

    Arnd
Martin Schwidefsky April 11, 2019, 7:32 a.m. | #4
On Wed, 10 Apr 2019 21:07:56 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote:

> > On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> >  

> > > > --- a/arch/s390/include/asm/ftrace.h

> > > > +++ b/arch/s390/include/asm/ftrace.h

> > > > @@ -13,7 +13,12 @@

> > > >

> > > >  #ifndef __ASSEMBLY__

> > > >

> > > > +#ifdef CONFIG_CC_IS_CLANG

> > > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */

> > > > +#define ftrace_return_address(n) __builtin_return_address(0)

> > > > +#else

> > > >  #define ftrace_return_address(n) __builtin_return_address(n)

> > > > +#endif

> > > >

> > > >  void _mcount(void);

> > > >  void ftrace_caller(void);  

> > >

> > > I can say I like this one. If the compiler can not do __builtin_return_address(n)

> > > it feels wrong to just use __builtin_return_address(0).  

> >

> > I agree. The proper return value is 0UL, see include/linux/ftrace.h

> >

> > /* Archs may use other ways for ADDR1 and beyond */

> > #ifndef ftrace_return_address

> > # ifdef CONFIG_FRAME_POINTER

> > #  define ftrace_return_address(n) __builtin_return_address(n)

> > # else

> > #  define ftrace_return_address(n) 0UL

> > # endif

> > #endif

> >

> > This is why we treat zero differently:

> >

> > #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)

> > #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))

> > #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))

> > #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))

> > #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))

> > #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))

> > #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))  

> 

> Right, got it.

> 

> Martin, do you want me to send a replacement patch, or can you

> commit the patch with

> 

> #ifdef CONFIG_CC_IS_CLANG

> /* https://bugs.llvm.org/show_bug.cgi?id=41424 */

> #define ftrace_return_address(n) 0UL

> #else

> #define ftrace_return_address(n) __builtin_return_address(n)

> #endif

> 

> instead?


Ok, done.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Patch

diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 5a3c95b11952..7923c63946fb 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -13,7 +13,12 @@ 
 
 #ifndef __ASSEMBLY__
 
+#ifdef CONFIG_CC_IS_CLANG
+/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
+#define ftrace_return_address(n) __builtin_return_address(0)
+#else
 #define ftrace_return_address(n) __builtin_return_address(n)
+#endif
 
 void _mcount(void);
 void ftrace_caller(void);