Skip to contentSkip to conversation

x86 BUG handling

PATCH In review Andrew Morton 2 participants 2 messages Mar 20, 2002 – Mar 21, 2002

lore
Share & tools
lore

Andrew Morton

author
#0 lore raw

PATCH 1/1 x86 BUG handling

This has been in 2.5 for a few weeks, no problems reported.

We embed the file-and-line info inline after the invalid opcode in the program text. This means that when we hit the invalid opcode handler, the stacked machine register info is correct. Fixes the problem where the printk() wrecks the non-call-preserved registers.

CONFIG_DEBUG_BUGVERBOSE goes away. Fixes the problem where kernel developers don't know where their BUGs are.

On my fairly lean kernel build, kernel size is reduced by 90 kbytes relative to a CONFIG_DEBUG_BUGVERBOSE=y build, due to all the do_BUG() calls which aren't there any more.

include/asm-i386/page.h renamed from include/asm-i386/page.h~BUG +9 -7
@@ -91,16 +91,18 @@ typedef struct { unsigned long pgprot; }
 /*
  * Tell the user there is some problem. Beep too, so we can
  * see^H^H^Hhear bugs in early bootup as well!
+ * The offending file and line are encoded after the "officially
+ * undefined" opcode for parsing in the trap handler.
  */
 
-#ifdef CONFIG_DEBUG_BUGVERBOSE
-extern void do_BUG(const char *file, int line);
-#define BUG() do {					\
-	do_BUG(__FILE__, __LINE__);			\
-	__asm__ __volatile__("ud2");			\
-} while (0)
+#if 1	/* Set to zero for a slightly smaller kernel */
+#define BUG()				\
+ __asm__ __volatile__(	"ud2\n"		\
+			"\t.word %c0\n"	\
+			"\t.long %c1\n"	\
+			 : : "i" (__LINE__), "i" (__FILE__))
 #else
-#define BUG() __asm__ __volatile__(".byte 0x0f,0x0b")
+#define BUG() __asm__ __volatile__("ud2\n")
 #endif
 
 #define PAGE_BUG(page) do { \
arch/i386/kernel/traps.c renamed from arch/i386/kernel/traps.c~BUG +36 -0
@@ -237,6 +237,41 @@ bad:
 	printk("\n");
 }	
 
+static void handle_BUG(struct pt_regs *regs)
+{
+	unsigned short ud2;
+	unsigned short line;
+	char *file;
+	char c;
+	unsigned long eip;
+
+	if (regs->xcs & 3)
+		goto no_bug;		/* Not in kernel */
+
+	eip = regs->eip;
+
+	if (eip < PAGE_OFFSET)
+		goto no_bug;
+	if (__get_user(ud2, (unsigned short *)eip))
+		goto no_bug;
+	if (ud2 != 0x0b0f)
+		goto no_bug;
+	if (__get_user(line, (unsigned short *)(eip + 2)))
+		goto bug;
+	if (__get_user(file, (char **)(eip + 4)) ||
+		(unsigned long)file < PAGE_OFFSET || __get_user(c, file))
+		file = "<bad filename>";
+
+	printk("kernel BUG at %s:%d!\n", file, line);
+
+no_bug:
+	return;
+
+	/* Here we know it was a BUG but file-n-line is unavailable */
+bug:
+	printk("Kernel BUG\n");
+}
+
 spinlock_t die_lock = SPIN_LOCK_UNLOCKED;
 
 void die(const char * str, struct pt_regs * regs, long err)

