From: Behan Webster <[email protected]>
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
From: Mark Charlebois <[email protected]>
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 <[email protected]>
Signed-off-by: Behan Webster <[email protected]>
---
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
On 08/14/2013 02:37 PM, [email protected] wrote:
> From: Mark Charlebois <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Behan Webster <[email protected]>
> ---
> 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;
> }
>
On Wed, 2013-08-14 at 17:37 -0400, [email protected] wrote:
> From: Mark Charlebois <[email protected]>
>
> 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;
> }
On 08/14/13 14:56, Joe Perches wrote:
> On Wed, 2013-08-14 at 17:37 -0400, [email protected] wrote:
>> From: Mark Charlebois <[email protected]>
>>
>> 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
On Wed, Aug 14, 2013 at 05:37:41PM -0400, [email protected] wrote:
> From: Mark Charlebois <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Behan Webster <[email protected]>
> ---
> 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?
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, [email protected] 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
[email protected]
From: Behan Webster <[email protected]>
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 static inline
and removing the NULL version of return_address in return_address.c does
the right thing.
Signed-off-by: Behan Webster <[email protected]>
---
arch/arm/include/asm/ftrace.h | 2 +-
arch/arm/kernel/return_address.c | 5 -----
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index f89515a..2bb8cac 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -45,7 +45,7 @@ void *return_address(unsigned int);
#else
-extern inline void *return_address(unsigned int level)
+static inline void *return_address(unsigned int level)
{
return NULL;
}
diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
index fafedd8..f6aa84d 100644
--- a/arch/arm/kernel/return_address.c
+++ b/arch/arm/kernel/return_address.c
@@ -63,11 +63,6 @@ void *return_address(unsigned int level)
#warning "TODO: return_address should use unwind tables"
#endif
-void *return_address(unsigned int level)
-{
- return NULL;
-}
-
#endif /* if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND) / else */
EXPORT_SYMBOL_GPL(return_address);
--
1.8.1.2