[PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Haiyue Wang]

When switching from CONFIG_GENKSYMS to CONFIG_GENDWARFKSYMS enabled, the
build is failed:

  AS      arch/x86/entry/entry.o
In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
                 from <stdin>:3:
./arch/x86/include/asm/ftrace.h: In function ‘arch_ftrace_get_symaddr’:
./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function ‘get_kernel_nofault’ [-Werror=implicit-function-declaration]
   46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
      |                     ^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

File "asm-prototypes.h" is added to entry.S by 'scripts/Makefile.build',
adding the missed declaration header file can also fix the error:

 getasmexports =                                                        \
    { echo "\#include <linux/kernel.h>" ;                               \
      echo "\#include <linux/string.h>" ;                               \
+     echo "\#include <linux/uaccess.h>";                               \
      echo "\#include <asm/asm-prototypes.h>" ;                         \
      $(call getexportsymbols,EXPORT_SYMBOL(\1);) ; }

Fixes: 2bc56fdae1ba ("ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr")

Signed-off-by: Haiyue Wang <haiyuewa@163.com>
---
 arch/x86/include/asm/ftrace.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index f9cb4d07df58..063ce70837bb 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -34,6 +34,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 	return addr;
 }
 
+#ifdef CONFIG_X86_KERNEL_IBT
+#include <linux/uaccess.h>
+#endif
+
 static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 {
 #ifdef CONFIG_X86_KERNEL_IBT
-- 
2.48.1

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Sami Tolvanen]

Hi,

On Wed, Feb 5, 2025 at 10:02 AM Haiyue Wang <haiyuewa@163.com> wrote:
>
> When switching from CONFIG_GENKSYMS to CONFIG_GENDWARFKSYMS enabled, the
> build is failed:
>
>   AS      arch/x86/entry/entry.o
> In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
>                  from <stdin>:3:
> ./arch/x86/include/asm/ftrace.h: In function ‘arch_ftrace_get_symaddr’:
> ./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function ‘get_kernel_nofault’ [-Werror=implicit-function-declaration]
>    46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
>       |                     ^~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
>
> File "asm-prototypes.h" is added to entry.S by 'scripts/Makefile.build',
> adding the missed declaration header file can also fix the error:
>
>  getasmexports =                                                        \
>     { echo "\#include <linux/kernel.h>" ;                               \
>       echo "\#include <linux/string.h>" ;                               \
> +     echo "\#include <linux/uaccess.h>";                               \
>       echo "\#include <asm/asm-prototypes.h>" ;                         \
>       $(call getexportsymbols,EXPORT_SYMBOL(\1);) ; }
>
> Fixes: 2bc56fdae1ba ("ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr")
>
> Signed-off-by: Haiyue Wang <haiyuewa@163.com>
> ---
>  arch/x86/include/asm/ftrace.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index f9cb4d07df58..063ce70837bb 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -34,6 +34,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>         return addr;
>  }
>
> +#ifdef CONFIG_X86_KERNEL_IBT
> +#include <linux/uaccess.h>
> +#endif
> +
>  static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
>  {
>  #ifdef CONFIG_X86_KERNEL_IBT

Thanks for the patch! This looks correct to me, and I confirmed that
it fixes the build issue. Feel free to add:

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Tested-by: Sami Tolvanen <samitolvanen@google.com>

Sami

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Masami Hiramatsu]

On Thu,  6 Feb 2025 02:00:42 +0800
Haiyue Wang <haiyuewa@163.com> wrote:

> When switching from CONFIG_GENKSYMS to CONFIG_GENDWARFKSYMS enabled, the
> build is failed:
> 
>   AS      arch/x86/entry/entry.o
> In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
>                  from <stdin>:3:
> ./arch/x86/include/asm/ftrace.h: In function ‘arch_ftrace_get_symaddr’:
> ./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function ‘get_kernel_nofault’ [-Werror=implicit-function-declaration]
>    46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
>       |                     ^~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> 
> File "asm-prototypes.h" is added to entry.S by 'scripts/Makefile.build',
> adding the missed declaration header file can also fix the error:
> 
>  getasmexports =                                                        \
>     { echo "\#include <linux/kernel.h>" ;                               \
>       echo "\#include <linux/string.h>" ;                               \
> +     echo "\#include <linux/uaccess.h>";                               \
>       echo "\#include <asm/asm-prototypes.h>" ;                         \
>       $(call getexportsymbols,EXPORT_SYMBOL(\1);) ; }
> 
> Fixes: 2bc56fdae1ba ("ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr")

Thanks for the fix. And this fixes one of problems.
Another one is that `is_endbr()` is defined in <asm/ibt.h> but that also depend
on CONFIG_HAVE_FENTRY.

Let's pick this and fix above issue.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> 
> Signed-off-by: Haiyue Wang <haiyuewa@163.com>
> ---
>  arch/x86/include/asm/ftrace.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index f9cb4d07df58..063ce70837bb 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -34,6 +34,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  	return addr;
>  }
>  
> +#ifdef CONFIG_X86_KERNEL_IBT
> +#include <linux/uaccess.h>
> +#endif
> +
>  static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
>  {
>  #ifdef CONFIG_X86_KERNEL_IBT
> -- 
> 2.48.1
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Steven Rostedt]

On Thu, 6 Feb 2025 09:38:53 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Thanks for the fix. And this fixes one of problems.
> Another one is that `is_endbr()` is defined in <asm/ibt.h> but that also depend
> on CONFIG_HAVE_FENTRY.
> 
> Let's pick this and fix above issue.

OK, I'll pull this into my urgent tree and start testing it.

