All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*'
       [not found] <20171013052544.euk7yawni47lhmdq@gmail.com>
@ 2017-10-13 20:02 ` Josh Poimboeuf
  2017-10-14 10:49   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
  2017-10-13 20:02 ` [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit Josh Poimboeuf
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2017-10-13 20:02 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, torvalds, peterz, hpa, tglx

Rename the unwinder config options from:

  CONFIG_ORC_UNWINDER
  CONFIG_FRAME_POINTER_UNWINDER
  CONFIG_GUESS_UNWINDER

to:

  CONFIG_UNWINDER_ORC
  CONFIG_UNWINDER_FRAME_POINTER
  CONFIG_UNWINDER_GUESS

... in order to give them a more logical config namespace.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/x86/orc-unwinder.txt |  2 +-
 Makefile                           |  4 ++--
 arch/x86/Kconfig                   |  2 +-
 arch/x86/Kconfig.debug             | 10 +++++-----
 arch/x86/configs/tiny.config       |  4 ++--
 arch/x86/configs/x86_64_defconfig  |  2 +-
 arch/x86/include/asm/module.h      |  2 +-
 arch/x86/include/asm/unwind.h      |  8 ++++----
 arch/x86/kernel/Makefile           |  6 +++---
 include/asm-generic/vmlinux.lds.h  |  2 +-
 lib/Kconfig.debug                  |  2 +-
 scripts/Makefile.build             |  2 +-
 12 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/Documentation/x86/orc-unwinder.txt b/Documentation/x86/orc-unwinder.txt
index af0c9a4c65a6..cd4b29be29af 100644
--- a/Documentation/x86/orc-unwinder.txt
+++ b/Documentation/x86/orc-unwinder.txt
@@ -4,7 +4,7 @@ ORC unwinder
 Overview
 --------
 
-The kernel CONFIG_ORC_UNWINDER option enables the ORC unwinder, which is
+The kernel CONFIG_UNWINDER_ORC option enables the ORC unwinder, which is
 similar in concept to a DWARF unwinder.  The difference is that the
 format of the ORC data is much simpler than DWARF, which in turn allows
 the ORC unwinder to be much simpler and faster.
diff --git a/Makefile b/Makefile
index 5bf6fa4d62d8..1ef636bebccc 100644
--- a/Makefile
+++ b/Makefile
@@ -933,8 +933,8 @@ ifdef CONFIG_STACK_VALIDATION
   ifeq ($(has_libelf),1)
     objtool_target := tools/objtool FORCE
   else
-    ifdef CONFIG_ORC_UNWINDER
-      $(error "Cannot generate ORC metadata for CONFIG_ORC_UNWINDER=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
+    ifdef CONFIG_UNWINDER_ORC
+      $(error "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
     else
       $(warning "Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
     endif
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 544273594c61..018556b375de 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -171,7 +171,7 @@ config X86
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_REGS_AND_STACK_ACCESS_API
-	select HAVE_RELIABLE_STACKTRACE		if X86_64 && FRAME_POINTER_UNWINDER && STACK_VALIDATION
+	select HAVE_RELIABLE_STACKTRACE		if X86_64 && UNWINDER_FRAME_POINTER && STACK_VALIDATION
 	select HAVE_STACK_VALIDATION		if X86_64
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_UNSTABLE_SCHED_CLOCK
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 71a48a30fc84..f274dbb87c26 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -358,13 +358,13 @@ config PUNIT_ATOM_DEBUG
 
 choice
 	prompt "Choose kernel unwinder"
-	default FRAME_POINTER_UNWINDER
+	default UNWINDER_FRAME_POINTER
 	---help---
 	  This determines which method will be used for unwinding kernel stack
 	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
 	  livepatch, lockdep, and more.
 
-config FRAME_POINTER_UNWINDER
+config UNWINDER_FRAME_POINTER
 	bool "Frame pointer unwinder"
 	select FRAME_POINTER
 	---help---
@@ -379,7 +379,7 @@ config FRAME_POINTER_UNWINDER
 	  consistency model, as this is currently the only way to get a
 	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
 
-config ORC_UNWINDER
+config UNWINDER_ORC
 	bool "ORC unwinder"
 	depends on X86_64
 	select STACK_VALIDATION
@@ -395,7 +395,7 @@ config ORC_UNWINDER
 	  Enabling this option will increase the kernel's runtime memory usage
 	  by roughly 2-4MB, depending on your kernel config.
 
-config GUESS_UNWINDER
+config UNWINDER_GUESS
 	bool "Guess unwinder"
 	depends on EXPERT
 	---help---
@@ -410,7 +410,7 @@ config GUESS_UNWINDER
 endchoice
 
 config FRAME_POINTER
-	depends on !ORC_UNWINDER && !GUESS_UNWINDER
+	depends on !UNWINDER_ORC && !UNWINDER_GUESS
 	bool
 
 endmenu
diff --git a/arch/x86/configs/tiny.config b/arch/x86/configs/tiny.config
index 550cd5012b73..66c9e2aab16c 100644
--- a/arch/x86/configs/tiny.config
+++ b/arch/x86/configs/tiny.config
@@ -1,5 +1,5 @@
 CONFIG_NOHIGHMEM=y
 # CONFIG_HIGHMEM4G is not set
 # CONFIG_HIGHMEM64G is not set
-CONFIG_GUESS_UNWINDER=y
-# CONFIG_FRAME_POINTER_UNWINDER is not set
+CONFIG_UNWINDER_GUESS=y
+# CONFIG_UNWINDER_FRAME_POINTER is not set
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index eb65c248708d..e32fc1f274d8 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -299,7 +299,7 @@ CONFIG_DEBUG_STACKOVERFLOW=y
 # CONFIG_DEBUG_RODATA_TEST is not set
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_OPTIMIZE_INLINING=y
-CONFIG_ORC_UNWINDER=y
+CONFIG_UNWINDER_ORC=y
 CONFIG_SECURITY=y
 CONFIG_SECURITY_NETWORK=y
 CONFIG_SECURITY_SELINUX=y
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 9eb7c718aaf8..9f05a1002aa9 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -5,7 +5,7 @@
 #include <asm/orc_types.h>
 
 struct mod_arch_specific {
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 	unsigned int num_orcs;
 	int *orc_unwind_ip;
 	struct orc_entry *orc_unwind;
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index e9f793e2df7a..35d67dc7b69f 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -12,11 +12,11 @@ struct unwind_state {
 	struct task_struct *task;
 	int graph_idx;
 	bool error;
-#if defined(CONFIG_ORC_UNWINDER)
+#if defined(CONFIG_UNWINDER_ORC)
 	bool signal, full_regs;
 	unsigned long sp, bp, ip;
 	struct pt_regs *regs;
-#elif defined(CONFIG_FRAME_POINTER_UNWINDER)
+#elif defined(CONFIG_UNWINDER_FRAME_POINTER)
 	bool got_irq;
 	unsigned long *bp, *orig_sp, ip;
 	struct pt_regs *regs;
@@ -50,7 +50,7 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,
 	__unwind_start(state, task, regs, first_frame);
 }
 
-#if defined(CONFIG_ORC_UNWINDER) || defined(CONFIG_FRAME_POINTER_UNWINDER)
+#if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
 static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 {
 	if (unwind_done(state))
@@ -65,7 +65,7 @@ static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 }
 #endif
 
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 void unwind_init(void);
 void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
 			void *orc, size_t orc_size);
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index d8e2b700d1db..5335d05b695e 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -126,9 +126,9 @@ obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
 obj-$(CONFIG_SCHED_MC_PRIO)		+= itmt.o
 
-obj-$(CONFIG_ORC_UNWINDER)		+= unwind_orc.o
-obj-$(CONFIG_FRAME_POINTER_UNWINDER)	+= unwind_frame.o
-obj-$(CONFIG_GUESS_UNWINDER)		+= unwind_guess.o
+obj-$(CONFIG_UNWINDER_ORC)		+= unwind_orc.o
+obj-$(CONFIG_UNWINDER_FRAME_POINTER)	+= unwind_frame.o
+obj-$(CONFIG_UNWINDER_GUESS)		+= unwind_guess.o
 
 ###
 # 64 bit specific files
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index e549bff87c5b..353f52fdc35e 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -688,7 +688,7 @@
 #define BUG_TABLE
 #endif
 
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 #define ORC_UNWIND_TABLE						\
 	. = ALIGN(4);							\
 	.orc_unwind_ip : AT(ADDR(.orc_unwind_ip) - LOAD_OFFSET) {	\
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c6401d325b0e..01792bb77199 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -422,7 +422,7 @@ config STACK_VALIDATION
 	  that runtime stack traces are more reliable.
 
 	  This is also a prerequisite for generation of ORC unwind data, which
-	  is needed for CONFIG_ORC_UNWINDER.
+	  is needed for CONFIG_UNWINDER_ORC.
 
 	  For more information, see
 	  tools/objtool/Documentation/stack-validation.txt.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 061d0c3a420a..f965f477832e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -258,7 +258,7 @@ ifneq ($(SKIP_STACK_VALIDATION),1)
 
 __objtool_obj := $(objtree)/tools/objtool/objtool
 
-objtool_args = $(if $(CONFIG_ORC_UNWINDER),orc generate,check)
+objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
 
 ifndef CONFIG_FRAME_POINTER
 objtool_args += --no-fp
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
       [not found] <20171013052544.euk7yawni47lhmdq@gmail.com>
  2017-10-13 20:02 ` [PATCH 1/2] x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*' Josh Poimboeuf
@ 2017-10-13 20:02 ` Josh Poimboeuf
  2017-10-14 10:50   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
                     ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Josh Poimboeuf @ 2017-10-13 20:02 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, torvalds, peterz, hpa, tglx

The ORC unwinder has been stable in testing so far.  Give it much wider
testing by making it the default in kconfig for x86_64.  It's not yet
supported for 32-bit, so leave frame pointers as the default there.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/Kconfig.debug | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index f274dbb87c26..a4ff214fb760 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -358,27 +358,13 @@ config PUNIT_ATOM_DEBUG
 
 choice
 	prompt "Choose kernel unwinder"
-	default UNWINDER_FRAME_POINTER
+	default UNWINDER_ORC if X86_64
+	default UNWINDER_FRAME_POINTER if X86_32
 	---help---
 	  This determines which method will be used for unwinding kernel stack
 	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
 	  livepatch, lockdep, and more.
 
-config UNWINDER_FRAME_POINTER
-	bool "Frame pointer unwinder"
-	select FRAME_POINTER
-	---help---
-	  This option enables the frame pointer unwinder for unwinding kernel
-	  stack traces.
-
-	  The unwinder itself is fast and it uses less RAM than the ORC
-	  unwinder, but the kernel text size will grow by ~3% and the kernel's
-	  overall performance will degrade by roughly 5-10%.
-
-	  This option is recommended if you want to use the livepatch
-	  consistency model, as this is currently the only way to get a
-	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
-
 config UNWINDER_ORC
 	bool "ORC unwinder"
 	depends on X86_64
@@ -395,6 +381,21 @@ config UNWINDER_ORC
 	  Enabling this option will increase the kernel's runtime memory usage
 	  by roughly 2-4MB, depending on your kernel config.
 
+config UNWINDER_FRAME_POINTER
+	bool "Frame pointer unwinder"
+	select FRAME_POINTER
+	---help---
+	  This option enables the frame pointer unwinder for unwinding kernel
+	  stack traces.
+
+	  The unwinder itself is fast and it uses less RAM than the ORC
+	  unwinder, but the kernel text size will grow by ~3% and the kernel's
+	  overall performance will degrade by roughly 5-10%.
+
+	  This option is recommended if you want to use the livepatch
+	  consistency model, as this is currently the only way to get a
+	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
+
 config UNWINDER_GUESS
 	bool "Guess unwinder"
 	depends on EXPERT
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip:x86/asm] x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*'
  2017-10-13 20:02 ` [PATCH 1/2] x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*' Josh Poimboeuf
@ 2017-10-14 10:49   ` tip-bot for Josh Poimboeuf
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-10-14 10:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, torvalds, mingo, hpa, jpoimboe, tglx, linux-kernel

Commit-ID:  11af847446ed0d131cf24d16a7ef3d5ea7a49554
Gitweb:     https://git.kernel.org/tip/11af847446ed0d131cf24d16a7ef3d5ea7a49554
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 13 Oct 2017 15:02:00 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 14 Oct 2017 10:12:12 +0200

x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*'

Rename the unwinder config options from:

  CONFIG_ORC_UNWINDER
  CONFIG_FRAME_POINTER_UNWINDER
  CONFIG_GUESS_UNWINDER

to:

  CONFIG_UNWINDER_ORC
  CONFIG_UNWINDER_FRAME_POINTER
  CONFIG_UNWINDER_GUESS

... in order to give them a more logical config namespace.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/73972fc7e2762e91912c6b9584582703d6f1b8cc.1507924831.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 Documentation/x86/orc-unwinder.txt |  2 +-
 Makefile                           |  4 ++--
 arch/x86/Kconfig                   |  2 +-
 arch/x86/Kconfig.debug             | 10 +++++-----
 arch/x86/configs/tiny.config       |  4 ++--
 arch/x86/configs/x86_64_defconfig  |  2 +-
 arch/x86/include/asm/module.h      |  2 +-
 arch/x86/include/asm/unwind.h      |  8 ++++----
 arch/x86/kernel/Makefile           |  6 +++---
 include/asm-generic/vmlinux.lds.h  |  2 +-
 lib/Kconfig.debug                  |  2 +-
 scripts/Makefile.build             |  2 +-
 12 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/Documentation/x86/orc-unwinder.txt b/Documentation/x86/orc-unwinder.txt
index af0c9a4..cd4b29b 100644
--- a/Documentation/x86/orc-unwinder.txt
+++ b/Documentation/x86/orc-unwinder.txt
@@ -4,7 +4,7 @@ ORC unwinder
 Overview
 --------
 
-The kernel CONFIG_ORC_UNWINDER option enables the ORC unwinder, which is
+The kernel CONFIG_UNWINDER_ORC option enables the ORC unwinder, which is
 similar in concept to a DWARF unwinder.  The difference is that the
 format of the ORC data is much simpler than DWARF, which in turn allows
 the ORC unwinder to be much simpler and faster.
diff --git a/Makefile b/Makefile
index bc5c79e..c0f723f 100644
--- a/Makefile
+++ b/Makefile
@@ -933,8 +933,8 @@ ifdef CONFIG_STACK_VALIDATION
   ifeq ($(has_libelf),1)
     objtool_target := tools/objtool FORCE
   else
-    ifdef CONFIG_ORC_UNWINDER
-      $(error "Cannot generate ORC metadata for CONFIG_ORC_UNWINDER=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
+    ifdef CONFIG_UNWINDER_ORC
+      $(error "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
     else
       $(warning "Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
     endif
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 971feac..6b94ca0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -170,7 +170,7 @@ config X86
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_REGS_AND_STACK_ACCESS_API
-	select HAVE_RELIABLE_STACKTRACE		if X86_64 && FRAME_POINTER_UNWINDER && STACK_VALIDATION
+	select HAVE_RELIABLE_STACKTRACE		if X86_64 && UNWINDER_FRAME_POINTER && STACK_VALIDATION
 	select HAVE_STACK_VALIDATION		if X86_64
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_UNSTABLE_SCHED_CLOCK
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 71a48a3..f274dbb 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -358,13 +358,13 @@ config PUNIT_ATOM_DEBUG
 
 choice
 	prompt "Choose kernel unwinder"
-	default FRAME_POINTER_UNWINDER
+	default UNWINDER_FRAME_POINTER
 	---help---
 	  This determines which method will be used for unwinding kernel stack
 	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
 	  livepatch, lockdep, and more.
 
-config FRAME_POINTER_UNWINDER
+config UNWINDER_FRAME_POINTER
 	bool "Frame pointer unwinder"
 	select FRAME_POINTER
 	---help---
@@ -379,7 +379,7 @@ config FRAME_POINTER_UNWINDER
 	  consistency model, as this is currently the only way to get a
 	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
 
-config ORC_UNWINDER
+config UNWINDER_ORC
 	bool "ORC unwinder"
 	depends on X86_64
 	select STACK_VALIDATION
@@ -395,7 +395,7 @@ config ORC_UNWINDER
 	  Enabling this option will increase the kernel's runtime memory usage
 	  by roughly 2-4MB, depending on your kernel config.
 
-config GUESS_UNWINDER
+config UNWINDER_GUESS
 	bool "Guess unwinder"
 	depends on EXPERT
 	---help---
@@ -410,7 +410,7 @@ config GUESS_UNWINDER
 endchoice
 
 config FRAME_POINTER
-	depends on !ORC_UNWINDER && !GUESS_UNWINDER
+	depends on !UNWINDER_ORC && !UNWINDER_GUESS
 	bool
 
 endmenu
diff --git a/arch/x86/configs/tiny.config b/arch/x86/configs/tiny.config
index 550cd50..66c9e2a 100644
--- a/arch/x86/configs/tiny.config
+++ b/arch/x86/configs/tiny.config
@@ -1,5 +1,5 @@
 CONFIG_NOHIGHMEM=y
 # CONFIG_HIGHMEM4G is not set
 # CONFIG_HIGHMEM64G is not set
-CONFIG_GUESS_UNWINDER=y
-# CONFIG_FRAME_POINTER_UNWINDER is not set
+CONFIG_UNWINDER_GUESS=y
+# CONFIG_UNWINDER_FRAME_POINTER is not set
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index eb65c24..e32fc1f 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -299,7 +299,7 @@ CONFIG_DEBUG_STACKOVERFLOW=y
 # CONFIG_DEBUG_RODATA_TEST is not set
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_OPTIMIZE_INLINING=y
-CONFIG_ORC_UNWINDER=y
+CONFIG_UNWINDER_ORC=y
 CONFIG_SECURITY=y
 CONFIG_SECURITY_NETWORK=y
 CONFIG_SECURITY_SELINUX=y
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 9eb7c71..9f05a10 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -5,7 +5,7 @@
 #include <asm/orc_types.h>
 
 struct mod_arch_specific {
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 	unsigned int num_orcs;
 	int *orc_unwind_ip;
 	struct orc_entry *orc_unwind;
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index e9f793e..35d67dc 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -12,11 +12,11 @@ struct unwind_state {
 	struct task_struct *task;
 	int graph_idx;
 	bool error;
-#if defined(CONFIG_ORC_UNWINDER)
+#if defined(CONFIG_UNWINDER_ORC)
 	bool signal, full_regs;
 	unsigned long sp, bp, ip;
 	struct pt_regs *regs;
-#elif defined(CONFIG_FRAME_POINTER_UNWINDER)
+#elif defined(CONFIG_UNWINDER_FRAME_POINTER)
 	bool got_irq;
 	unsigned long *bp, *orig_sp, ip;
 	struct pt_regs *regs;
@@ -50,7 +50,7 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,
 	__unwind_start(state, task, regs, first_frame);
 }
 
-#if defined(CONFIG_ORC_UNWINDER) || defined(CONFIG_FRAME_POINTER_UNWINDER)
+#if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
 static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 {
 	if (unwind_done(state))
@@ -65,7 +65,7 @@ static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 }
 #endif
 
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 void unwind_init(void);
 void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
 			void *orc, size_t orc_size);
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index fd0a789..6209ab6 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -127,9 +127,9 @@ obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
 obj-$(CONFIG_SCHED_MC_PRIO)		+= itmt.o
 
-obj-$(CONFIG_ORC_UNWINDER)		+= unwind_orc.o
-obj-$(CONFIG_FRAME_POINTER_UNWINDER)	+= unwind_frame.o
-obj-$(CONFIG_GUESS_UNWINDER)		+= unwind_guess.o
+obj-$(CONFIG_UNWINDER_ORC)		+= unwind_orc.o
+obj-$(CONFIG_UNWINDER_FRAME_POINTER)	+= unwind_frame.o
+obj-$(CONFIG_UNWINDER_GUESS)		+= unwind_guess.o
 
 ###
 # 64 bit specific files
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8acfc1e..63e56f6 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -687,7 +687,7 @@
 #define BUG_TABLE
 #endif
 
-#ifdef CONFIG_ORC_UNWINDER
+#ifdef CONFIG_UNWINDER_ORC
 #define ORC_UNWIND_TABLE						\
 	. = ALIGN(4);							\
 	.orc_unwind_ip : AT(ADDR(.orc_unwind_ip) - LOAD_OFFSET) {	\
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2689b7c..7566eff 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -376,7 +376,7 @@ config STACK_VALIDATION
 	  that runtime stack traces are more reliable.
 
 	  This is also a prerequisite for generation of ORC unwind data, which
-	  is needed for CONFIG_ORC_UNWINDER.
+	  is needed for CONFIG_UNWINDER_ORC.
 
 	  For more information, see
 	  tools/objtool/Documentation/stack-validation.txt.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 061d0c3..f965f47 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -258,7 +258,7 @@ ifneq ($(SKIP_STACK_VALIDATION),1)
 
 __objtool_obj := $(objtree)/tools/objtool/objtool
 
-objtool_args = $(if $(CONFIG_ORC_UNWINDER),orc generate,check)
+objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
 
 ifndef CONFIG_FRAME_POINTER
 objtool_args += --no-fp

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip:x86/asm] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-13 20:02 ` [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit Josh Poimboeuf
@ 2017-10-14 10:50   ` tip-bot for Josh Poimboeuf
  2017-10-19 16:51   ` [2/2] " Andrei Vagin
  2018-03-19 18:57   ` [PATCH 2/2] " Matthias Kaehlcke
  2 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-10-14 10:50 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, tglx, hpa, mingo, jpoimboe, linux-kernel, torvalds

Commit-ID:  fc72ae40e30327aa24eb88a24b9c7058f938bd36
Gitweb:     https://git.kernel.org/tip/fc72ae40e30327aa24eb88a24b9c7058f938bd36
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 13 Oct 2017 15:02:01 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 14 Oct 2017 10:12:12 +0200

x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit

The ORC unwinder has been stable in testing so far.  Give it much wider
testing by making it the default in kconfig for x86_64.  It's not yet
supported for 32-bit, so leave frame pointers as the default there.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/9b1237bbe7244ed9cdf8db2dcb1253e37e1c341e.1507924831.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/Kconfig.debug | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index f274dbb..a4ff214 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -358,27 +358,13 @@ config PUNIT_ATOM_DEBUG
 
 choice
 	prompt "Choose kernel unwinder"
-	default UNWINDER_FRAME_POINTER
+	default UNWINDER_ORC if X86_64
+	default UNWINDER_FRAME_POINTER if X86_32
 	---help---
 	  This determines which method will be used for unwinding kernel stack
 	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
 	  livepatch, lockdep, and more.
 
-config UNWINDER_FRAME_POINTER
-	bool "Frame pointer unwinder"
-	select FRAME_POINTER
-	---help---
-	  This option enables the frame pointer unwinder for unwinding kernel
-	  stack traces.
-
-	  The unwinder itself is fast and it uses less RAM than the ORC
-	  unwinder, but the kernel text size will grow by ~3% and the kernel's
-	  overall performance will degrade by roughly 5-10%.
-
-	  This option is recommended if you want to use the livepatch
-	  consistency model, as this is currently the only way to get a
-	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
-
 config UNWINDER_ORC
 	bool "ORC unwinder"
 	depends on X86_64
@@ -395,6 +381,21 @@ config UNWINDER_ORC
 	  Enabling this option will increase the kernel's runtime memory usage
 	  by roughly 2-4MB, depending on your kernel config.
 
+config UNWINDER_FRAME_POINTER
+	bool "Frame pointer unwinder"
+	select FRAME_POINTER
+	---help---
+	  This option enables the frame pointer unwinder for unwinding kernel
+	  stack traces.
+
+	  The unwinder itself is fast and it uses less RAM than the ORC
+	  unwinder, but the kernel text size will grow by ~3% and the kernel's
+	  overall performance will degrade by roughly 5-10%.
+
+	  This option is recommended if you want to use the livepatch
+	  consistency model, as this is currently the only way to get a
+	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
+
 config UNWINDER_GUESS
 	bool "Guess unwinder"
 	depends on EXPERT

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-13 20:02 ` [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit Josh Poimboeuf
  2017-10-14 10:50   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
@ 2017-10-19 16:51   ` Andrei Vagin
  2017-10-19 18:16     ` Josh Poimboeuf
  2018-03-19 18:57   ` [PATCH 2/2] " Matthias Kaehlcke
  2 siblings, 1 reply; 19+ messages in thread
From: Andrei Vagin @ 2017-10-19 16:51 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

Hi,

We run CRIU tests for tip/auto-latest regularly, and a few days ago our
test job started to detect this warning in a kernel log:

[   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b

What does it mean? How critical is it?

Our test job fails if it detects any warning in a kernel log. Maybe we
need to investigate reasons of this warning and try to eliminate it?

Here are logs:
https://travis-ci.org/avagin/linux/jobs/289676634

Thanks,
Andrei

On Fri, Oct 13, 2017 at 03:02:01PM -0500, Josh Poimboeuf wrote:
> The ORC unwinder has been stable in testing so far.  Give it much wider
> testing by making it the default in kconfig for x86_64.  It's not yet
> supported for 32-bit, so leave frame pointers as the default there.
> 
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  arch/x86/Kconfig.debug | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
> index f274dbb87c26..a4ff214fb760 100644
> --- a/arch/x86/Kconfig.debug
> +++ b/arch/x86/Kconfig.debug
> @@ -358,27 +358,13 @@ config PUNIT_ATOM_DEBUG
>  
>  choice
>  	prompt "Choose kernel unwinder"
> -	default UNWINDER_FRAME_POINTER
> +	default UNWINDER_ORC if X86_64
> +	default UNWINDER_FRAME_POINTER if X86_32
>  	---help---
>  	  This determines which method will be used for unwinding kernel stack
>  	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
>  	  livepatch, lockdep, and more.
>  
> -config UNWINDER_FRAME_POINTER
> -	bool "Frame pointer unwinder"
> -	select FRAME_POINTER
> -	---help---
> -	  This option enables the frame pointer unwinder for unwinding kernel
> -	  stack traces.
> -
> -	  The unwinder itself is fast and it uses less RAM than the ORC
> -	  unwinder, but the kernel text size will grow by ~3% and the kernel's
> -	  overall performance will degrade by roughly 5-10%.
> -
> -	  This option is recommended if you want to use the livepatch
> -	  consistency model, as this is currently the only way to get a
> -	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
> -
>  config UNWINDER_ORC
>  	bool "ORC unwinder"
>  	depends on X86_64
> @@ -395,6 +381,21 @@ config UNWINDER_ORC
>  	  Enabling this option will increase the kernel's runtime memory usage
>  	  by roughly 2-4MB, depending on your kernel config.
>  
> +config UNWINDER_FRAME_POINTER
> +	bool "Frame pointer unwinder"
> +	select FRAME_POINTER
> +	---help---
> +	  This option enables the frame pointer unwinder for unwinding kernel
> +	  stack traces.
> +
> +	  The unwinder itself is fast and it uses less RAM than the ORC
> +	  unwinder, but the kernel text size will grow by ~3% and the kernel's
> +	  overall performance will degrade by roughly 5-10%.
> +
> +	  This option is recommended if you want to use the livepatch
> +	  consistency model, as this is currently the only way to get a
> +	  reliable stack trace (CONFIG_HAVE_RELIABLE_STACKTRACE).
> +
>  config UNWINDER_GUESS
>  	bool "Guess unwinder"
>  	depends on EXPERT

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-19 16:51   ` [2/2] " Andrei Vagin
@ 2017-10-19 18:16     ` Josh Poimboeuf
  2017-10-19 22:35       ` Andrei Vagin
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2017-10-19 18:16 UTC (permalink / raw)
  To: Andrei Vagin; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Thu, Oct 19, 2017 at 09:51:04AM -0700, Andrei Vagin wrote:
> Hi,
> 
> We run CRIU tests for tip/auto-latest regularly, and a few days ago our
> test job started to detect this warning in a kernel log:
> 
> [   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> 
> What does it mean? How critical is it?
> 
> Our test job fails if it detects any warning in a kernel log. Maybe we
> need to investigate reasons of this warning and try to eliminate it?
> 
> Here are logs:
> https://travis-ci.org/avagin/linux/jobs/289676634

I think it means the unwinder found some bad ORC unwinder metadata.  Any
chance you have access to the kernel binary?  I need to know what code
corresponds to that ffffffff95f0d94b address.

Or if you can reproduce with the following patch, that should help:


diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 570b70d3f604..95b633f0ce51 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_TYPE_REGS_IRET:
 		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
-			orc_warn("can't dereference iret registers at %p for ip %p\n",
+			orc_warn("can't dereference iret registers at %p for ip %pB\n",
 				 (void *)sp, (void *)orig_ip);
 			goto done;
 		}

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-19 18:16     ` Josh Poimboeuf
@ 2017-10-19 22:35       ` Andrei Vagin
  2017-10-20  0:38         ` Andrei Vagin
  2017-10-20  1:28         ` Josh Poimboeuf
  0 siblings, 2 replies; 19+ messages in thread
From: Andrei Vagin @ 2017-10-19 22:35 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Thu, Oct 19, 2017 at 01:16:55PM -0500, Josh Poimboeuf wrote:
> On Thu, Oct 19, 2017 at 09:51:04AM -0700, Andrei Vagin wrote:
> > Hi,
> > 
> > We run CRIU tests for tip/auto-latest regularly, and a few days ago our
> > test job started to detect this warning in a kernel log:
> > 
> > [   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> > 
> > What does it mean? How critical is it?
> > 
> > Our test job fails if it detects any warning in a kernel log. Maybe we
> > need to investigate reasons of this warning and try to eliminate it?
> > 
> > Here are logs:
> > https://travis-ci.org/avagin/linux/jobs/289676634
> 
> I think it means the unwinder found some bad ORC unwinder metadata.  Any
> chance you have access to the kernel binary?  I need to know what code
> corresponds to that ffffffff95f0d94b address.
> 
> Or if you can reproduce with the following patch, that should help:
> 
> 
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index 570b70d3f604..95b633f0ce51 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
>  
>  	case ORC_TYPE_REGS_IRET:
>  		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
> -			orc_warn("can't dereference iret registers at %p for ip %p\n",
> +			orc_warn("can't dereference iret registers at %p for ip %pB\n",
>  				 (void *)sp, (void *)orig_ip);
>  			goto done;
>  		}

I applied your patch and rerun tests.

[   44.947699] WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60

and now here is a warning from kasan:

[  477.775676] ==================================================================
[  477.775845] BUG: KASAN: stack-out-of-bounds in deref_stack_reg+0x11d/0x150
[  477.775952] Read of size 8 at addr ffff880166b7fe90 by task make/16028
[  477.776055] 
[  477.776149] CPU: 0 PID: 16028 Comm: make Not tainted 4.14.0-rc5+ #1
[  477.776152] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[  477.776155] Call Trace:
[  477.776159]  <IRQ>
[  477.776167]  dump_stack+0x5c/0x7e
[  477.776175]  print_address_description+0x6b/0x290
[  477.776182]  ? deref_stack_reg+0x11d/0x150
[  477.776186]  kasan_report+0x25d/0x340
[  477.776194]  deref_stack_reg+0x11d/0x150
[  477.776201]  ? __read_once_size_nocheck.constprop.6+0x10/0x10
[  477.776206]  ? get_stack_info+0x37/0x170
[  477.776212]  ? stack_access_ok+0xdc/0x150
[  477.776221]  unwind_next_frame+0xe35/0x1c10
[  477.776230]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776238]  ? deref_stack_reg+0x150/0x150
[  477.776247]  ? is_bpf_text_address+0x54/0x60
[  477.776253]  ? kernel_text_address+0xf4/0x100
[  477.776257]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776266]  __save_stack_trace+0x73/0xd0
[  477.776277]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776285]  save_stack+0x33/0xb0
[  477.776291]  ? kasan_slab_free+0x70/0xc0
[  477.776298]  ? kmem_cache_free+0x9f/0x230
[  477.776303]  ? rcu_process_callbacks+0x451/0xd60
[  477.776307]  ? __do_softirq+0x1d3/0x5e0
[  477.776312]  ? irq_exit+0x146/0x170
[  477.776322]  ? smp_apic_timer_interrupt+0x13e/0x3b0
[  477.776326]  ? apic_timer_interrupt+0x8c/0xa0
[  477.776331]  ? lock_acquire+0x6b/0x260
[  477.776336]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776347]  ? update_curr+0x2d6/0x600
[  477.776354]  ? posix_cpu_timers_exit_group+0x50/0x50
[  477.776365]  ? trigger_load_balance+0x1fd/0x8a0
[  477.776374]  ? note_gp_changes+0x14e/0x1b0
[  477.776384]  ? lock_downgrade+0x590/0x590
[  477.776389]  ? rcu_accelerate_cbs+0x106/0x5e0
[  477.776398]  ? lock_acquire+0x113/0x260
[  477.776402]  ? rcu_process_callbacks+0x407/0xd60
[  477.776407]  kasan_slab_free+0x70/0xc0
[  477.776414]  ? rcu_process_callbacks+0x451/0xd60
[  477.776418]  kmem_cache_free+0x9f/0x230
[  477.776425]  ? free_inode_nonrcu+0x20/0x20
[  477.776430]  rcu_process_callbacks+0x451/0xd60
[  477.776443]  ? note_gp_changes+0x1b0/0x1b0
[  477.776451]  ? native_apic_msr_write+0x27/0x30
[  477.776456]  ? lapic_next_event+0x55/0x80
[  477.776465]  __do_softirq+0x1d3/0x5e0
[  477.776479]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776483]  irq_exit+0x146/0x170
[  477.776487]  smp_apic_timer_interrupt+0x13e/0x3b0
[  477.776494]  apic_timer_interrupt+0x8c/0xa0
[  477.776497]  </IRQ>
[  477.776502] RIP: 0010:lock_acquire+0x6b/0x260
[  477.776505] RSP: 0018:ffff880166b7fd48 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff11
[  477.776512] RAX: 0000000000000007 RBX: ffff8801c91cb080 RCX: 0000000000000000
[  477.776515] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8801c91cb8b4
[  477.776518] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
[  477.776521] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[  477.776524] R13: 0000000000000000 R14: 0000000000000000 R15: ffffffff9f651fce
[  477.776528]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776552]  do_execveat_common.isra.34+0x78e/0x1890
[  477.776559]  ? fs_reclaim_acquire.part.71+0x29/0x30
[  477.776564]  ? fs_reclaim_acquire.part.71+0x5/0x30
[  477.776569]  ? kmem_cache_alloc+0x29/0x1f0
[  477.776577]  ? do_execveat_common.isra.34+0x78e/0x1890
[  477.776589]  ? strncpy_from_user+0x74/0x260
[  477.776595]  ? prepare_bprm_creds+0x100/0x100
[  477.776599]  ? kmem_cache_alloc+0x18d/0x1f0
[  477.776607]  ? getname_flags+0xff/0x500
[  477.776615]  ? SyS_execve+0x2c/0x40
[  477.776623]  ? ptregs_sys_vfork+0x10/0x10
[  477.776628]  ? do_syscall_64+0x181/0x450
[  477.776638]  ? entry_SYSCALL64_slow_path+0x25/0x25
[  477.776653] 
[  477.776747] The buggy address belongs to the page:
[  477.776849] page:ffffea00059adfc0 count:0 mapcount:0 mapping:          (null) index:0x0
[  477.776968] flags: 0x17fff8000000000()
[  477.777067] raw: 017fff8000000000 0000000000000000 0000000000000000 00000000ffffffff
[  477.777184] raw: 0000000000000000 dead000000000200 0000000000000000 0000000000000000
[  477.777298] page dumped because: kasan: bad access detected
[  477.777404] 
[  477.777494] Memory state around the buggy address:
[  477.777594]  ffff880166b7fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  477.777709]  ffff880166b7fe00: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
[  477.777823] >ffff880166b7fe80: f4 f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00
[  477.777937]                          ^
[  477.778034]  ffff880166b7ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  477.778147]  ffff880166b7ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  477.778260] ==================================================================
[  477.778376] Disabling lock debugging due to kernel taint

All logs are here https://travis-ci.org/avagin/linux/jobs/290190646

Unfortunately vmlinux was not saved for this run. Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-19 22:35       ` Andrei Vagin
@ 2017-10-20  0:38         ` Andrei Vagin
  2017-10-20  1:28         ` Josh Poimboeuf
  1 sibling, 0 replies; 19+ messages in thread
From: Andrei Vagin @ 2017-10-20  0:38 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Thu, Oct 19, 2017 at 03:35:22PM -0700, Andrei Vagin wrote:
> On Thu, Oct 19, 2017 at 01:16:55PM -0500, Josh Poimboeuf wrote:
> > On Thu, Oct 19, 2017 at 09:51:04AM -0700, Andrei Vagin wrote:
> > > Hi,
> > > 
> > > We run CRIU tests for tip/auto-latest regularly, and a few days ago our
> > > test job started to detect this warning in a kernel log:
> > > 
> > > [   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> > > 
> > > What does it mean? How critical is it?
> > > 
> > > Our test job fails if it detects any warning in a kernel log. Maybe we
> > > need to investigate reasons of this warning and try to eliminate it?
> > > 
> > > Here are logs:
> > > https://travis-ci.org/avagin/linux/jobs/289676634
> > 
> > I think it means the unwinder found some bad ORC unwinder metadata.  Any
> > chance you have access to the kernel binary?  I need to know what code
> > corresponds to that ffffffff95f0d94b address.
> > 
> > Or if you can reproduce with the following patch, that should help:
> > 
> > 
> > diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> > index 570b70d3f604..95b633f0ce51 100644
> > --- a/arch/x86/kernel/unwind_orc.c
> > +++ b/arch/x86/kernel/unwind_orc.c
> > @@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
> >  
> >  	case ORC_TYPE_REGS_IRET:
> >  		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
> > -			orc_warn("can't dereference iret registers at %p for ip %p\n",
> > +			orc_warn("can't dereference iret registers at %p for ip %pB\n",
> >  				 (void *)sp, (void *)orig_ip);
> >  			goto done;
> >  		}
> 
> I applied your patch and rerun tests.
> 
> [   44.947699] WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60
> 
> and now here is a warning from kasan:
> 
> [  477.775676] ==================================================================
> [  477.775845] BUG: KASAN: stack-out-of-bounds in deref_stack_reg+0x11d/0x150
> [  477.775952] Read of size 8 at addr ffff880166b7fe90 by task make/16028
> [  477.776055] 
> [  477.776149] CPU: 0 PID: 16028 Comm: make Not tainted 4.14.0-rc5+ #1
> [  477.776152] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> [  477.776155] Call Trace:
> [  477.776159]  <IRQ>
> [  477.776167]  dump_stack+0x5c/0x7e
> [  477.776175]  print_address_description+0x6b/0x290
> [  477.776182]  ? deref_stack_reg+0x11d/0x150
> [  477.776186]  kasan_report+0x25d/0x340
> [  477.776194]  deref_stack_reg+0x11d/0x150
> [  477.776201]  ? __read_once_size_nocheck.constprop.6+0x10/0x10
> [  477.776206]  ? get_stack_info+0x37/0x170
> [  477.776212]  ? stack_access_ok+0xdc/0x150
> [  477.776221]  unwind_next_frame+0xe35/0x1c10
> [  477.776230]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776238]  ? deref_stack_reg+0x150/0x150
> [  477.776247]  ? is_bpf_text_address+0x54/0x60
> [  477.776253]  ? kernel_text_address+0xf4/0x100
> [  477.776257]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776266]  __save_stack_trace+0x73/0xd0
> [  477.776277]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776285]  save_stack+0x33/0xb0
> [  477.776291]  ? kasan_slab_free+0x70/0xc0
> [  477.776298]  ? kmem_cache_free+0x9f/0x230
> [  477.776303]  ? rcu_process_callbacks+0x451/0xd60
> [  477.776307]  ? __do_softirq+0x1d3/0x5e0
> [  477.776312]  ? irq_exit+0x146/0x170
> [  477.776322]  ? smp_apic_timer_interrupt+0x13e/0x3b0
> [  477.776326]  ? apic_timer_interrupt+0x8c/0xa0
> [  477.776331]  ? lock_acquire+0x6b/0x260
> [  477.776336]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776347]  ? update_curr+0x2d6/0x600
> [  477.776354]  ? posix_cpu_timers_exit_group+0x50/0x50
> [  477.776365]  ? trigger_load_balance+0x1fd/0x8a0
> [  477.776374]  ? note_gp_changes+0x14e/0x1b0
> [  477.776384]  ? lock_downgrade+0x590/0x590
> [  477.776389]  ? rcu_accelerate_cbs+0x106/0x5e0
> [  477.776398]  ? lock_acquire+0x113/0x260
> [  477.776402]  ? rcu_process_callbacks+0x407/0xd60
> [  477.776407]  kasan_slab_free+0x70/0xc0
> [  477.776414]  ? rcu_process_callbacks+0x451/0xd60
> [  477.776418]  kmem_cache_free+0x9f/0x230
> [  477.776425]  ? free_inode_nonrcu+0x20/0x20
> [  477.776430]  rcu_process_callbacks+0x451/0xd60
> [  477.776443]  ? note_gp_changes+0x1b0/0x1b0
> [  477.776451]  ? native_apic_msr_write+0x27/0x30
> [  477.776456]  ? lapic_next_event+0x55/0x80
> [  477.776465]  __do_softirq+0x1d3/0x5e0
> [  477.776479]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776483]  irq_exit+0x146/0x170
> [  477.776487]  smp_apic_timer_interrupt+0x13e/0x3b0
> [  477.776494]  apic_timer_interrupt+0x8c/0xa0
> [  477.776497]  </IRQ>
> [  477.776502] RIP: 0010:lock_acquire+0x6b/0x260
> [  477.776505] RSP: 0018:ffff880166b7fd48 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff11
> [  477.776512] RAX: 0000000000000007 RBX: ffff8801c91cb080 RCX: 0000000000000000
> [  477.776515] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8801c91cb8b4
> [  477.776518] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
> [  477.776521] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [  477.776524] R13: 0000000000000000 R14: 0000000000000000 R15: ffffffff9f651fce
> [  477.776528]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776552]  do_execveat_common.isra.34+0x78e/0x1890
> [  477.776559]  ? fs_reclaim_acquire.part.71+0x29/0x30
> [  477.776564]  ? fs_reclaim_acquire.part.71+0x5/0x30
> [  477.776569]  ? kmem_cache_alloc+0x29/0x1f0
> [  477.776577]  ? do_execveat_common.isra.34+0x78e/0x1890
> [  477.776589]  ? strncpy_from_user+0x74/0x260
> [  477.776595]  ? prepare_bprm_creds+0x100/0x100
> [  477.776599]  ? kmem_cache_alloc+0x18d/0x1f0
> [  477.776607]  ? getname_flags+0xff/0x500
> [  477.776615]  ? SyS_execve+0x2c/0x40
> [  477.776623]  ? ptregs_sys_vfork+0x10/0x10
> [  477.776628]  ? do_syscall_64+0x181/0x450
> [  477.776638]  ? entry_SYSCALL64_slow_path+0x25/0x25
> [  477.776653] 
> [  477.776747] The buggy address belongs to the page:
> [  477.776849] page:ffffea00059adfc0 count:0 mapcount:0 mapping:          (null) index:0x0
> [  477.776968] flags: 0x17fff8000000000()
> [  477.777067] raw: 017fff8000000000 0000000000000000 0000000000000000 00000000ffffffff
> [  477.777184] raw: 0000000000000000 dead000000000200 0000000000000000 0000000000000000
> [  477.777298] page dumped because: kasan: bad access detected
> [  477.777404] 
> [  477.777494] Memory state around the buggy address:
> [  477.777594]  ffff880166b7fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  477.777709]  ffff880166b7fe00: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
> [  477.777823] >ffff880166b7fe80: f4 f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00
> [  477.777937]                          ^
> [  477.778034]  ffff880166b7ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  477.778147]  ffff880166b7ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  477.778260] ==================================================================
> [  477.778376] Disabling lock debugging due to kernel taint
> 
> All logs are here https://travis-ci.org/avagin/linux/jobs/290190646
> 
> Unfortunately vmlinux was not saved for this run. Thanks.

