Re: [Qemu-devel] [RFA] Always consider infcall breakpoints as non-permanent.

2014-11-23 Thread Joel Brobecker
  And seen from QEMU:
  
  qemu: fatal: Trap 0x02 while interrupts disabled, Error state
  [register dump removed]
 
 Bah.  Do you know whether this happens only on SPARC QEMU, or
 is this an issue for all QEMU ports?

According to Fabien, one of our QEMU experts, it's related to the
handling of that specific instruction: it does what the manual says
it should do - when interrupts are disables, the CPU resets. So
while it does not preclude from other ports from having the same
sort of issue, it looks like it's mostly target-specific.

  gdb/ChangeLog:
  
  * breakpoint.c (bp_loc_is_permanent): Return 0 if LOC corresponds
  to a bp_call_dummy breakpoint type.
 Patch is OK.  Thanks Joel.

Thank you. Attached is the patch I checked in with the corrections
you pointed out.

Thanks, Pedro.
-- 
Joel
From e8af5d7a5cd4c58136a4733e87612f49061bf28b Mon Sep 17 00:00:00 2001
From: Joel Brobecker brobec...@adacore.com
Date: Thu, 20 Nov 2014 20:41:25 +0400
Subject: [PATCH] Always consider infcall breakpoints as non-permanent.

A recent change...

commit 1a853c5224e2b8fedfac6d029365522b83080b40
Date:   Wed Nov 12 10:10:49 2014 +
Subject: make permanent breakpoints per location and disableable

... broke function calls on sparc-elf when running over QEMU. Any
function call should demonstrate the problem.

For instance, seen from the debugger:

(gdb) call pn(1234)
[Inferior 1 (Remote target) exited normally]
The program being debugged exited while in a function called from GDB.
Evaluation of the expression containing the function

And seen from QEMU:

qemu: fatal: Trap 0x02 while interrupts disabled, Error state
[register dump removed]

What happens in this case is that GDB sets the inferior function call
by not only creating the dummy frame, but also writing a breakpoint
instruction at the return address for our function call. See infcall.c:

/* Write a legitimate instruction at the point where the infcall
   breakpoint is going to be inserted.  While this instruction
   is never going to be executed, a user investigating the
   memory from GDB would see this instruction instead of random
   uninitialized bytes.  We chose the breakpoint instruction
   as it may look as the most logical one to the user and also
   valgrind 3.7.0 needs it for proper vgdb inferior calls.

   If software breakpoints are unsupported for this target we
   leave the user visible memory content uninitialized.  */

bp_addr_as_address = bp_addr;
bp_bytes = gdbarch_breakpoint_from_pc (gdbarch, bp_addr_as_address,
   bp_size);
if (bp_bytes != NULL)
  write_memory (bp_addr_as_address, bp_bytes, bp_size);

This instruction triggers a change introduced by the commit above,
where we consider bp locations as being permanent breakpoints
if there is already a breakpoint instruction at that address:

+  if (bp_loc_is_permanent (loc))
+{
+  loc-inserted = 1;
+  loc-permanent = 1;
+}

As a result, when resuming the program's execution for the inferior
function call, GDB decides that it does not need to insert a breakpoint
at this address, expecting the target to just report a SIGTRAP when
trying to execute that instruction.

But unfortunately for us, at least some versions of QEMU for SPARC
just terminate the execution entirely instead of reporting a breakpoint,
thus producing the behavior reported here.

Although it appears like QEMU might be misbehaving and should therefore
be fixed (to be verified) from the user's point of view, the recent
change does introduce a regression. So this patch tries to mitigate
a bit the damage by handling such infcall breakpoints as special and
making sure that they are never considered permanent, thus restoring
the previous behavior specifically for those breakpoints.

The option of not writing the breakpoint instructions in the first
place was considered, and would probably work also. But the comment
associated to it seems to indicate that there is still reason to
keep it.

gdb/ChangeLog:

* breakpoint.c (bp_loc_is_permanent): Return 0 if LOC corresponds
to a bp_call_dummy breakpoint type.

Tested on x86_64-linux. Also testing on sparc-elf/QEMU using
AdaCore's testsuite.
---
 gdb/ChangeLog|  5 +
 gdb/breakpoint.c | 14 ++
 2 files changed, 19 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 45435fc..4e8284e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2014-11-23  Joel Brobecker  brobec...@adacore.com
+
+	* breakpoint.c (bp_loc_is_permanent): Return 0 if LOC corresponds
+	to a bp_call_dummy breakpoint type.
+
 2014-11-23  Patrick Palka  patr...@parcs.ath.cx
 
 	* tui/tui-win.c (tui_initialize_win): Specify SA_RESTART when
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7b56260..574d06c 

Re: [Qemu-devel] [RFA] Always consider infcall breakpoints as non-permanent.

