Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 4 Feb 2025 15:19:45 +0100
Gabriel de Perthuis <g2p.code@xxxxxxxxx> wrote:

> Thank you for the prompt response!
> Sorry, this doesn't build.

Thanks for testing!
OK, so including linux/uaccess.h in asm/ftrace.h will cause another
issue. This is a good information!
I'll move the ftrace_call_adjust() to arch/x86/kernel/ftrace.c then.

Thank you!

> New, different errors:
> 
>   AS      arch/x86/kernel/ftrace_64.o
> 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:20,
>                  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__)
>       |                    ^
> ./include/linux/compiler_attributes.h:148:20: error: missing binary
> operator before token "("
>   148 | #if __has_attribute(__externally_visible__)
>       |                    ^
> ./include/linux/compiler_attributes.h:185:20: error: missing binary
> operator before token "("
>   185 | #if __has_attribute(__no_caller_saved_registers__)
>       |                    ^
> ./include/linux/compiler_attributes.h:196:20: error: missing binary
> operator before token "("
>   196 | #if __has_attribute(__noclone__)
>       |                    ^
> ./include/linux/compiler_attributes.h:213:20: error: missing binary
> operator before token "("
>   213 | #if __has_attribute(__fallthrough__)
>       |                    ^
> ./include/linux/compiler_attributes.h:239:20: error: missing binary
> operator before token "("
>   239 | #if __has_attribute(__nonstring__)
>       |                    ^
> ./include/linux/compiler_attributes.h:251:20: error: missing binary
> operator before token "("
>   251 | #if __has_attribute(__no_profile_instrument_function__)
>       |                    ^
> ./include/linux/compiler_attributes.h:270:20: error: missing binary
> operator before token "("
>   270 | #if __has_attribute(__no_stack_protector__)
>       |                    ^
> ./include/linux/compiler_attributes.h:281:20: error: missing binary
> operator before token "("
>   281 | #if __has_attribute(__overloadable__)
>       |                    ^
> ./include/linux/compiler_attributes.h:300:20: error: missing binary
> operator before token "("
>   300 | #if __has_attribute(__pass_dynamic_object_size__)
>       |                    ^
> ./include/linux/compiler_attributes.h:305:20: error: missing binary
> operator before token "("
>   305 | #if __has_attribute(__pass_object_size__)
>       |                    ^
> ./include/linux/compiler_attributes.h:329:20: error: missing binary
> operator before token "("
>   329 | #if __has_attribute(__uninitialized__)
>       |                    ^
> ./include/linux/compiler_attributes.h:375:20: error: missing binary
> operator before token "("
>   375 | #if __has_attribute(__warning__)
>       |                    ^
> ./include/linux/compiler_attributes.h:392:20: error: missing binary
> operator before token "("
>   392 | #if __has_attribute(disable_sanitizer_instrumentation)
>       |                    ^
> In file included from ./include/linux/mm_types_task.h:11,
>                  from ./include/linux/sched.h:38,
>                  from ./include/linux/uaccess.h:9:
> ./include/linux/align.h:8:9: warning: "ALIGN" redefined
>     8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
>       |         ^~~~~
> In file included from ./include/linux/export.h:6,
>                  from arch/x86/kernel/ftrace_64.S:6:
> ./include/linux/linkage.h:103:9: note: this is the location of the
> previous definition
>   103 | #define ALIGN __ALIGN
>       |         ^~~~~
> make[7]: *** [scripts/Makefile.build:339: arch/x86/kernel/ftrace_64.o] Error 1
> 
> Am attaching the .config so you can reproduce these easily.
> 
> Le mar. 4 févr. 2025 à 10:19, Masami Hiramatsu <mhiramat@xxxxxxxxxx> a écrit :
> >
> > On Mon, 3 Feb 2025 22:33:48 +0100
> > Gabriel de Perthuis <g2p.code@xxxxxxxxx> wrote:
> >
> > > Hello,
> > >
> > > I got errors building Linux 6.14-rc1 that were solved by reverting this
> > > patch and the one after (19/20 and 20/20).
> > >
> > > Errors look like:
> > >
> > > 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' [-Wimplicit-function-declaration]
> > >     46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip
> > > - ENDBR_INSN_SIZE)))
> > >        | ^~~~~~~~~~~~~~~~~~
> > >
> > > Will send .config on request if needed.
> >
> >
> > Thanks for the report!
> >
> > -------<arch/x86/include/asm/asm-prototypes.h>
> > /* SPDX-License-Identifier: GPL-2.0 */
> > #include <asm/ftrace.h>
> > #include <linux/uaccess.h>
> > -----
> >
> > Ah, that's why... get_kernel_nofault() is defined in linux/uaccess.h.
> > I also found that is_endbr() is in asm/ibt.h.
> >
> > Can you try this?
> >
> > Thank you,
> >
> > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> > index f9cb4d07df58..d24d7c71253f 100644
> > --- a/arch/x86/include/asm/ftrace.h
> > +++ b/arch/x86/include/asm/ftrace.h
> > @@ -16,24 +16,9 @@
> >  # include <asm/ibt.h>
> >  /* Add offset for endbr64 if IBT enabled */
> >  # define FTRACE_MCOUNT_MAX_OFFSET      ENDBR_INSN_SIZE
> > -#endif
> > -
> > -#ifdef CONFIG_DYNAMIC_FTRACE
> > -#define ARCH_SUPPORTS_FTRACE_OPS 1
> > -#endif
> > -
> > -#ifndef __ASSEMBLY__
> > -extern void __fentry__(void);
> > -
> > -static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > -{
> > -       /*
> > -        * addr is the address of the mcount call instruction.
> > -        * recordmcount does the necessary offset calculation.
> > -        */
> > -       return addr;
> > -}
> >
> > +#include <linux/uaccess.h>
> > +/* This only supports fentry based ftrace. */
> >  static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
> >  {
> >  #ifdef CONFIG_X86_KERNEL_IBT
> > @@ -55,6 +40,24 @@ static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
> >  }
> >  #define ftrace_get_symaddr(fentry_ip)  arch_ftrace_get_symaddr(fentry_ip)
> >
> > +#endif
> > +
> > +#ifdef CONFIG_DYNAMIC_FTRACE
> > +#define ARCH_SUPPORTS_FTRACE_OPS 1
> > +#endif
> > +
> > +#ifndef __ASSEMBLY__
> > +extern void __fentry__(void);
> > +
> > +static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > +{
> > +       /*
> > +        * addr is the address of the mcount call instruction.
> > +        * recordmcount does the necessary offset calculation.
> > +        */
> > +       return addr;
> > +}
> > +
> >  #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> >
> >  #include <linux/ftrace_regs.h>
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>




[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux