On Tue, 4 Feb 2025 15:19:45 +0100
Gabriel de Perthuis [off-list ref] 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 [off-list ref] a écrit :

quoted

On Mon, 3 Feb 2025 22:33:48 +0100
Gabriel de Perthuis [off-list ref] wrote:

quoted

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) [off-list ref]

-- 
Masami Hiramatsu (Google) [off-list ref]