@@ -244,6 +279,7 @@ void die(const char * str, struct pt_reg
 	console_verbose();
 	spin_lock_irq(&die_lock);
 	bust_spinlocks(1);
+	handle_BUG(regs);
 	printk("%s: %04lx\n", str, err & 0xffff);
 	show_registers(regs);
 	bust_spinlocks(0);
arch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c~BUG +0 -4
@@ -169,9 +169,5 @@ EXPORT_SYMBOL_NOVERS(memset);
 EXPORT_SYMBOL(atomic_dec_and_lock);
 #endif
 
-#ifdef CONFIG_DEBUG_BUGVERBOSE
-EXPORT_SYMBOL(do_BUG);
-#endif
-
 extern int is_sony_vaio_laptop;
 EXPORT_SYMBOL(is_sony_vaio_laptop);
arch/i386/config.in renamed from arch/i386/config.in~BUG +0 -1
@@ -424,7 +424,6 @@ if [ "$CONFIG_DEBUG_KERNEL" != "n" ]; th
    bool '  Memory mapped I/O debugging' CONFIG_DEBUG_IOVIRT
    bool '  Magic SysRq key' CONFIG_MAGIC_SYSRQ
    bool '  Spinlock debugging' CONFIG_DEBUG_SPINLOCK
-   bool '  Verbose BUG() reporting (adds 70K)' CONFIG_DEBUG_BUGVERBOSE
 fi
 
 endmenu
arch/i386/mm/fault.c renamed from arch/i386/mm/fault.c~BUG +0 -6
@@ -125,12 +125,6 @@ void bust_spinlocks(int yes)
 	}
 }
 
-void do_BUG(const char *file, int line)
-{
-	bust_spinlocks(1);
-	printk("kernel BUG at %s:%d!\n", file, line);
-}
-
 asmlinkage void do_invalid_op(struct pt_regs *, unsigned long);
 extern unsigned long idt;

-

#1 lore raw
Quoted 1 line · Andrew Morton

This has been in 2.5 for a few weeks, no problems reported.

Please, let's at least warn the x86 disassembler opcode folks...

Quoted 6 lines

We embed the file-and-line info inline after the invalid opcode in the program text. This means that when we hit the invalid opcode handler, the stacked machine register info is correct. Fixes the problem where the printk() wrecks the non-call-preserved registers.

Quoted 6 lines

CONFIG_DEBUG_BUGVERBOSE goes away. Fixes the problem where kernel developers don't know where their BUGs are.

On my fairly lean kernel build, kernel size is reduced by 90 kbytes relative to a CONFIG_DEBUG_BUGVERBOSE=y build, due to all the do_BUG() calls which aren't there any more.

This patch already went into Linux 2.5.5 and 2.4.19-pre3-ac4, sounds like it may be heading for 2.4.19-pre5. It's good for kernel debug, and good for kernel space-saving, but... it confuses disassemblers which think that "ud2" is a 2-byte instruction: they need to pretend it's an 8-byte instruction when dealing with the new kernel objects. I suspect it could be transparent if we used one more byte per BUG, but that's not how it is, so may affect gdb, kdb, ksymoops, lcrash, objdump...

Hugh

Quoted 18 lines
include/asm-i386/page.h renamed from include/asm-i386/page.h~BUG +4 -6
@@ -91,16 +91,18 @@ typedef struct { unsigned long pgprot; }
 /*
  * Tell the user there is some problem. Beep too, so we can
  * see^H^H^Hhear bugs in early bootup as well!
+ * The offending file and line are encoded after the "officially
+ * undefined" opcode for parsing in the trap handler.
  */
 
-#ifdef CONFIG_DEBUG_BUGVERBOSE
-extern void do_BUG(const char *file, int line);
-#define BUG() do {					\
-	do_BUG(__FILE__, __LINE__);			\
-	__asm__ __volatile__("ud2");			\
-} while (0)
+#if 1	/* Set to zero for a slightly smaller kernel */
+#define BUG()				\

include/asm-i386/page.h typedef struct { unsigned long pgprot; } · line 108

Quoted 10 lines
include/asm-i386/page.h renamed from include/asm-i386/page.h~BUG +5 -1
+ __asm__ __volatile__(	"ud2\n"		\
+			"\t.word %c0\n"	\
+			"\t.long %c1\n"	\
+			 : : "i" (__LINE__), "i" (__FILE__))
 #else
-#define BUG() __asm__ __volatile__(".byte 0x0f,0x0b")
+#define BUG() __asm__ __volatile__("ud2\n")
 #endif
 
 #define PAGE_BUG(page) do { \

[ remainder of patch irrelevant to disassembly issue ]