Re: 2.1.111 simple enhancement/fix for safe debugging

Eric Paire (e.paire@opengroup.org)
Thu, 30 Jul 1998 10:52:43 +0200


>
>
> On Wed, 29 Jul 1998, Eric Paire wrote:
> >
> > Yet another bug using GDB on i386 Linux systems (which should be the same
> > for other processors managing the PTRACE_SINGLESTEP ptrace request):
>
> The patch looks a bit strange - I'm sure you're fixing a bug, but it also
> looks like a fairly unmaintainable way of doing so.
>
> I'd much prefer an approach where you leave the TF bit completely alone,
> and then when you get a debug trap you look at whether the process is
> being debugged or not, and if not you just ignore the trap and clear TF on
> return. What way it's much more obvious at least to me what's up (call it
> "lazy TF clearing" ;)
>
> Ok?
>
As you wisely suggested ;-), I have modified the previous fix for a
"lazy TF clearing" style one. I have also modified the debugreg.h
file and moved it from "include/linux" to "include/asm-i386" because it
contained pure-i386 definitions. I also modified "ptrace.c" to use this new
location from "arch/i386/kernel" and from "arch/alpha/kernel" (hopefully,
no defines were used in this file).

The following patch contains all these modifications,

Best regards,
Eric
P.S. I also fixed in traps.c two "force_sig()" inversions with "tss" update
+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ Eric PAIRE
Email : e.paire@gr.opengroup.org | The Open Group - Grenoble Research Institute
Phone : +33 (0) 476 63 48 71 | 2, avenue de Vignate
Fax : +33 (0) 476 51 05 32 | F-38610 Gieres FRANCE

------ Cut here ------ Cut here ------ Cut here ------ Cut here ------
--- arch/i386/kernel/traps.c.OLD Fri Jul 17 07:48:53 1998
+++ arch/i386/kernel/traps.c Thu Jul 30 10:44:55 1998
@@ -27,6 +27,7 @@
#include <asm/io.h>
#include <asm/spinlock.h>
#include <asm/atomic.h>
+#include <asm/debugreg.h>

asmlinkage int system_call(void);
asmlinkage void lcall7(void);
@@ -296,14 +297,22 @@

