[PATCH/RFC] Simplify adjust_pc_after_break?

[PATCH/RFC] Simplify adjust_pc_after_break?

[Mark Kettenis]

The signal tests are failing horribly on Solaris x86.  The problem is
that upon hitting a breakpoint in the signal handler, we don't
properly back up the PC.

The problem here seems to stem from the fact that we were
single-stepping when the signal arrived.  The signal takes us out of
the single-stepping range, and when we hit the breakpoint, we're atan
address that's miles away from PREV_PC.  As a result
adjust_pc_after_break decides that there is no reason to back up the
PC.

Why don't we see this on other i386 targets?  There is some code in
handle_inferior_event() that notices when a signal takes us out of the
single-stepping range.  However, procfs(4) has this nice feature that
you can tell it what signals to report.  We tell it to only report
signals that we're interested in.  In this particular case we're not
interested in the SIGALRM, so GDB never sees it, and the code in
handle_inferior_event() is never executed.

Looking at adjust_pc_after_break() I couldn't figure out why we're
trying do.  I could imagine that we shouldn't back up the PC if a
breakpoint and single-step coalesce, but that's exactly the condition
where we *do* back up the PC.  So why not simplify things and just
back up the PC if we detect an inserted breakpoint?

The attached patch works for me on Solaris x86 and various other i386
and amd64 targets.  But I'm probably overlooking something.

Mark


Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.163
diff -u -p -r1.163 infrun.c
--- infrun.c 14 May 2004 18:45:42 -0000 1.163
+++ infrun.c 16 May 2004 23:12:52 -0000
@@ -1234,29 +1234,8 @@ adjust_pc_after_break (struct execution_
     }
   else
     {
-      /* When using hardware single-step, a SIGTRAP is reported for
-	 both a completed single-step and a software breakpoint.  Need
-	 to differentiate between the two as the latter needs
-	 adjusting but the former does not.  */
-      if (currently_stepping (ecs))
-	{
-	  if (prev_pc == breakpoint_pc
-	      && software_breakpoint_inserted_here_p (breakpoint_pc))
-	    /* Hardware single-stepped a software breakpoint (as
-	       occures when the inferior is resumed with PC pointing
-	       at not-yet-hit software breakpoint).  Since the
-	       breakpoint really is executed, the inferior needs to be
-	       backed up to the breakpoint address.  */
-	    write_pc_pid (breakpoint_pc, ecs->ptid);
-	}
-      else
-	{
-	  if (software_breakpoint_inserted_here_p (breakpoint_pc))
-	    /* The inferior was free running (i.e., no hardware
-	       single-step and no possibility of a false SIGTRAP) and
-	       hit a software breakpoint.  */
-	    write_pc_pid (breakpoint_pc, ecs->ptid);
-	}
+      if (software_breakpoint_inserted_here_p (breakpoint_pc))
+	write_pc_pid (breakpoint_pc, ecs->ptid);
     }
 }
 

Re: [PATCH/RFC] Simplify adjust_pc_after_break?

[Andrew Cagney]

What happens when you hardware single step through a branch vis:

	branch 1 <prev_pc>

	<breakpoint>
1:	<pc>
Andrew

Re: [PATCH/RFC] Simplify adjust_pc_after_break?

[Mark Kettenis]

   Date: Mon, 17 May 2004 10:22:56 -0400
   From: Andrew Cagney <cagney@gnu.org>

   What happens when you hardware single step through a branch vis:

	   branch 1 <prev_pc>

	   <breakpoint>
   1:	<pc>

