[PATCH v5 1/1] gdb: avoid conversion of SIGSEGV to SIGTRAP on user breakpoints

[Klaus Gerlicher <klaus.gerlicher@intel.com>]

From: "Gerlicher, Klaus" <klaus.gerlicher@intel.com>

GDB converts signals GDB_SIGNAL_ILL, GDB_SIGNAL_SEGV and GDB_SIGNAL_EMT to
GDB_SIGNAL_TRAP if a breakpoint is inserted at the fault location.  This
conversion logic assumes that when these signals occur at a breakpoint
location, the signal was actually caused by hitting the breakpoint rather
than a genuine fault.

However, some architectures have "imprecise page fault reporting," where a
memory access violation can be reported several instructions after the
faulting instruction.  For example:

  INSN1  <-- generates a SIGSEGV
  INSN2
  INSN3  <-- breakpoint installed here

If INSN1 causes a memory access violation, the backend may report the stop
at INSN3, where a breakpoint happens to be installed.  GDB's logic then
incorrectly assumes "there is a breakpoint at INSN3, so this SIGSEGV must
mean we hit the breakpoint" and converts it to SIGTRAP.  On architectures
with imprecise fault reporting, this is wrong: if a breakpoint is never
reported via SIGSEGV, then receiving a SIGSEGV at a breakpoint location
means we have a genuine SIGSEGV, not a breakpoint hit.

Add a new gdbarch function, imprecise_pagefault_reporting, that allows the
signal conversion from GDB_SIGNAL_SEGV to GDB_SIGNAL_TRAP to be skipped for
an architecture.  The default is false (conversion enabled), preserving
existing behavior.  Architectures with imprecise fault reporting should
override this to return true.
---
 gdb/gdbarch-gen.c         | 22 ++++++++++++++++++++++
 gdb/gdbarch-gen.h         | 24 ++++++++++++++++++++++++
 gdb/gdbarch_components.py | 29 +++++++++++++++++++++++++++++
 gdb/infrun.c              |  4 +++-
 4 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c
index 6008003466c..b3a699598f9 100644
--- a/gdb/gdbarch-gen.c
+++ b/gdb/gdbarch-gen.c
@@ -253,6 +253,7 @@ struct gdbarch
   gdbarch_core_parse_exec_context_ftype *core_parse_exec_context = default_core_parse_exec_context;
   gdbarch_shadow_stack_push_ftype *shadow_stack_push = nullptr;
   gdbarch_get_shadow_stack_pointer_ftype *get_shadow_stack_pointer = default_get_shadow_stack_pointer;
+  gdbarch_imprecise_pagefault_reporting_ftype *imprecise_pagefault_reporting = [] () -> bool {return false;};
 };
 
 /* Create a new ``struct gdbarch'' based on information provided by
@@ -513,6 +514,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of core_parse_exec_context, invalid_p == 0.  */
   /* Skip verify of shadow_stack_push, has predicate.  */
   /* Skip verify of get_shadow_stack_pointer, invalid_p == 0.  */
+  /* Skip verify of imprecise_pagefault_reporting, invalid_p == 0.  */
   if (!log.empty ())
     internal_error (_("verify_gdbarch: the following are invalid ...%s"),
 		    log.c_str ());
@@ -1339,6 +1341,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
 	      "gdbarch_dump: get_shadow_stack_pointer = <%s>\n",
 	      host_address_to_string (gdbarch->get_shadow_stack_pointer));
+  gdb_printf (file,
+	      "gdbarch_dump: imprecise_pagefault_reporting = <%s>\n",
+	      host_address_to_string (gdbarch->imprecise_pagefault_reporting));
   if (gdbarch->dump_tdep != nullptr)
     gdbarch->dump_tdep (gdbarch, file);
 }