asmlinkage void do_debug(struct pt_regs * regs, long error_code)
{
+ int condition;
+
lock_kernel();
if (regs->eflags & VM_MASK) {
handle_vm86_trap((struct kernel_vm86_regs *) regs, error_code, 1);
goto out;
}
- force_sig(SIGTRAP, current);
- current->tss.trap_no = 1;
- current->tss.error_code = error_code;
+
+ /* Check if this a spurious single-step trap */
+ __asm__ __volatile__("movl %%db6,%0" : "=r" (condition));
+ if ((condition & DR_STEP) == 0 || (current->flags & PF_PTRACED) != 0) {
+ current->tss.trap_no = 1;
+ current->tss.error_code = error_code;
+ force_sig(SIGTRAP, current);
+ } else
+ regs->eflags &= ~TF_MASK;
if ((regs->xcs & 3) == 0) {
/* If this is a kernel mode trap, then reset db7 and allow us to continue */
__asm__("movl %0,%%db7"
@@ -333,9 +342,9 @@
task->flags&=~PF_USEDFPU;
stts();

- force_sig(SIGFPE, task);
task->tss.trap_no = 16;
task->tss.error_code = 0;
+ force_sig(SIGFPE, task);
unlock_kernel();
}

--- arch/i386/kernel/ptrace.c.OLD Tue Jul 28 15:50:04 1998
+++ arch/i386/kernel/ptrace.c Thu Jul 30 09:42:29 1998
@@ -12,12 +12,12 @@
#include <linux/errno.h>
#include <linux/ptrace.h>
#include <linux/user.h>
-#include <linux/debugreg.h>

#include <asm/uaccess.h>
#include <asm/pgtable.h>
#include <asm/system.h>
#include <asm/processor.h>
+#include <asm/debugreg.h>

/*
* does not yet catch signals sent when the child dies.
--- arch/alpha/kernel/ptrace.c.OLD Mon Jul 27 09:38:08 1998
+++ arch/alpha/kernel/ptrace.c Thu Jul 30 09:41:39 1998
@@ -13,7 +13,6 @@
#include <linux/errno.h>
#include <linux/ptrace.h>
#include <linux/user.h>
-#include <linux/debugreg.h>

#include <asm/uaccess.h>
#include <asm/pgtable.h>
--- include/linux/debugreg.h.OLD Tue Nov 21 07:34:54 1995
+++ include/linux/debugreg.h Thu Jul 30 09:37:29 1998
@@ -1,61 +0,0 @@
-#ifndef _LINUX_DEBUGREG_H
-#define _LINUX_DEBUGREG_H
-
-
-/* Indicate the register numbers for a number of the specific
- debug registers. Registers 0-3 contain the addresses we wish to trap on */
-#define DR_FIRSTADDR 0 /* u_debugreg[DR_FIRSTADDR] */
-#define DR_LASTADDR 3 /* u_debugreg[DR_LASTADDR] */
-
-#define DR_STATUS 6 /* u_debugreg[DR_STATUS] */
-#define DR_CONTROL 7 /* u_debugreg[DR_CONTROL] */
-
-/* Define a few things for the status register. We can use this to determine
- which debugging register was responsible for the trap. The other bits
- are either reserved or not of interest to us. */
-
-#define DR_TRAP0 (0x1) /* Trap due to db0 */
-#define DR_TRAP1 (0x2) /* Trap due to db1 */
-#define DR_TRAP2 (0x4) /* Trap due to db2 */
-#define DR_TRAP3 (0x8) /* Trap due to db3 */
-
-/* Now define a bunch of things for manipulating the control register.
- The top two bytes of the control register consist of 4 fields of 4
- bits - each field corresponds to one of the four debug registers,
- and indicates what types of access we trap on, and how large the data
- field is that we are looking at */
-
-#define DR_CONTROL_SHIFT 16 /* Skip this many bits in ctl register */
-#define DR_CONTROL_SIZE 4 /* 4 control bits per register */
-
-#define DR_RW_EXECUTE (0x0) /* Settings for the access types to trap on */
-#define DR_RW_WRITE (0x1)
-#define DR_RW_READ (0x3)
-
-#define DR_LEN_1 (0x0) /* Settings for data length to trap on */
-#define DR_LEN_2 (0x4)
-#define DR_LEN_4 (0xC)
-
-/* The low byte to the control register determine which registers are
- enabled. There are 4 fields of two bits. One bit is "local", meaning
- that the processor will reset the bit after a task switch and the other
- is global meaning that we have to explicitly reset the bit. With linux,
- you can use either one, since we explicitly zero the register when we enter
- kernel mode. */
-
-#define DR_LOCAL_ENABLE_SHIFT 0 /* Extra shift to the local enable bit */
-#define DR_GLOBAL_ENABLE_SHIFT 1 /* Extra shift to the global enable bit */
-#define DR_ENABLE_SIZE 2 /* 2 enable bits per register */
-
-#define DR_LOCAL_ENABLE_MASK (0x55) /* Set local bits for all 4 regs */
-#define DR_GLOBAL_ENABLE_MASK (0xAA) /* Set global bits for all 4 regs */
-
-/* The second byte to the control register has a few special things.
- We can slow the instruction pipeline for instructions coming via the
- gdt or the ldt if we want to. I am not sure why this is an advantage */
-
-#define DR_CONTROL_RESERVED (0xFC00) /* Reserved by Intel */
-#define DR_LOCAL_SLOWDOWN (0x100) /* Local slow the pipeline */
-#define DR_GLOBAL_SLOWDOWN (0x200) /* Global slow the pipeline */
-
-#endif
--- include/linux/debugreg.h.OLD Tue Nov 21 07:34:54 1995
+++ include/linux/debugreg.h Thu Jul 30 09:37:29 1998
@@ -1,61 +0,0 @@
-#ifndef _LINUX_DEBUGREG_H
-#define _LINUX_DEBUGREG_H
-
-
-/* Indicate the register numbers for a number of the specific
- debug registers. Registers 0-3 contain the addresses we wish to trap on */
-#define DR_FIRSTADDR 0 /* u_debugreg[DR_FIRSTADDR] */
-#define DR_LASTADDR 3 /* u_debugreg[DR_LASTADDR] */
-
-#define DR_STATUS 6 /* u_debugreg[DR_STATUS] */
-#define DR_CONTROL 7 /* u_debugreg[DR_CONTROL] */
-
-/* Define a few things for the status register. We can use this to determine
- which debugging register was responsible for the trap. The other bits
- are either reserved or not of interest to us. */
-
-#define DR_TRAP0 (0x1) /* Trap due to db0 */
-#define DR_TRAP1 (0x2) /* Trap due to db1 */
-#define DR_TRAP2 (0x4) /* Trap due to db2 */
-#define DR_TRAP3 (0x8) /* Trap due to db3 */
-
-/* Now define a bunch of things for manipulating the control register.
- The top two bytes of the control register consist of 4 fields of 4
- bits - each field corresponds to one of the four debug registers,
- and indicates what types of access we trap on, and how large the data
- field is that we are looking at */
-
-#define DR_CONTROL_SHIFT 16 /* Skip this many bits in ctl register */
-#define DR_CONTROL_SIZE 4 /* 4 control bits per register */
-
-#define DR_RW_EXECUTE (0x0) /* Settings for the access types to trap on */
-#define DR_RW_WRITE (0x1)
-#define DR_RW_READ (0x3)
-
-#define DR_LEN_1 (0x0) /* Settings for data length to trap on */
-#define DR_LEN_2 (0x4)
-#define DR_LEN_4 (0xC)
-
-/* The low byte to the control register determine which registers are
- enabled. There are 4 fields of two bits. One bit is "local", meaning
- that the processor will reset the bit after a task switch and the other
- is global meaning that we have to explicitly reset the bit. With linux,
- you can use either one, since we explicitly zero the register when we enter
- kernel mode. */
-
-#define DR_LOCAL_ENABLE_SHIFT 0 /* Extra shift to the local enable bit */
-#define DR_GLOBAL_ENABLE_SHIFT 1 /* Extra shift to the global enable bit */
-#define DR_ENABLE_SIZE 2 /* 2 enable bits per register */
-
-#define DR_LOCAL_ENABLE_MASK (0x55) /* Set local bits for all 4 regs */
-#define DR_GLOBAL_ENABLE_MASK (0xAA) /* Set global bits for all 4 regs */
-
-/* The second byte to the control register has a few special things.
- We can slow the instruction pipeline for instructions coming via the
- gdt or the ldt if we want to. I am not sure why this is an advantage */
-
-#define DR_CONTROL_RESERVED (0xFC00) /* Reserved by Intel */
-#define DR_LOCAL_SLOWDOWN (0x100) /* Local slow the pipeline */
-#define DR_GLOBAL_SLOWDOWN (0x200) /* Global slow the pipeline */
-
-#endif

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html