Browse Source

Fix PC single step issues.

Single stepping a process from user mode that had not yet set up its
signal handler was cauing kernel mode to take the single step exception.
This was not appropriate, as the single step was handled by the debugger
process.

Also fixed x64 double faults. The hardware pushes its own error code for
this exception, so there's no need to push a dummy one.

I was originally trying to debug an x64 issue with single stepping over a
syscall instruction in user mode. The FMASK MSR should have cleared TF,
preventing a single step exception in kernel mode. On Qemu this was not
happening, and ugliness was occurring because we were executing on the
user mode stack. It turns out this was a Qemu bug, so I'm not going to
worry about it.

I think there is still a problem with the system call refactoring on ARM,
but I cannot reproduce any problems with Qemu. I'll take a look at a real
machine when I get home.
Evan Green 7 years ago
parent
commit
a56582bba3
4 changed files with 12 additions and 15 deletions
  1. 4 5
      kernel/armv7/trap.S
  2. 4 7
      kernel/ke/x64/dispatch.c
  3. 4 2
      kernel/ke/x86/dispatch.c
  4. 0 1
      kernel/x64/trap.S

+ 4 - 5
kernel/armv7/trap.S

@@ -242,10 +242,9 @@ FUNCTION ArpSoftwareInterruptEntry
     str     %r3, [%r2, #TRAP_EXCEPTION_CPSR]    @ Save into exception CPSR.
 
     //
-    // The system call routine takes three parameters: the system call number,
-    // system call parameter, and a pointer to the trap frame. The number was
-    // moved from R2 into R0 early in the function, the parameter is in R1 from
-    // user mode, and R2 == SP == trap frame.
+    // The system call routine takes four parameters: the system call number,
+    // system call parameter, a pointer to the trap frame, and a pointer to an
+    // outgoing boolean.
     //
 
     mov     %r3, %sp                            @ Set boolean pointer parameter.
@@ -304,7 +303,7 @@ ArpSoftwareInterruptRestore:
     CFI_UNDEFINED(pc)
 
     //
-    // Clear the remainig volatile register to avoid leaking kernel state.
+    // Clear the remaining volatile register to avoid leaking kernel state.
     //
 
     eor     %r1, %r1

+ 4 - 7
kernel/ke/x64/dispatch.c

@@ -93,21 +93,18 @@ Return Value:
         ArDisableInterrupts();
 
         //
-        // If there is no handler yet, go into the kernel debugger.
+        // If there is no handler or debugger yet, go into the kernel debugger.
         //
 
-        if (Thread->OwningProcess->SignalHandlerRoutine == NULL) {
+        if ((Thread->OwningProcess->SignalHandlerRoutine == NULL) &&
+            (Thread->OwningProcess->DebugData == NULL)) {
+
             KdDebugExceptionHandler(EXCEPTION_SINGLE_STEP, NULL, TrapFrame);
         }
 
         KeBeginCycleAccounting(PreviousPeriod);
 
     } else {
-
-        //
-        // TODO: On x64, does syscall clear the TF flag?
-        //
-
         KdDebugExceptionHandler(EXCEPTION_SINGLE_STEP, NULL, TrapFrame);
     }
 

+ 4 - 2
kernel/ke/x86/dispatch.c

@@ -92,10 +92,12 @@ Return Value:
         ArDisableInterrupts();
 
         //
-        // If there is no handler yet, go into the kernel debugger.
+        // If there is no handler or debugger yet, go into the kernel debugger.
         //
 
-        if (Thread->OwningProcess->SignalHandlerRoutine == NULL) {
+        if ((Thread->OwningProcess->SignalHandlerRoutine == NULL) &&
+            (Thread->OwningProcess->DebugData == NULL)) {
+
             KdDebugExceptionHandler(EXCEPTION_SINGLE_STEP, NULL, TrapFrame);
         }
 

+ 0 - 1
kernel/x64/trap.S

@@ -306,7 +306,6 @@ Return Value:
 --*/
 
 FUNCTION(ArDoubleFaultHandlerAsm)
-    pushq   $0                      # Push a dummy error code.
     call    ArGenerateTrapFrame     # Create a local trap frame.
     CFI_TRAP_FRAME_PUSHED           # Set unwind info for the debugger.
     movq    %rsp, %rdi              # 1st parameter is trap frame pointer.