@@ -5286,3 +5291,20 @@ set_gdbarch_get_shadow_stack_pointer (struct gdbarch *gdbarch,
 {
   gdbarch->get_shadow_stack_pointer = get_shadow_stack_pointer;
 }
+
+bool
+gdbarch_imprecise_pagefault_reporting (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != nullptr);
+  gdb_assert (gdbarch->imprecise_pagefault_reporting != nullptr);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_imprecise_pagefault_reporting called\n");
+  return gdbarch->imprecise_pagefault_reporting ();
+}
+
+void
+set_gdbarch_imprecise_pagefault_reporting (struct gdbarch *gdbarch,
+					   gdbarch_imprecise_pagefault_reporting_ftype imprecise_pagefault_reporting)
+{
+  gdbarch->imprecise_pagefault_reporting = imprecise_pagefault_reporting;
+}
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 6eda8693d58..3175e712c55 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -1758,3 +1758,27 @@ void set_gdbarch_shadow_stack_push (struct gdbarch *gdbarch, gdbarch_shadow_stac
 using gdbarch_get_shadow_stack_pointer_ftype = std::optional<CORE_ADDR> (struct gdbarch *gdbarch, regcache *regcache, bool &shadow_stack_enabled);
 std::optional<CORE_ADDR> gdbarch_get_shadow_stack_pointer (struct gdbarch *gdbarch, regcache *regcache, bool &shadow_stack_enabled);
 void set_gdbarch_get_shadow_stack_pointer (struct gdbarch *gdbarch, gdbarch_get_shadow_stack_pointer_ftype *get_shadow_stack_pointer);
+
+/* Returns true if architecture has imprecise page fault reporting, where a
+   memory access violation may be reported several instructions after the
+   faulting instruction.
+
+   On such architectures, if an instruction causes a memory access violation,
+   the backend may report the stop at a later instruction where a breakpoint
+   happens to be installed.  GDB's normal logic would incorrectly assume that
+   the SIGSEGV was caused by hitting the breakpoint and convert it to SIGTRAP.
+
+   However, if an architecture never reports breakpoints via SIGSEGV (e.g.,
+   breakpoints are always reported via a dedicated mechanism), then receiving
+   a SIGSEGV at a breakpoint location means we have a genuine SIGSEGV, not a
+   breakpoint hit.
+
+   Return true for architectures with imprecise fault reporting to disable the
+   SIGSEGV-to-SIGTRAP conversion.  The default is false (conversion enabled)
+   to preserve existing behavior for architectures where breakpoints may be
+   reported as SIGSEGV (e.g., executing a breakpoint instruction on a
+   non-executable stack). */
+
+using gdbarch_imprecise_pagefault_reporting_ftype = bool ();
+bool gdbarch_imprecise_pagefault_reporting (struct gdbarch *gdbarch);
+void set_gdbarch_imprecise_pagefault_reporting (struct gdbarch *gdbarch, gdbarch_imprecise_pagefault_reporting_ftype *imprecise_pagefault_reporting);
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index d8b2d114909..09d3130e753 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -2789,3 +2789,32 @@ SHADOW_STACK_ENABLED to false.
     predefault="default_get_shadow_stack_pointer",
     invalid=False,
 )
+
+Function(
+    comment="""
+Returns true if architecture has imprecise page fault reporting, where a
+memory access violation may be reported several instructions after the
+faulting instruction.
+
+On such architectures, if an instruction causes a memory access violation,
+the backend may report the stop at a later instruction where a breakpoint
+happens to be installed.  GDB's normal logic would incorrectly assume that
+the SIGSEGV was caused by hitting the breakpoint and convert it to SIGTRAP.
+
+However, if an architecture never reports breakpoints via SIGSEGV (e.g.,
+breakpoints are always reported via a dedicated mechanism), then receiving
+a SIGSEGV at a breakpoint location means we have a genuine SIGSEGV, not a
+breakpoint hit.
+
+Return true for architectures with imprecise fault reporting to disable the
+SIGSEGV-to-SIGTRAP conversion.  The default is false (conversion enabled)
+to preserve existing behavior for architectures where breakpoints may be
+reported as SIGSEGV (e.g., executing a breakpoint instruction on a
+non-executable stack).
+""",
+    type="bool",
+    name="imprecise_pagefault_reporting",
+    params=[],
+    predefault="[] () -> bool {return false;}",
+    invalid=False,
+)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c0767e7f764..25ed7e7523f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6331,7 +6331,9 @@ handle_inferior_event (struct execution_control_state *ecs)
      stack.  */
   if (ecs->ws.kind () == TARGET_WAITKIND_STOPPED
       && (ecs->ws.sig () == GDB_SIGNAL_ILL
-	  || ecs->ws.sig () == GDB_SIGNAL_SEGV
+	  || (ecs->ws.sig () == GDB_SIGNAL_SEGV
+	      && !gdbarch_imprecise_pagefault_reporting
+		  (target_thread_architecture (ecs->event_thread->ptid)))
 	  || ecs->ws.sig () == GDB_SIGNAL_EMT))
     {
       struct regcache *regcache = get_thread_regcache (ecs->event_thread);
-- 
2.34.1

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.