-- Steve

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Masami Hiramatsu]

On Wed, 5 Feb 2025 19:53:00 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 6 Feb 2025 09:38:53 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Thanks for the fix. And this fixes one of problems.
> > Another one is that `is_endbr()` is defined in <asm/ibt.h> but that also depend
> > on CONFIG_HAVE_FENTRY.
> > 
> > Let's pick this and fix above issue.
> 
> OK, I'll pull this into my urgent tree and start testing it.

Sorry, Gabriel found another issue.
Let me fix this instead of this patch.

https://lore.kernel.org/all/CAFEugPfFykYsHqS-J36CNWDn+d34bSQg-e5hJ=AaJPC8=6Ej-g@mail.gmail.com/

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Haiyue Wang]

On 2025/2/6 10:45, Masami Hiramatsu (Google) wrote:
> On Wed, 5 Feb 2025 19:53:00 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> On Thu, 6 Feb 2025 09:38:53 +0900
>> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>>
>>> Thanks for the fix. And this fixes one of problems.
>>> Another one is that `is_endbr()` is defined in <asm/ibt.h> but that also depend
>>> on CONFIG_HAVE_FENTRY.
>>>
>>> Let's pick this and fix above issue.
>>
>> OK, I'll pull this into my urgent tree and start testing it.
> 
> Sorry, Gabriel found another issue.
> Let me fix this instead of this patch.
> 
> https://lore.kernel.org/all/CAFEugPfFykYsHqS-J36CNWDn+d34bSQg-e5hJ=AaJPC8=6Ej-g@mail.gmail.com/
> 

If I put the fix above '#ifndef __ASSEMBLY__', then I can reproduce 
Gabriel's founding:

+#ifdef CONFIG_X86_KERNEL_IBT
+#include <linux/uaccess.h>
+#endif
+
  #ifndef __ASSEMBLY__


In file included from ./include/linux/kcsan-checks.h:13,
                  from ./include/linux/instrumented.h:12,
                  from ./include/linux/uaccess.h:6,
                  from ./arch/x86/include/asm/ftrace.h:26,
                  from arch/x86/kernel/ftrace_64.S:11:
./include/linux/compiler_attributes.h:91:20: error: missing binary 
operator before token "("
    91 | #if __has_attribute(__copy__)
       |                    ^
./include/linux/compiler_attributes.h:103:20: error: missing binary 
operator before token "("
   103 | #if __has_attribute(__diagnose_as_builtin__)
       |                    ^
./include/linux/compiler_attributes.h:126:20: error: missing binary 
operator before token "("
   126 | #if __has_attribute(__designated_init__)
       |                    ^
./include/linux/compiler_attributes.h:137:20: error: missing binary 
operator before token "("
   137 | #if __has_attribute(__error__)
       |                    ^


>>
>> -- Steve
> 
> 

Re: [PATCH v1] ftrace: Fix compile error when CONFIG_GENDWARFKSYMS is enabled

[Masami Hiramatsu]

On Thu, 6 Feb 2025 11:23:02 +0800
Haiyue Wang <haiyuewa@163.com> wrote:

> 
> 
> On 2025/2/6 10:45, Masami Hiramatsu (Google) wrote:
> > On Wed, 5 Feb 2025 19:53:00 -0500
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> >> On Thu, 6 Feb 2025 09:38:53 +0900
> >> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> >>
> >>> Thanks for the fix. And this fixes one of problems.
> >>> Another one is that `is_endbr()` is defined in <asm/ibt.h> but that also depend
> >>> on CONFIG_HAVE_FENTRY.
> >>>
> >>> Let's pick this and fix above issue.
> >>
> >> OK, I'll pull this into my urgent tree and start testing it.
> > 
> > Sorry, Gabriel found another issue.
> > Let me fix this instead of this patch.
> > 
> > https://lore.kernel.org/all/CAFEugPfFykYsHqS-J36CNWDn+d34bSQg-e5hJ=AaJPC8=6Ej-g@mail.gmail.com/
> > 
> 
> If I put the fix above '#ifndef __ASSEMBLY__', then I can reproduce 
> Gabriel's founding:
> 

Yeah, thanks. Anyway, I would not like introduce header file dependency
too much, especially header files under asm.
Please check below my fix;

https://lore.kernel.org/all/173881156244.211648.1242168038709680511.stgit@devnote2/

Thank you,

> +#ifdef CONFIG_X86_KERNEL_IBT
> +#include <linux/uaccess.h>
> +#endif
> +
>   #ifndef __ASSEMBLY__
> 
> 
> In file included from ./include/linux/kcsan-checks.h:13,
>                   from ./include/linux/instrumented.h:12,
>                   from ./include/linux/uaccess.h:6,
>                   from ./arch/x86/include/asm/ftrace.h:26,
>                   from arch/x86/kernel/ftrace_64.S:11:
> ./include/linux/compiler_attributes.h:91:20: error: missing binary 
> operator before token "("
>     91 | #if __has_attribute(__copy__)
>        |                    ^
> ./include/linux/compiler_attributes.h:103:20: error: missing binary 
> operator before token "("
>    103 | #if __has_attribute(__diagnose_as_builtin__)
>        |                    ^
> ./include/linux/compiler_attributes.h:126:20: error: missing binary 
> operator before token "("
>    126 | #if __has_attribute(__designated_init__)
>        |                    ^
> ./include/linux/compiler_attributes.h:137:20: error: missing binary 
> operator before token "("
>    137 | #if __has_attribute(__error__)
>        |                    ^
> 
> 
> >>
> >> -- Steve
> > 
> > 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>