Here is vmlinux:
https://www.dropbox.com/s/e70u6oxxj4pwe2h/vmlinux?dl=0

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-19 22:35       ` Andrei Vagin
  2017-10-20  0:38         ` Andrei Vagin
@ 2017-10-20  1:28         ` Josh Poimboeuf
  2017-10-20  6:54           ` Andrei Vagin
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2017-10-20  1:28 UTC (permalink / raw)
  To: Andrei Vagin; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Thu, Oct 19, 2017 at 03:35:22PM -0700, Andrei Vagin wrote:
> On Thu, Oct 19, 2017 at 01:16:55PM -0500, Josh Poimboeuf wrote:
> > On Thu, Oct 19, 2017 at 09:51:04AM -0700, Andrei Vagin wrote:
> > > Hi,
> > > 
> > > We run CRIU tests for tip/auto-latest regularly, and a few days ago our
> > > test job started to detect this warning in a kernel log:
> > > 
> > > [   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> > > 
> > > What does it mean? How critical is it?
> > > 
> > > Our test job fails if it detects any warning in a kernel log. Maybe we
> > > need to investigate reasons of this warning and try to eliminate it?
> > > 
> > > Here are logs:
> > > https://travis-ci.org/avagin/linux/jobs/289676634
> > 
> > I think it means the unwinder found some bad ORC unwinder metadata.  Any
> > chance you have access to the kernel binary?  I need to know what code
> > corresponds to that ffffffff95f0d94b address.
> > 
> > Or if you can reproduce with the following patch, that should help:
> > 
> > 
> > diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> > index 570b70d3f604..95b633f0ce51 100644
> > --- a/arch/x86/kernel/unwind_orc.c
> > +++ b/arch/x86/kernel/unwind_orc.c
> > @@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
> >  
> >  	case ORC_TYPE_REGS_IRET:
> >  		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
> > -			orc_warn("can't dereference iret registers at %p for ip %p\n",
> > +			orc_warn("can't dereference iret registers at %p for ip %pB\n",
> >  				 (void *)sp, (void *)orig_ip);
> >  			goto done;
> >  		}
> 
> I applied your patch and rerun tests.
> 
> [   44.947699] WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60

Thanks, that was enough for me to figure it out.  Can you test the below fix?

> and now here is a warning from kasan:
> 
> [  477.775676] ==================================================================
> [  477.775845] BUG: KASAN: stack-out-of-bounds in deref_stack_reg+0x11d/0x150

The KASAN warning is a known issue for which the fix is a little more
complicated.  v1 of the patch was here:

  https://lkml.kernel.org/r/cover.1507128293.git.jpoimboe@redhat.com



diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 49167258d587..f6cdb7a1455e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -808,7 +808,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
 
 .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
 ENTRY(\sym)
-	UNWIND_HINT_IRET_REGS offset=8
+	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 
 	/* Sanity check */
 	.if \shift_ist != -1 && \paranoid == 0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-20  1:28         ` Josh Poimboeuf
@ 2017-10-20  6:54           ` Andrei Vagin
  0 siblings, 0 replies; 19+ messages in thread
From: Andrei Vagin @ 2017-10-20  6:54 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Thu, Oct 19, 2017 at 08:28:04PM -0500, Josh Poimboeuf wrote:
> On Thu, Oct 19, 2017 at 03:35:22PM -0700, Andrei Vagin wrote:
> > On Thu, Oct 19, 2017 at 01:16:55PM -0500, Josh Poimboeuf wrote:
> > > On Thu, Oct 19, 2017 at 09:51:04AM -0700, Andrei Vagin wrote:
> > > > Hi,
> > > > 
> > > > We run CRIU tests for tip/auto-latest regularly, and a few days ago our
> > > > test job started to detect this warning in a kernel log:
> > > > 
> > > > [   44.235786] WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> > > > 
> > > > What does it mean? How critical is it?
> > > > 
> > > > Our test job fails if it detects any warning in a kernel log. Maybe we
> > > > need to investigate reasons of this warning and try to eliminate it?
> > > > 
> > > > Here are logs:
> > > > https://travis-ci.org/avagin/linux/jobs/289676634
> > > 
> > > I think it means the unwinder found some bad ORC unwinder metadata.  Any
> > > chance you have access to the kernel binary?  I need to know what code
> > > corresponds to that ffffffff95f0d94b address.
> > > 
> > > Or if you can reproduce with the following patch, that should help:
> > > 
> > > 
> > > diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> > > index 570b70d3f604..95b633f0ce51 100644
> > > --- a/arch/x86/kernel/unwind_orc.c
> > > +++ b/arch/x86/kernel/unwind_orc.c
> > > @@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
> > >  
> > >  	case ORC_TYPE_REGS_IRET:
> > >  		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
> > > -			orc_warn("can't dereference iret registers at %p for ip %p\n",
> > > +			orc_warn("can't dereference iret registers at %p for ip %pB\n",
> > >  				 (void *)sp, (void *)orig_ip);
> > >  			goto done;
> > >  		}
> > 
> > I applied your patch and rerun tests.
> > 
> > [   44.947699] WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60
> 
> Thanks, that was enough for me to figure it out.  Can you test the below fix?

This patch works for me. I run tests a few times and they found nothing
suspicious.

Tested-by: Andrei Vagin <avagin@virtuozzo.com>

Thank you!

> 
> > and now here is a warning from kasan:
> > 
> > [  477.775676] ==================================================================
> > [  477.775845] BUG: KASAN: stack-out-of-bounds in deref_stack_reg+0x11d/0x150
> 
> The KASAN warning is a known issue for which the fix is a little more
> complicated.  v1 of the patch was here:
> 
>   https://lkml.kernel.org/r/cover.1507128293.git.jpoimboe@redhat.com
> 
> 
> 
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 49167258d587..f6cdb7a1455e 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -808,7 +808,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
>  
>  .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
>  ENTRY(\sym)
> -	UNWIND_HINT_IRET_REGS offset=8
> +	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
>  
>  	/* Sanity check */
>  	.if \shift_ist != -1 && \paranoid == 0
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2017-10-13 20:02 ` [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit Josh Poimboeuf
  2017-10-14 10:50   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
  2017-10-19 16:51   ` [2/2] " Andrei Vagin
@ 2018-03-19 18:57   ` Matthias Kaehlcke
  2018-03-19 19:29     ` Josh Poimboeuf
  2 siblings, 1 reply; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-03-19 18:57 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

Hi Josh,

El Fri, Oct 13, 2017 at 03:02:01PM -0500 Josh Poimboeuf ha dit:

> The ORC unwinder has been stable in testing so far.  Give it much wider
> testing by making it the default in kconfig for x86_64.  It's not yet
> supported for 32-bit, so leave frame pointers as the default there.
> 
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---

Building an upstream kernel with clang results in plenty of objtool
warnings like these:

drivers/gpu/drm/i915/dvo_ch7017.o: warning: objtool: ch7017_get_hw_state()+0x80: return with modified stack frame
  CC      drivers/gpu/drm/i915/i915_oa_cflgt2.o
...
  CC      drivers/gpu/drm/i915/intel_lpe_audio.o
drivers/gpu/drm/i915/i915_gpu_error.o: warning: objtool: i915_error_printf()+0x6c: return with modified stack frame
drivers/gpu/drm/i915/intel_display.o: warning: objtool: pipe_config_err()+0xa6: return with modified stack frame

Bisecting shows that the warnings are generated since the ORC unwinder
was made the default. Not sure if the issue is on the unwinder side
or clang.

Any ideas on what could be the problems?

Thanks

Matthias

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 18:57   ` [PATCH 2/2] " Matthias Kaehlcke
@ 2018-03-19 19:29     ` Josh Poimboeuf
  2018-03-19 20:31       ` Matthias Kaehlcke
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2018-03-19 19:29 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Mon, Mar 19, 2018 at 11:57:32AM -0700, Matthias Kaehlcke wrote:
> Hi Josh,
> 
> El Fri, Oct 13, 2017 at 03:02:01PM -0500 Josh Poimboeuf ha dit:
> 
> > The ORC unwinder has been stable in testing so far.  Give it much wider
> > testing by making it the default in kconfig for x86_64.  It's not yet
> > supported for 32-bit, so leave frame pointers as the default there.
> > 
> > Suggested-by: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> 
> Building an upstream kernel with clang results in plenty of objtool
> warnings like these:
> 
> drivers/gpu/drm/i915/dvo_ch7017.o: warning: objtool: ch7017_get_hw_state()+0x80: return with modified stack frame
>   CC      drivers/gpu/drm/i915/i915_oa_cflgt2.o
> ...
>   CC      drivers/gpu/drm/i915/intel_lpe_audio.o
> drivers/gpu/drm/i915/i915_gpu_error.o: warning: objtool: i915_error_printf()+0x6c: return with modified stack frame
> drivers/gpu/drm/i915/intel_display.o: warning: objtool: pipe_config_err()+0xa6: return with modified stack frame
> 
> Bisecting shows that the warnings are generated since the ORC unwinder
> was made the default. Not sure if the issue is on the unwinder side
> or clang.
> 
> Any ideas on what could be the problems?

Hi Matthias,

The ORC unwinder relies on objtool, which reverse engineers the compiled
code.  This is objtool's first exposure to clang, so I'm not at all
surprised if it's getting confused.

Send me one of the .o files and I can take a quick look to see how bad
it is, but I'm guessing it's going to be a lot of work to make objtool
compatible with clang (and unfortunately I won't have the bandwidth to
work on that in the near term.)

In the meantime I'd recommend that you use frame pointers (and
CONFIG_STACK_VALIDATION=n) for clang-compiled kernels.

-- 
Josh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 19:29     ` Josh Poimboeuf
@ 2018-03-19 20:31       ` Matthias Kaehlcke
  2018-03-19 21:20         ` Josh Poimboeuf
  0 siblings, 1 reply; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-03-19 20:31 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]