2014-11-21 Thread Pedro Alves
[Adding qemu-devel@, so folks over there are aware of this.
Original thread at https://sourceware.org/ml/gdb-patches/2014-11/msg00478.html]

On 11/20/2014 04:41 PM, Joel Brobecker wrote:
 A recent change...
 
 commit 1a853c5224e2b8fedfac6d029365522b83080b40
 Date:   Wed Nov 12 10:10:49 2014 +
 Subject: make permanent breakpoints per location and disableable
 
 ... broke function calls on sparc-elf when running over QEMU. Any
 function call should demonstrate the problem.
 
 For instance, seen from the debugger:
 
 (gdb) call pn(1234)
 [Inferior 1 (Remote target) exited normally]
 The program being debugged exited while in a function called from GDB.
 Evaluation of the expression containing the function
 
 And seen from QEMU:
 
 qemu: fatal: Trap 0x02 while interrupts disabled, Error state
 [register dump removed]

Bah.  Do you know whether this happens only on SPARC QEMU, or
is this an issue for all QEMU ports?

Basically, it sounds like with QEMU, GDB can't ever poke
breakpoint instructions to memory.  It must always set breakpoints
with the Z0 packet.

 
 What happens in this case is that GDB sets the inferior function call
 by not only creating the dummy frame, but also writing a breakpoint
 instruction at the return address for our funcation call. See infcall.c:

funcation

 
 /* Write a legitimate instruction at the point where the infcall
breakpoint is going to be inserted.  While this instruction
is never going to be executed, a user investigating the
memory from GDB would see this instruction instead of random
uninitialized bytes.  We chose the breakpoint instruction
as it may look as the most logical one to the user and also
valgrind 3.7.0 needs it for proper vgdb inferior calls.
 
If software breakpoints are unsupported for this target we
leave the user visible memory content uninitialized.  */
 
 bp_addr_as_address = bp_addr;
 bp_bytes = gdbarch_breakpoint_from_pc (gdbarch, bp_addr_as_address,
bp_size);
 if (bp_bytes != NULL)
   write_memory (bp_addr_as_address, bp_bytes, bp_size);
 
 This instruction triggers a change introduced by the commit above,
 where we consider bp locations as being permanent breakpoints
 if there is already a breakpoint instruction at that address:
 
 +  if (bp_loc_is_permanent (loc))
 +{
 +  loc-inserted = 1;
 +  loc-permanent = 1;
 +}
 
 As a result, when resuming the program's execution for the inferior
 function call, GDB decides that it does not need to insert a breakpoint
 at this address, expecting the target to just report a SIGTRAP when
 trying to execute that instruction.
 
 But unfortunately for us, at least some versions of QEMU for SPARC
 just terminate the execution entirely instead of reporting a breakpoint,
 thus producing the behavior reported here.
 
 Although it appears like QEMU might be misbehaving and should therefore
 be fixed (to be verified) from the user's point of view, the recent
 change does introduce a regression. So this patch tries to mitigate
 a bit the damage by handling such infcall breakpoints as special and
 making sure that they are never considered permanent, thus restoring
 the previous behavior specifically for those breakpoints.
 
 The option of not writing the breakpoint instructions ini the first

ini

 place was considered, and would probably work also. But the comment
 associated to it seems to indicate that there is still reason to
 keep it.
 
 gdb/ChangeLog:
 
 * breakpoint.c (bp_loc_is_permanent): Return 0 if LOC corresponds
 to a bp_call_dummy breakpoint type.
 
 Tested on x86_64-linux. Also testing on sparc-elf/QEMU using
 AdaCore's testsuite.
 ---
  gdb/breakpoint.c | 13 +
  1 file changed, 13 insertions(+)
 
 diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
 index 1c0a417..e22ac92 100644
 --- a/gdb/breakpoint.c
 +++ b/gdb/breakpoint.c
 @@ -9289,6 +9289,19 @@ bp_loc_is_permanent (struct bp_location *loc)
  
gdb_assert (loc != NULL);
  
 +  /* bp_call_dummy breakpoint locations are usually memory locations where
 + GDB just wrote a breakpoint instruction, making it look as if there is
 + a permanent breakpoint at that location.  Considering it permanent makes
 + GDB rely on that breakpoint instruction to stop the program, thus 
 removing
 + the need to insert its own breakpoint there.  This is normally expected 
 to
 + work, except that some versions of QEMU (Eg: QEMU 2.0.0 for SPARC) just
 + report a fatal problem (Trap 0x02 while interrupts disabled, Error 
 state)
 + intead of reporting a SIGTRAP.  QEMU should probably be fixed, but in

intead

 + the interest of compatibility with versions that behave this way, we 
 always
 + consider bp_call_dummy breakpoint locations as