Get frame after pull single-step breakpoints out
Commit Message
On 04/03/2014 07:09 AM, Yao Qi wrote:
> Hi,
> I find that instruction that red by signal trampoline unwinder sniffer
> is always the breakpoint instruction. I suspect that GDB tries to get
> frame before pulls breakpoint out of the target.
Thing is, breakpoints are supposed to be transparent to memory
accesses.
Say, as is, a background step + disassemble will see breakpoints
instructions. E.g., stepping over this line:
while (1);
with s&
and then "disassemble" would show sss breakpoints.
It may actually not be possible to see that today, because:
- in native Linux, you can't read memory while the program
is running.
- with Linux gdbserver, you can, but in the all-stop RSP you
can't talk to the server while the program is running...
- and with non-stop, on software step targets, we presently
force the use of displaced-stepping for all single-steps,
so no single-step breakpoints are used...
I've been working towards making non-stop not force
displaced stepping on sss targets, and I noticed the issue
then. With that, I see this:
(gdb) set remote Z-packet off
(gdb) s&
(gdb) disassemble main
Dump of assembler code for function main:
0x000000000040049c <+0>: push %rbp
0x000000000040049d <+1>: mov %rsp,%rbp
0x00000000004004a0 <+4>: int3
0x00000000004004a1 <+5>: (bad)
End of assembler dump.
Instead of the correct:
(gdb) disassemble main
Dump of assembler code for function main:
0x000000000040049c <+0>: push %rbp
0x000000000040049d <+1>: mov %rsp,%rbp
0x00000000004004a0 <+4>: jmp 0x4004a0 <main+4>
This is actually one thing that my v1 of the recent
"fix a bunch of run control bugs" series was fixing, because
it made sss breakpoints be regular breakpoints in the breakpoint
chain. But I had to drop that in v2.
For all kinds of breakpoints breakpoint_xfer_memory hides the
breakpoint instructions, but sss breakpoints aren't tracked like all
other breakpoints.
So instead of making sss breakpoints regular breakpoints, go with
a simpler fix (at least for now) -- make breakpoint_xfer_memory
take software single-step breakpoints into account. Something
like the patch below. After the patch, I get the correct
disassemble output.
Tested on x86_64 Fedora 17, and also on top of my
"use software single-step on x86" series.
Does this work for you?
-------
Subject: [PATCH] breakpoint shadowing, take single-step breakpoints into
account.
gdb/
2014-04-04 Pedro Alves <palves@redhat.com>
* breakpoint.c (single_step_breakpoints)
(single_step_gdbarch): Move up in the file.
(one_breakpoint_xfer_memory): New function, factored out from ...
(breakpoint_xfer_memory): ... here. Also process single-step
breakpoints.
---
gdb/breakpoint.c | 186 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 117 insertions(+), 69 deletions(-)
Comments
This patch fixes the fails I saw on nios2-linux testing. Thanks.
On 04/04/2014 08:49 PM, Pedro Alves wrote:
> This is actually one thing that my v1 of the recent
> "fix a bunch of run control bugs" series was fixing, because
> it made sss breakpoints be regular breakpoints in the breakpoint
> chain. But I had to drop that in v2.
>
> For all kinds of breakpoints breakpoint_xfer_memory hides the
> breakpoint instructions, but sss breakpoints aren't tracked like all
> other breakpoints.
>
> So instead of making sss breakpoints regular breakpoints, go with
> a simpler fix (at least for now) -- make breakpoint_xfer_memory
> take software single-step breakpoints into account. Something
> like the patch below. After the patch, I get the correct
> disassemble output.
This will be complicated when we need per-thread software single step
breakpoint. IIRC, 'run all-stop mode on top of non-stop target' needs
it.
The problem is sss breakpoint is there when frame unwinder sniffer
reads memory. Your fix is to make sss breakpoint invisible, while mine
is pull sss breakpoint out before sniffer reads memory. I can't see
anything wrong with my patch. It is simple, and are you OK to consider
it?
On 04/10/2014 04:46 AM, Yao Qi wrote:
> This patch fixes the fails I saw on nios2-linux testing. Thanks.
Thanks, pushed:
https://sourceware.org/ml/gdb-patches/2014-04/msg00176.html
>
> On 04/04/2014 08:49 PM, Pedro Alves wrote:
>> This is actually one thing that my v1 of the recent
>> "fix a bunch of run control bugs" series was fixing, because
>> it made sss breakpoints be regular breakpoints in the breakpoint
>> chain. But I had to drop that in v2.
>>
>> For all kinds of breakpoints breakpoint_xfer_memory hides the
>> breakpoint instructions, but sss breakpoints aren't tracked like all
>> other breakpoints.
>>
>> So instead of making sss breakpoints regular breakpoints, go with
>> a simpler fix (at least for now) -- make breakpoint_xfer_memory
>> take software single-step breakpoints into account. Something
>> like the patch below. After the patch, I get the correct
>> disassemble output.
>
> This will be complicated when we need per-thread software single step
> breakpoint.
Hmm, what's the "this" you're referring to? Either the single step
breakpoints remain separate from regular breakpoints, and breakpoint_xfer_memory
will iterate over sss breakpoints of all threads, or sss breakpoints
become regular breakpoints in the breakpoint location chain,
and breakpoint_xfer_memory doesn't need to care about them at all.
> IIRC, 'run all-stop mode on top of non-stop target' needs
> it.
Right, currently non-stop forces displaced stepping on software
single-step targets.
> The problem is sss breakpoint is there when frame unwinder sniffer
> reads memory.
Well, that's _one_ manifestation of the issue. The "disassemble"
one I've shown is another. I've previously said that we may not
be able to trigger it today, but actually we can. Native GNU/Linux
gdb can't read memory off a running thread, but I had forgotten
multi-threading and scheduler-locking. With scheduler-locking
on, we can switch to a stopped thread, and disassemble the code
that is being stepped over.
E.g., setting one thread stepping over this inf loop in the
background:
void *thread_function0(void *arg) {
while (1);
Like so:
(gdb) info threads
Id Target Id Frame
3 Thread 0x7ffff77ca700 (LWP 2644) "threads_inf_ste" 0x000000323d4ba6ed in nanosleep () at ../sysdeps/unix/syscall-template.S:82
2 Thread 0x7ffff7fcb700 (LWP 2643) "threads_inf_ste" thread_function0 (arg=0x0) at threads_inf_step.c:60
* 1 Thread 0x7ffff7fcc740 (LWP 2639) "threads_inf_ste" 0x00000034cf408e60 in pthread_join (threadid=140737353922304, thread_return=0x7fffffffd9e8) at pthread_join.c:93
(gdb) t 2
[Switching to thread 2 (Thread 0x7ffff7fcb700 (LWP 2643))]
#0 thread_function0 (arg=0x0) at threads_inf_step.c:60
60 while (1);
(gdb) set scheduler-locking on
(gdb) s&
Now switch to a stopped thread:
(gdb) t 1
[Switching to thread 1 (Thread 0x7ffff7fcc740 (LWP 2639))]
#0 0x00000034cf408e60 in pthread_join (threadid=140737353922304, thread_return=0x7fffffffd9e8) at pthread_join.c:93
93 lll_wait_tid (pd->tid);
And disassemble the function thread 2 is running:
(gdb) disassemble thread_function0
Dump of assembler code for function thread_function0:
0x0000000000400791 <+0>: push %rbp
0x0000000000400792 <+1>: mov %rsp,%rbp
0x0000000000400795 <+4>: push %rbx
0x0000000000400796 <+5>: mov %rdi,-0x20(%rbp)
0x000000000040079a <+9>: mov -0x20(%rbp),%rax
0x000000000040079e <+13>: mov %eax,%ebx
0x00000000004007a0 <+15>: movslq %ebx,%rax
0x00000000004007a3 <+18>: shl $0x2,%rax
0x00000000004007a7 <+22>: add $0x600c70,%rax
0x00000000004007ad <+28>: mov %rax,-0x10(%rbp)
0x00000000004007b1 <+32>: int3
0x00000000004007b2 <+33>: (bad)
End of assembler dump.
Notice "int3", and "(bad)".
So we should actually be able to turn this into a test case.
(one that disassembles the function before the step, and
compares that to the disassembly while the step is ongoing.)
And we may well have other manifestations of the same issue.
The root cause is that all breakpoints should be transparent to memory
reads, but these weren't because they use the
deprecated_insert_raw_breakpoint mechanism, _and_ nios2 gdbserver
doesn't support Z0 breakpoints. With Z0 breakpoints, gdbserver
takes care of hiding the sss breakpoints from memory reads, because
from gdbserver's perspective, it's just another breakpoint.
> Your fix is to make sss breakpoint invisible, while mine
> is pull sss breakpoint out before sniffer reads memory. I can't see
> anything wrong with my patch. It is simple, and are you OK to consider
> it?
But after we fix the root issue, that change just looks pointless
to me?
On 04/10/2014 09:58 PM, Pedro Alves wrote:
> Hmm, what's the "this" you're referring to? Either the single step
> breakpoints remain separate from regular breakpoints, and breakpoint_xfer_memory
> will iterate over sss breakpoints of all threads, or sss breakpoints
> become regular breakpoints in the breakpoint location chain,
> and breakpoint_xfer_memory doesn't need to care about them at all.
>
I meant breakpoint_xfer_memory will iterate over sss breakpoints of all
threads. Let's see what the problem might be when we really go there.
@@ -293,6 +293,12 @@ static struct breakpoint_ops bkpt_probe_breakpoint_ops;
/* Dynamic printf class type. */
struct breakpoint_ops dprintf_breakpoint_ops;
+/* One (or perhaps two) breakpoints used for software single
+ stepping. */
+
+static void *single_step_breakpoints[2];
+static struct gdbarch *single_step_gdbarch[2];
+
/* The style in which to perform a dynamic printf. This is a user
option because different output options have different tradeoffs;
if GDB does the printing, there is better error handling if there
@@ -1409,6 +1415,100 @@ bp_location_has_shadow (struct bp_location *bl)
return 1;
}
+/* Update BUF, which is LEN bytes read from the target address
+ MEMADDR, by replacing a memory breakpoint with its shadowed
+ contents.
+
+ If READBUF is not NULL, this buffer must not overlap with the of
+ the breakpoint location's shadow_contents buffer. Otherwise, a
+ failed assertion internal error will be raised. */
+
+static void
+one_breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
+ const gdb_byte *writebuf_org,
+ ULONGEST memaddr, LONGEST len,
+ struct bp_target_info *target_info,
+ struct gdbarch *gdbarch)
+{
+ /* Now do full processing of the found relevant range of elements. */
+ CORE_ADDR bp_addr = 0;
+ int bp_size = 0;
+ int bptoffset = 0;
+
+ if (!breakpoint_address_match (target_info->placed_address_space, 0,
+ current_program_space->aspace, 0))
+ {
+ /* The breakpoint is inserted in a different address space. */
+ return;
+ }
+
+ /* Addresses and length of the part of the breakpoint that
+ we need to copy. */
+ bp_addr = target_info->placed_address;
+ bp_size = target_info->shadow_len;
+
+ if (bp_addr + bp_size <= memaddr)
+ {
+ /* The breakpoint is entirely before the chunk of memory we are
+ reading. */
+ return;
+ }
+
+ if (bp_addr >= memaddr + len)
+ {
+ /* The breakpoint is entirely after the chunk of memory we are
+ reading. */
+ return;
+ }
+
+ /* Offset within shadow_contents. */
+ if (bp_addr < memaddr)
+ {
+ /* Only copy the second part of the breakpoint. */
+ bp_size -= memaddr - bp_addr;
+ bptoffset = memaddr - bp_addr;
+ bp_addr = memaddr;
+ }
+
+ if (bp_addr + bp_size > memaddr + len)
+ {
+ /* Only copy the first part of the breakpoint. */
+ bp_size -= (bp_addr + bp_size) - (memaddr + len);
+ }
+
+ if (readbuf != NULL)
+ {
+ /* Verify that the readbuf buffer does not overlap with the
+ shadow_contents buffer. */
+ gdb_assert (target_info->shadow_contents >= readbuf + len
+ || readbuf >= (target_info->shadow_contents
+ + target_info->shadow_len));
+
+ /* Update the read buffer with this inserted breakpoint's
+ shadow. */
+ memcpy (readbuf + bp_addr - memaddr,
+ target_info->shadow_contents + bptoffset, bp_size);
+ }
+ else
+ {
+ const unsigned char *bp;
+ CORE_ADDR placed_address = target_info->placed_address;
+ int placed_size = target_info->placed_size;
+
+ /* Update the shadow with what we want to write to memory. */
+ memcpy (target_info->shadow_contents + bptoffset,
+ writebuf_org + bp_addr - memaddr, bp_size);
+
+ /* Determine appropriate breakpoint contents and size for this
+ address. */
+ bp = gdbarch_breakpoint_from_pc (gdbarch, &placed_address, &placed_size);
+
+ /* Update the final write buffer with this inserted
+ breakpoint's INSN. */
+ memcpy (writebuf + bp_addr - memaddr, bp + bptoffset, bp_size);
+ }
+}
+
/* Update BUF, which is LEN bytes read from the target address MEMADDR,
by replacing any memory breakpoints with their shadowed contents.
@@ -1435,6 +1535,7 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
/* Left boundary, right boundary and median element of our binary
search. */
unsigned bc_l, bc_r, bc;
+ size_t i;
/* Find BC_L which is a leftmost element which may affect BUF
content. It is safe to report lower value but a failure to
@@ -1508,74 +1609,27 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
if (!bp_location_has_shadow (bl))
continue;
- if (!breakpoint_address_match (bl->target_info.placed_address_space, 0,
- current_program_space->aspace, 0))
- continue;
-
- /* Addresses and length of the part of the breakpoint that
- we need to copy. */
- bp_addr = bl->target_info.placed_address;
- bp_size = bl->target_info.shadow_len;
-
- if (bp_addr + bp_size <= memaddr)
- /* The breakpoint is entirely before the chunk of memory we
- are reading. */
- continue;
- if (bp_addr >= memaddr + len)
- /* The breakpoint is entirely after the chunk of memory we are
- reading. */
- continue;
+ one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
+ memaddr, len, &bl->target_info, bl->gdbarch);
+ }
- /* Offset within shadow_contents. */
- if (bp_addr < memaddr)
- {
- /* Only copy the second part of the breakpoint. */
- bp_size -= memaddr - bp_addr;
- bptoffset = memaddr - bp_addr;
- bp_addr = memaddr;
- }
+ /* Now process single-step breakpoints. These are not found in the
+ bp_location array. */
+ for (i = 0; i < 2; i++)
+ {
+ struct bp_target_info *bp_tgt = single_step_breakpoints[i];
- if (bp_addr + bp_size > memaddr + len)
- {
- /* Only copy the first part of the breakpoint. */
- bp_size -= (bp_addr + bp_size) - (memaddr + len);
- }
+ if (bp_tgt != NULL)
+ {
+ struct gdbarch *gdbarch = single_step_gdbarch[i];
- if (readbuf != NULL)
- {
- /* Verify that the readbuf buffer does not overlap with
- the shadow_contents buffer. */
- gdb_assert (bl->target_info.shadow_contents >= readbuf + len
- || readbuf >= (bl->target_info.shadow_contents
- + bl->target_info.shadow_len));
-
- /* Update the read buffer with this inserted breakpoint's
- shadow. */
- memcpy (readbuf + bp_addr - memaddr,
- bl->target_info.shadow_contents + bptoffset, bp_size);
- }
- else
- {
- struct gdbarch *gdbarch = bl->gdbarch;
- const unsigned char *bp;
- CORE_ADDR placed_address = bl->target_info.placed_address;
- int placed_size = bl->target_info.placed_size;
-
- /* Update the shadow with what we want to write to memory. */
- memcpy (bl->target_info.shadow_contents + bptoffset,
- writebuf_org + bp_addr - memaddr, bp_size);
-
- /* Determine appropriate breakpoint contents and size for this
- address. */
- bp = gdbarch_breakpoint_from_pc (gdbarch, &placed_address, &placed_size);
-
- /* Update the final write buffer with this inserted
- breakpoint's INSN. */
- memcpy (writebuf + bp_addr - memaddr, bp + bptoffset, bp_size);
- }
- }
+ one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
+ memaddr, len, bp_tgt, gdbarch);
+ }
+ }
}
+
/* Return true if BPT is either a software breakpoint or a hardware
@@ -15036,12 +15090,6 @@ deprecated_remove_raw_breakpoint (struct gdbarch *gdbarch, void *bp)
return ret;
}
-/* One (or perhaps two) breakpoints used for software single
- stepping. */
-
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
-
/* Create and insert a breakpoint for software single step. */
void