El Mon, Mar 19, 2018 at 02:29:10PM -0500 Josh Poimboeuf ha dit:

> On Mon, Mar 19, 2018 at 11:57:32AM -0700, Matthias Kaehlcke wrote:
> > Hi Josh,
> > 
> > El Fri, Oct 13, 2017 at 03:02:01PM -0500 Josh Poimboeuf ha dit:
> > 
> > > The ORC unwinder has been stable in testing so far.  Give it much wider
> > > testing by making it the default in kconfig for x86_64.  It's not yet
> > > supported for 32-bit, so leave frame pointers as the default there.
> > > 
> > > Suggested-by: Ingo Molnar <mingo@kernel.org>
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > > ---
> > 
> > Building an upstream kernel with clang results in plenty of objtool
> > warnings like these:
> > 
> > drivers/gpu/drm/i915/dvo_ch7017.o: warning: objtool: ch7017_get_hw_state()+0x80: return with modified stack frame
> >   CC      drivers/gpu/drm/i915/i915_oa_cflgt2.o
> > ...
> >   CC      drivers/gpu/drm/i915/intel_lpe_audio.o
> > drivers/gpu/drm/i915/i915_gpu_error.o: warning: objtool: i915_error_printf()+0x6c: return with modified stack frame
> > drivers/gpu/drm/i915/intel_display.o: warning: objtool: pipe_config_err()+0xa6: return with modified stack frame
> > 
> > Bisecting shows that the warnings are generated since the ORC unwinder
> > was made the default. Not sure if the issue is on the unwinder side
> > or clang.
> > 
> > Any ideas on what could be the problems?
> 
> Hi Matthias,
> 
> The ORC unwinder relies on objtool, which reverse engineers the compiled
> code.  This is objtool's first exposure to clang, so I'm not at all
> surprised if it's getting confused.
> 
> Send me one of the .o files and I can take a quick look to see how bad
> it is, but I'm guessing it's going to be a lot of work to make objtool
> compatible with clang (and unfortunately I won't have the bandwidth to
> work on that in the near term.)
> 
> In the meantime I'd recommend that you use frame pointers (and
> CONFIG_STACK_VALIDATION=n) for clang-compiled kernels.

