[PATCH] Fix extern inline in ftrace.h for ARM

[PATCH] Fix extern inline in ftrace.h for ARM

[behanw]

From: Behan Webster <behanw@converseincode.com>

The LLVMLinux Project is working to be able to build the Linux kernel with
clang/LLVM. With the release of LLVM 3.3 clang is now able to compile the Linux
kernel with a number of small patches (available from the LLVMLinux git repo).

This patch removes the use of "extern inline" from the ftrace code for ARM.
This same work has already been completed for ftrace on x86.  Amongst other
things this patch is required in order to have the Linux kernel to be able to
be compiled with both gcc and clang.

Mark Charlebois (1):
  ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

 arch/arm/include/asm/ftrace.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
1.8.1.2

[PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[behanw]

From: Mark Charlebois <charlebm@gmail.com>

With compilers which follow the C99 standard (like modern versions of gcc and
clang), "extern inline" does the wrong thing (emits code for an externally
linkable version of the inline function). In this case using the gnu_inline
attribute makes inline do the right thing on gcc and on clang.

Signed-off-by: Mark Charlebois <charlebm@gmail.com>
Signed-off-by: Behan Webster <behanw@converseincode.com>
---
 arch/arm/include/asm/ftrace.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index f89515a..fb7fdc4 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -45,7 +45,8 @@ void *return_address(unsigned int);
 
 #else
 
-extern inline void *return_address(unsigned int level)
+extern inline __attribute__((gnu_inline))
+void *return_address(unsigned int level)
 {
 	return NULL;
 }
-- 
1.8.1.2

Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[David Daney]

On 08/14/2013 02:37 PM, behanw@converseincode.com wrote:
> From: Mark Charlebois <charlebm@gmail.com>
>
> With compilers which follow the C99 standard (like modern versions of gcc and
> clang), "extern inline" does the wrong thing (emits code for an externally
> linkable version of the inline function). In this case using the gnu_inline
> attribute makes inline do the right thing on gcc and on clang.
>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> ---
>   arch/arm/include/asm/ftrace.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index f89515a..fb7fdc4 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -45,7 +45,8 @@ void *return_address(unsigned int);
>
>   #else
>
> -extern inline void *return_address(unsigned int level)
> +extern inline __attribute__((gnu_inline))

That seems very ugly.

Is it possible to put something in linux/compiler.h that encapsulates 
the desired semantics, and then use that instead?

We already define "inline" that way, if you need something else, put it 
in compiler.h with a nice symbolic name, and then use it.


> +void *return_address(unsigned int level)
>   {
>   	return NULL;
>   }
>

Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[Joe Perches]

On Wed, 2013-08-14 at 17:37 -0400, behanw@converseincode.com wrote:
> From: Mark Charlebois <charlebm@gmail.com>
> 
> With compilers which follow the C99 standard (like modern versions of gcc and
> clang), "extern inline" does the wrong thing (emits code for an externally
> linkable version of the inline function). In this case using the gnu_inline
> attribute makes inline do the right thing on gcc and on clang.

Why not convert these to static inline?

> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
[]
> @@ -45,7 +45,8 @@ void *return_address(unsigned int);
>  
>  #else
>  
> -extern inline void *return_address(unsigned int level)
> +extern inline __attribute__((gnu_inline))
> +void *return_address(unsigned int level)
>  {
>  	return NULL;
>  }

Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[Stephen Boyd]

On 08/14/13 14:56, Joe Perches wrote:
> On Wed, 2013-08-14 at 17:37 -0400, behanw@converseincode.com wrote:
>> From: Mark Charlebois <charlebm@gmail.com>
>>
>> With compilers which follow the C99 standard (like modern versions of gcc and
>> clang), "extern inline" does the wrong thing (emits code for an externally
>> linkable version of the inline function). In this case using the gnu_inline
>> attribute makes inline do the right thing on gcc and on clang.
> Why not convert these to static inline?

In this case we should probably just delete the entire thing and make it
unconditional in the header. It looks like it compiles with gcc still.

>
>> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> []
>> @@ -45,7 +45,8 @@ void *return_address(unsigned int);
>>  
>>  #else
>>  
>> -extern inline void *return_address(unsigned int level)
>> +extern inline __attribute__((gnu_inline))
>> +void *return_address(unsigned int level)
>>  {
>>  	return NULL;
>>  }
>
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[Russell King - ARM Linux]

On Wed, Aug 14, 2013 at 05:37:41PM -0400, behanw@converseincode.com wrote:
> From: Mark Charlebois <charlebm@gmail.com>
> 
> With compilers which follow the C99 standard (like modern versions of gcc and
> clang), "extern inline" does the wrong thing (emits code for an externally
> linkable version of the inline function). In this case using the gnu_inline
> attribute makes inline do the right thing on gcc and on clang.
> 
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> ---
>  arch/arm/include/asm/ftrace.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index f89515a..fb7fdc4 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -45,7 +45,8 @@ void *return_address(unsigned int);
>  
>  #else
>  
> -extern inline void *return_address(unsigned int level)
> +extern inline __attribute__((gnu_inline))
> +void *return_address(unsigned int level)

Well, that should be static inline, not extern inline in any case.  Does
clang work if that's static inline?

Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "gnu_inline" in ARM ftrace.h

[Behan Webster]

Sorry for the delay. A mistake in my email filters ate all your replies. 
Doh!

On 08/14/13 18:45, Russell King - ARM Linux wrote:
> On Wed, Aug 14, 2013 at 05:37:41PM -0400, behanw@converseincode.com wrote:
>> -extern inline void *return_address(unsigned int level)
>> +extern inline __attribute__((gnu_inline))
>> +void *return_address(unsigned int level)
> Well, that should be static inline, not extern inline in any case.  Does
> clang work if that's static inline?

Actually, neither gcc nor clang work with it merely changed to "static 
inline".

Which is why we left it with the explicit GNU89 meaning of "extern 
inline" which is gnu_inline. C99 changed the meaning of what "extern 
inline" means. One of the major issues we've had with the clang kernel 
port is that clang defaults to gnu99 (which is mostly just C99) while 
until recently gcc defaulted to gnu89.

For recent versions of gcc:

     http://gcc.gnu.org/onlinedocs/gcc/Standards.html

"The default, if no C language dialect options are given, is -std=gnu90; 
this will change to -std=gnu99 or -std=gnu11 in some future release when 
the C99 or C11 support is complete. Some features that are part of the 
C99 standard are accepted as extensions in C90 mode, and some features 
that are part of the C11 standard are accepted as extensions in C90 and 
C99 modes."

However, having said all that, it seems if I remove the corresponding 
NULL definition for return_address in arch/arm/kernel/return_address.c, 
I can make it "static inline" and it seems to work for both gcc and clang.

I'll send a new patch. :)

Incidentally the LLVMLinux project tests all the project's patches with 
both gcc and clang. The idea is to make it work with both compilers 
after all.

Behan

-- 
Behan Webster
behanw@converseincode.com