That breaks :-(.

Thanks,

Mark

Re: [PATCH/RFC] Simplify adjust_pc_after_break?

[Daniel Jacobowitz]

On Mon, May 17, 2004 at 01:28:36AM +0200, Mark Kettenis wrote:
> The signal tests are failing horribly on Solaris x86.  The problem is
> that upon hitting a breakpoint in the signal handler, we don't
> properly back up the PC.

Hi Mark,

While sorting through old mail I found this message.  Andrew explained
why your patch was wrong; has this been fixed some other way, or are we
still broken on Solaris x86?

> The problem here seems to stem from the fact that we were
> single-stepping when the signal arrived.  The signal takes us out of
> the single-stepping range, and when we hit the breakpoint, we're atan
> address that's miles away from PREV_PC.  As a result
> adjust_pc_after_break decides that there is no reason to back up the
> PC.
> 
> Why don't we see this on other i386 targets?  There is some code in
> handle_inferior_event() that notices when a signal takes us out of the
> single-stepping range.  However, procfs(4) has this nice feature that
> you can tell it what signals to report.  We tell it to only report
> signals that we're interested in.  In this particular case we're not
> interested in the SIGALRM, so GDB never sees it, and the code in
> handle_inferior_event() is never executed.
> 
> Looking at adjust_pc_after_break() I couldn't figure out why we're
> trying do.  I could imagine that we shouldn't back up the PC if a
> breakpoint and single-step coalesce, but that's exactly the condition
> where we *do* back up the PC.  So why not simplify things and just
> back up the PC if we detect an inserted breakpoint?
> 
> The attached patch works for me on Solaris x86 and various other i386
> and amd64 targets.  But I'm probably overlooking something.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH/RFC] Simplify adjust_pc_after_break?

[Daniel Jacobowitz]

[Trying again with Mark's current email.]

On Mon, May 17, 2004 at 01:28:36AM +0200, Mark Kettenis wrote:
> The signal tests are failing horribly on Solaris x86.  The problem is
> that upon hitting a breakpoint in the signal handler, we don't
> properly back up the PC.

Hi Mark,

While sorting through old mail I found this message.  Andrew explained
why your patch was wrong; has this been fixed some other way, or are we
still broken on Solaris x86?

> The problem here seems to stem from the fact that we were
> single-stepping when the signal arrived.  The signal takes us out of
> the single-stepping range, and when we hit the breakpoint, we're atan
> address that's miles away from PREV_PC.  As a result
> adjust_pc_after_break decides that there is no reason to back up the
> PC.
> 
> Why don't we see this on other i386 targets?  There is some code in
> handle_inferior_event() that notices when a signal takes us out of the
> single-stepping range.  However, procfs(4) has this nice feature that
> you can tell it what signals to report.  We tell it to only report
> signals that we're interested in.  In this particular case we're not
> interested in the SIGALRM, so GDB never sees it, and the code in
> handle_inferior_event() is never executed.
> 
> Looking at adjust_pc_after_break() I couldn't figure out why we're
> trying do.  I could imagine that we shouldn't back up the PC if a
> breakpoint and single-step coalesce, but that's exactly the condition
> where we *do* back up the PC.  So why not simplify things and just
> back up the PC if we detect an inserted breakpoint?
> 
> The attached patch works for me on Solaris x86 and various other i386
> and amd64 targets.  But I'm probably overlooking something.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH/RFC] Simplify adjust_pc_after_break?

[Mark Kettenis]

   Date: Sat, 26 Mar 2005 17:23:32 -0500
   From: Daniel Jacobowitz <drow@false.org>

   On Mon, May 17, 2004 at 01:28:36AM +0200, Mark Kettenis wrote:
   > The signal tests are failing horribly on Solaris x86.  The problem is
   > that upon hitting a breakpoint in the signal handler, we don't
   > properly back up the PC.

   Hi Mark,

   While sorting through old mail I found this message.  Andrew explained
   why your patch was wrong; has this been fixed some other way, or are we
   still broken on Solaris x86?

AFAIK it's still broken.  I had a patch to fix this using a procfs
option that turns Solaris x86 into a "no need to backup the pc"
system.  But that option is only available on Solaris 2.7 and up (or
something like it).  Unfortunately I lost that patch in a recent disk
crash :-(.

Mark