Thanks for your assessment!

dvo_ch7017.o is attached.

[-- Attachment #2: dvo_ch7017.o --]
[-- Type: application/x-object, Size: 8960 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 20:31       ` Matthias Kaehlcke
@ 2018-03-19 21:20         ` Josh Poimboeuf
  2018-03-19 23:22           ` Matthias Kaehlcke
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2018-03-19 21:20 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Mon, Mar 19, 2018 at 01:31:30PM -0700, Matthias Kaehlcke wrote:
> > The ORC unwinder relies on objtool, which reverse engineers the compiled
> > code.  This is objtool's first exposure to clang, so I'm not at all
> > surprised if it's getting confused.
> > 
> > Send me one of the .o files and I can take a quick look to see how bad
> > it is, but I'm guessing it's going to be a lot of work to make objtool
> > compatible with clang (and unfortunately I won't have the bandwidth to
> > work on that in the near term.)
> > 
> > In the meantime I'd recommend that you use frame pointers (and
> > CONFIG_STACK_VALIDATION=n) for clang-compiled kernels.
> 
> Thanks for your assessment!
> 
> dvo_ch7017.o is attached.

Here's a (surprisingly easy) fix for this particular issue, though I'd
be shocked if there weren't a bunch more issues lurking elsewhere.  Let
me know how it goes.

BTW, one thing I noticed in the .o file is that most of the functions'
stacks are aligned to 16 bytes.  It might be worth checking if the clang
-mstack-alignment=8 option is getting set, and if so, if it's working
properly.  Otherwise, with aligned stacks, the frame pointer is forced,
which defeats most of the benefits of ORC.

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 92b6a2c21631..f02df714c18e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1386,6 +1386,17 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
 				state->vals[op->dest.reg].offset = -state->stack_size;
 			}
 
+			else if (op->src.reg == CFI_BP && op->dest.reg == CFI_SP &&
+				 cfa->base == CFI_BP) {
+
+				/*
+				 * mov %rbp, %rsp
+				 *
+				 * Restore the original stack pointer (clang).
+				 */
+				state->stack_size = -state->regs[CFI_BP].offset;
+			}
+
 			else if (op->dest.reg == cfa->base) {
 
 				/* mov %reg, %rsp */

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 21:20         ` Josh Poimboeuf
@ 2018-03-19 23:22           ` Matthias Kaehlcke
  2018-03-20  2:28             ` Josh Poimboeuf
  2018-03-21  2:45             ` Josh Poimboeuf
  0 siblings, 2 replies; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-03-19 23:22 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]

El Mon, Mar 19, 2018 at 04:20:53PM -0500 Josh Poimboeuf ha dit:

> On Mon, Mar 19, 2018 at 01:31:30PM -0700, Matthias Kaehlcke wrote:
> > > The ORC unwinder relies on objtool, which reverse engineers the compiled
> > > code.  This is objtool's first exposure to clang, so I'm not at all
> > > surprised if it's getting confused.
> > > 
> > > Send me one of the .o files and I can take a quick look to see how bad
> > > it is, but I'm guessing it's going to be a lot of work to make objtool
> > > compatible with clang (and unfortunately I won't have the bandwidth to
> > > work on that in the near term.)
> > > 
> > > In the meantime I'd recommend that you use frame pointers (and
> > > CONFIG_STACK_VALIDATION=n) for clang-compiled kernels.
> > 
> > Thanks for your assessment!
> > 
> > dvo_ch7017.o is attached.
> 
> Here's a (surprisingly easy) fix for this particular issue, though I'd
> be shocked if there weren't a bunch more issues lurking elsewhere.  Let
> me know how it goes.

Thanks for having a look, this fixes the vast majority of warnings in
a defconfig build!

The remaining warnings are:

arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
function pti_user_pagetable_walk_pmd()
s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls
through to next function fops_u8_open()

In all these functions it's an 'early' return that 'causes' the
warning.

Obviously I don't expect you to spend large amounts of time
investigating this, but should there be a similarily easy fix it would
be certainly welcome :)

> BTW, one thing I noticed in the .o file is that most of the functions'
> stacks are aligned to 16 bytes.  It might be worth checking if the clang
> -mstack-alignment=8 option is getting set, and if so, if it's working
> properly.  Otherwise, with aligned stacks, the frame pointer is forced,
> which defeats most of the benefits of ORC.

Thanks for pointing this out. I verified that -mstack-alignment=8 is
set. Do the "and $0xfffffffffffffff0,%rsp" instructions indicate the
stack alignment of 16 bytes? These are also present in the object file
generated by gcc.

[-- Attachment #2: pti.o --]
[-- Type: application/x-object, Size: 9176 bytes --]

[-- Attachment #3: file.o --]
[-- Type: application/x-object, Size: 65376 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 23:22           ` Matthias Kaehlcke
@ 2018-03-20  2:28             ` Josh Poimboeuf
  2018-03-20 19:39               ` Matthias Kaehlcke
  2018-03-21  2:45             ` Josh Poimboeuf
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2018-03-20  2:28 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Mon, Mar 19, 2018 at 04:22:55PM -0700, Matthias Kaehlcke wrote:
> > Here's a (surprisingly easy) fix for this particular issue, though I'd
> > be shocked if there weren't a bunch more issues lurking elsewhere.  Let
> > me know how it goes.
> 
> Thanks for having a look, this fixes the vast majority of warnings in
> a defconfig build!

Wow.  Consider me shocked :-)

> The remaining warnings are:
> 
> arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
> function pti_user_pagetable_walk_pmd()
> s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
> fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
> fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
> fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
> fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls through to next function fops_u8_open()

These all seem to be related to some weirdness with the UD2 instruction,
not necessarily an objtool issue per se.

Any chance this fixes some of the warnings?  If not, I can try to build
with clang and look a little deeper.

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ab4711c63601..315a8757b565 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -124,7 +124,12 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 # define ASM_UNREACHABLE
 #endif
 #ifndef unreachable
-# define unreachable() do { annotate_reachable(); do { } while (1); } while (0)
+#define unreachable() \
+	do {					\
+		annotate_unreachable();		\
+		barrier_before_unreachable();	\
+		__builtin_unreachable();	\
+	} while (0)
 #endif
 
 /*

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-20  2:28             ` Josh Poimboeuf
@ 2018-03-20 19:39               ` Matthias Kaehlcke
  0 siblings, 0 replies; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-03-20 19:39 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

El Mon, Mar 19, 2018 at 09:28:03PM -0500 Josh Poimboeuf ha dit:

> On Mon, Mar 19, 2018 at 04:22:55PM -0700, Matthias Kaehlcke wrote:
> > > Here's a (surprisingly easy) fix for this particular issue, though I'd
> > > be shocked if there weren't a bunch more issues lurking elsewhere.  Let
> > > me know how it goes.
> > 
> > Thanks for having a look, this fixes the vast majority of warnings in
> > a defconfig build!
> 
> Wow.  Consider me shocked :-)
> 
> > The remaining warnings are:
> > 
> > arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
> > function pti_user_pagetable_walk_pmd()
> > s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
> > fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
> > fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
> > fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
> > fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls through to next function fops_u8_open()
> 
> These all seem to be related to some weirdness with the UD2 instruction,
> not necessarily an objtool issue per se.
> 
> Any chance this fixes some of the warnings?  If not, I can try to build
> with clang and look a little deeper.
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index ab4711c63601..315a8757b565 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -124,7 +124,12 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>  # define ASM_UNREACHABLE
>  #endif
>  #ifndef unreachable
> -# define unreachable() do { annotate_reachable(); do { } while (1); } while (0)
> +#define unreachable() \
> +	do {					\
> +		annotate_unreachable();		\
> +		barrier_before_unreachable();	\
> +		__builtin_unreachable();	\
> +	} while (0)
>  #endif

Thanks, unfortunately negative on this one, all the warnings are still
generated.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-19 23:22           ` Matthias Kaehlcke
  2018-03-20  2:28             ` Josh Poimboeuf
@ 2018-03-21  2:45             ` Josh Poimboeuf
  2018-03-21 21:19               ` Matthias Kaehlcke
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Poimboeuf @ 2018-03-21  2:45 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

On Mon, Mar 19, 2018 at 04:22:55PM -0700, Matthias Kaehlcke wrote:
> arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
> function pti_user_pagetable_walk_pmd()
> s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
> fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
> fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
> fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
> fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls
> through to next function fops_u8_open()

Ok, I did a little more digging.  Surprise, these objtool warnings are
actually correct.  Somehow the WARN macros are causing Clang to produce
bad code.  In some cases, Clang is assuming that a WARN is "noreturn",
i.e. that the UD2 doesn't return from the exception.

As an example, here's the full_proxy_llseek() function and its compiled
code:

#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args)		\
static ret_type full_proxy_ ## name(proto)				\
{									\
	struct dentry *dentry = F_DENTRY(filp);			\
	const struct file_operations *real_fops;			\
	ret_type r;							\
									\
	r = debugfs_file_get(dentry);					\
	if (unlikely(r))						\
		return r;						\
	real_fops = debugfs_real_fops(filp);				\
	r = real_fops->name(args);					\
	debugfs_file_put(dentry);					\
	return r;							\
}

FULL_PROXY_FUNC(llseek, loff_t, filp,
		PROTO(struct file *filp, loff_t offset, int whence),
		ARGS(filp, offset, whence));


0000000000000ca0 <full_proxy_llseek>:
     ca0:       55                      push   %rbp
     ca1:       41 57                   push   %r15
     ca3:       41 56                   push   %r14
     ca5:       53                      push   %rbx
     ca6:       48 83 ec 10             sub    $0x10,%rsp
     caa:       41 89 d6                mov    %edx,%r14d
     cad:       49 89 f7                mov    %rsi,%r15
     cb0:       48 89 fd                mov    %rdi,%rbp
     cb3:       65 48 8b 04 25 28 00    mov    %gs:0x28,%rax
     cba:       00 00
     cbc:       48 89 44 24 08          mov    %rax,0x8(%rsp)
     cc1:       48 8b 5d 18             mov    0x18(%rbp),%rbx
     cc5:       48 89 df                mov    %rbx,%rdi
     cc8:       e8 00 00 00 00          callq  ccd <full_proxy_llseek+0x2d>
                        cc9: R_X86_64_PC32      debugfs_file_get-0x4
     ccd:       85 c0                   test   %eax,%eax
     ccf:       75 65                   jne    d36 <full_proxy_llseek+0x96>
     cd1:       48 8b 45 18             mov    0x18(%rbp),%rax
     cd5:       48 8b 40 78             mov    0x78(%rax),%rax
     cd9:       a8 01                   test   $0x1,%al
     cdb:       75 63                   jne    d40 <full_proxy_llseek+0xa0>
     ...
     d40:       0f 0b                   ud2
     d42:       0f 1f 40 00             nopl   0x0(%rax)
     d46:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
     d4d:       00 00 00

0000000000000d50 <full_proxy_read>:


After the debugfs_file_get() call, the debugfs_real_fops() call is
inlined.  Here it is:

const struct file_operations *debugfs_real_fops(const struct file *filp)
{
	struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;

	if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) {
		/*
		 * Urgh, we've been called w/o a protecting
		 * debugfs_file_get().
		 */
		WARN_ON(1);
		return NULL;
	}

	return fsd->real_fops;
}

The WARN_ON() is the UD2 at offset d40.  Notice that, as objtool found,
it just falls through to the next function instead of "returning" to
full_proxy_llseek().


At first I thought this might be some kind of "optimization", since, if
you look closely, you'll see that the warning can never happen in this
situation.  But no, I downloaded the Clang binary, changed the 

  if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) /* DEBUGFS_FSDATA_IS_REAL_FOPS_BIT == 1 */

to

  if ((unsigned long)fsd & 0x2)

and still got the same issue.


Then I figured that Clang must be peeking into the inline asm, and is
assuming that UD2 is a dead end.  But no, I changed ASM_UD2 to a 2-byte
NOP, and got the same issue.


So unless I'm missing some "undefined behavior", this looks like a Clang
bug to me.  Somehow it doesn't like the WARN macros (but apparently only
in a small number of cases).

-- 
Josh

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit
  2018-03-21  2:45             ` Josh Poimboeuf
@ 2018-03-21 21:19               ` Matthias Kaehlcke
  0 siblings, 0 replies; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-03-21 21:19 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: Ingo Molnar, linux-kernel, torvalds, peterz, hpa, tglx

El Tue, Mar 20, 2018 at 09:45:07PM -0500 Josh Poimboeuf ha dit:

> On Mon, Mar 19, 2018 at 04:22:55PM -0700, Matthias Kaehlcke wrote:
> > arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
> > function pti_user_pagetable_walk_pmd()
> > s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
> > fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
> > fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
> > fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
> > fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls
> > through to next function fops_u8_open()
> 
> Ok, I did a little more digging.  Surprise, these objtool warnings are
> actually correct.  Somehow the WARN macros are causing Clang to produce
> bad code.  In some cases, Clang is assuming that a WARN is "noreturn",
> i.e. that the UD2 doesn't return from the exception.
> 
> As an example, here's the full_proxy_llseek() function and its compiled
> code:
> 
> #define FULL_PROXY_FUNC(name, ret_type, filp, proto, args)		\
> static ret_type full_proxy_ ## name(proto)				\
> {									\
> 	struct dentry *dentry = F_DENTRY(filp);			\
> 	const struct file_operations *real_fops;			\
> 	ret_type r;							\
> 									\
> 	r = debugfs_file_get(dentry);					\
> 	if (unlikely(r))						\
> 		return r;						\
> 	real_fops = debugfs_real_fops(filp);				\
> 	r = real_fops->name(args);					\
> 	debugfs_file_put(dentry);					\
> 	return r;							\
> }
> 
> FULL_PROXY_FUNC(llseek, loff_t, filp,
> 		PROTO(struct file *filp, loff_t offset, int whence),
> 		ARGS(filp, offset, whence));
> 
> 
> 0000000000000ca0 <full_proxy_llseek>:
>      ca0:       55                      push   %rbp
>      ca1:       41 57                   push   %r15
>      ca3:       41 56                   push   %r14
>      ca5:       53                      push   %rbx
>      ca6:       48 83 ec 10             sub    $0x10,%rsp
>      caa:       41 89 d6                mov    %edx,%r14d
>      cad:       49 89 f7                mov    %rsi,%r15
>      cb0:       48 89 fd                mov    %rdi,%rbp
>      cb3:       65 48 8b 04 25 28 00    mov    %gs:0x28,%rax
>      cba:       00 00
>      cbc:       48 89 44 24 08          mov    %rax,0x8(%rsp)
>      cc1:       48 8b 5d 18             mov    0x18(%rbp),%rbx
>      cc5:       48 89 df                mov    %rbx,%rdi
>      cc8:       e8 00 00 00 00          callq  ccd <full_proxy_llseek+0x2d>
>                         cc9: R_X86_64_PC32      debugfs_file_get-0x4
>      ccd:       85 c0                   test   %eax,%eax
>      ccf:       75 65                   jne    d36 <full_proxy_llseek+0x96>
>      cd1:       48 8b 45 18             mov    0x18(%rbp),%rax
>      cd5:       48 8b 40 78             mov    0x78(%rax),%rax
>      cd9:       a8 01                   test   $0x1,%al
>      cdb:       75 63                   jne    d40 <full_proxy_llseek+0xa0>
>      ...
>      d40:       0f 0b                   ud2
>      d42:       0f 1f 40 00             nopl   0x0(%rax)
>      d46:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
>      d4d:       00 00 00
> 
> 0000000000000d50 <full_proxy_read>:
> 
> 
> After the debugfs_file_get() call, the debugfs_real_fops() call is
> inlined.  Here it is:
> 
> const struct file_operations *debugfs_real_fops(const struct file *filp)
> {
> 	struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
> 
> 	if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) {
> 		/*
> 		 * Urgh, we've been called w/o a protecting
> 		 * debugfs_file_get().
> 		 */
> 		WARN_ON(1);
> 		return NULL;
> 	}
> 
> 	return fsd->real_fops;
> }
> 
> The WARN_ON() is the UD2 at offset d40.  Notice that, as objtool found,
> it just falls through to the next function instead of "returning" to
> full_proxy_llseek().
> 
> 
> At first I thought this might be some kind of "optimization", since, if
> you look closely, you'll see that the warning can never happen in this
> situation.  But no, I downloaded the Clang binary, changed the 
> 
>   if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) /* DEBUGFS_FSDATA_IS_REAL_FOPS_BIT == 1 */
> 
> to
> 
>   if ((unsigned long)fsd & 0x2)
> 
> and still got the same issue.
> 
> 
> Then I figured that Clang must be peeking into the inline asm, and is
> assuming that UD2 is a dead end.  But no, I changed ASM_UD2 to a 2-byte
> NOP, and got the same issue.
> 
> 
> So unless I'm missing some "undefined behavior", this looks like a Clang
> bug to me.  Somehow it doesn't like the WARN macros (but apparently only
> in a small number of cases).

The behavior is indeed mysterious. Thanks for digging!

I'll see if our compiler folks can get to the bottom of this.

Could you send your fix for objtool that restores the original stack
pointer as an official patch?

Thanks

Matthias

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-03-21 21:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20171013052544.euk7yawni47lhmdq@gmail.com>
2017-10-13 20:02 ` [PATCH 1/2] x86/unwind: Rename unwinder config options to 'CONFIG_UNWINDER_*' Josh Poimboeuf
2017-10-14 10:49   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
2017-10-13 20:02 ` [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit Josh Poimboeuf
2017-10-14 10:50   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
2017-10-19 16:51   ` [2/2] " Andrei Vagin
2017-10-19 18:16     ` Josh Poimboeuf
2017-10-19 22:35       ` Andrei Vagin
2017-10-20  0:38         ` Andrei Vagin
2017-10-20  1:28         ` Josh Poimboeuf
2017-10-20  6:54           ` Andrei Vagin
2018-03-19 18:57   ` [PATCH 2/2] " Matthias Kaehlcke
2018-03-19 19:29     ` Josh Poimboeuf
2018-03-19 20:31       ` Matthias Kaehlcke
2018-03-19 21:20         ` Josh Poimboeuf
2018-03-19 23:22           ` Matthias Kaehlcke
2018-03-20  2:28             ` Josh Poimboeuf
2018-03-20 19:39               ` Matthias Kaehlcke
2018-03-21  2:45             ` Josh Poimboeuf
2018-03-21 21:19               ` Matthias Kaehlcke

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.