From e015c2d86e9f3cd424f51657adc434b7671fc757 Mon Sep 17 00:00:00 2001
From: Chris Fernald <chfernal@microsoft.com>
Date: Mon, 6 May 2024 09:16:37 -0700
Subject: [PATCH] Remove windbg breakpoint workaround & improvements

---
 .../Library/DebugAgent/AARCH64/DebugAarch64.c | 15 ++-------------
 .../Library/DebugAgent/DebugAgent.h           |  3 ++-
 .../Library/DebugAgent/GdbStub/GdbStub.c      | 16 +++++++++++++---
 .../Library/DebugAgent/X64/DebugX64.c         | 19 +++++--------------
 4 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c b/DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c
index 4ce42d0..166ac73 100644
--- a/DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c
+++ b/DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c
@@ -115,7 +115,7 @@ DebuggerExceptionHandler (
     case 0x21: // Current EL instruction abort
     case 0x24: // Lower EL data abort
     case 0x25: // Current EL data abort
-      ExceptionInfo.ExceptionType    = ExceptionGenericFault;
+      ExceptionInfo.ExceptionType    = ExceptionAccessViolation;
       ExceptionInfo.ExceptionAddress = Context->ELR;
       break;
 
@@ -158,18 +158,7 @@ DebuggerExceptionHandler (
 
   ExceptionInfo.ArchExceptionCode = ExceptionType;
 
-  if (PcdGetBool (PcdEnableWindbgWorkarounds) &&
-      (ExceptionType == 0x3c) &&
-      (DebuggerBreakpointReason != BreakpointReasonNone) &&
-      (CompareMem ((UINT8 *)Context->ELR, &ArchBreakpointInstruction[0], ArchBreakpointInstructionSize) == 0))
-  {
-    //
-    // Windbg will act oddly when broken in on a actual debug breakpoint instruction,
-    // so preemptively step past this.
-    //
-    Context->ELR += ArchBreakpointInstructionSize;
-  }
-
+  // Call into the core debugger module.
   ReportEntryToDebugger (&ExceptionInfo, SystemContext);
 
   //
diff --git a/DebuggerFeaturePkg/Library/DebugAgent/DebugAgent.h b/DebuggerFeaturePkg/Library/DebugAgent/DebugAgent.h
index 79ae36c..5e07ac1 100644
--- a/DebuggerFeaturePkg/Library/DebugAgent/DebugAgent.h
+++ b/DebuggerFeaturePkg/Library/DebugAgent/DebugAgent.h
@@ -51,7 +51,8 @@ typedef enum _EXCEPTION_TYPE {
   ExceptionBreakpoint,
   ExceptionGenericFault,
   ExceptionInvalidOp,
-  ExceptionAlignment
+  ExceptionAlignment,
+  ExceptionAccessViolation
 } EXCEPTION_TYPE;
 
 typedef struct _EXCEPTION_INFO {
diff --git a/DebuggerFeaturePkg/Library/DebugAgent/GdbStub/GdbStub.c b/DebuggerFeaturePkg/Library/DebugAgent/GdbStub/GdbStub.c
index 5ef9afd..b0aa13a 100644
--- a/DebuggerFeaturePkg/Library/DebugAgent/GdbStub/GdbStub.c
+++ b/DebuggerFeaturePkg/Library/DebugAgent/GdbStub/GdbStub.c
@@ -40,7 +40,15 @@ STATIC CONST CHAR8  *EXCEPTION_TYPE_STRINGS[] = {
   "ExceptionBreakpoint",
   "ExceptionGenericFault",
   "ExceptionInvalidOp",
-  "ExceptionAlignment"
+  "ExceptionAlignment",
+  "ExceptionAccessViolation"
+};
+
+STATIC CONST CHAR8  *BREAK_REASON_STRINGS[] = {
+  "N/A",
+  "Initial Breakpoint",
+  "Module Load",
+  "Debugger Break"
 };
 
 //
@@ -483,12 +491,14 @@ ProcessMonitorCmd (
         "%a\n\r"
         "Exception Type: %a (%d)\n\r"
         "Exception Address: %llx\n\r"
-        "Architecture Exception Code: 0x%llx\n\r",
+        "Architecture Exception Code: 0x%llx\n\r"
+        "Break Reason: %a\n\r",
         gDebuggerInfo,
         EXCEPTION_TYPE_STRINGS[gExceptionInfo->ExceptionType],
         gExceptionInfo->ExceptionType,
         gExceptionInfo->ExceptionAddress,
-        gExceptionInfo->ArchExceptionCode
+        gExceptionInfo->ArchExceptionCode,
+        BREAK_REASON_STRINGS[DebuggerBreakpointReason]
         );
 
       break;
diff --git a/DebuggerFeaturePkg/Library/DebugAgent/X64/DebugX64.c b/DebuggerFeaturePkg/Library/DebugAgent/X64/DebugX64.c
index d04bbb0..0538d89 100644
--- a/DebuggerFeaturePkg/Library/DebugAgent/X64/DebugX64.c
+++ b/DebuggerFeaturePkg/Library/DebugAgent/X64/DebugX64.c
@@ -93,10 +93,14 @@ DebuggerExceptionHandler (
       ExceptionInfo.ExceptionAddress = Context->Rip;
       break;
 
+    case EXCEPT_X64_PAGE_FAULT:
+      ExceptionInfo.ExceptionType    = ExceptionAccessViolation;
+      ExceptionInfo.ExceptionAddress = Context->Rip;
+      break;
+
     case EXCEPT_X64_DOUBLE_FAULT:
     case EXCEPT_X64_SEG_NOT_PRESENT:
     case EXCEPT_X64_GP_FAULT:
-    case EXCEPT_X64_PAGE_FAULT:
       ExceptionInfo.ExceptionType    = ExceptionGenericFault;
       ExceptionInfo.ExceptionAddress = Context->Rip;
       break;
@@ -112,19 +116,6 @@ DebuggerExceptionHandler (
 
   ExceptionInfo.ArchExceptionCode = InterruptType;
 
-  if (PcdGetBool (PcdEnableWindbgWorkarounds) &&
-      (InterruptType == EXCEPT_X64_BREAKPOINT) &&
-      (DebuggerBreakpointReason != BreakpointReasonNone) &&
-      (*((UINT8 *)Context->Rip) == 0xCC))
-  {
-    //
-    // Windbg will act oddly when broken in on a actual INT 3 instruction, so
-    // preemptively step past this.
-    //
-
-    Context->Rip++;
-  }
-
   // Call into the core debugger module.
   ReportEntryToDebugger (&ExceptionInfo, SystemContext);