[rfc] Call init_wait_for_inferior before get_offsets

Yao Qi
2011-06-20T05:12:39Z

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

The flag `inserted' in bp loc is not managed correctly in the case that
gdb gets reply from gdbserver on packet "qOffset".

In order to illustrate this problem clearly, here is a simple callgraph,

remote_start_remote
|
+-> get_offsets
| |
| `-> objfile_relocate // if gdbserver replies offsets
| |
| `-> breakpoint_re_set//[1] insert bp and loc->inserted = 1
|
`-> start_remote
|
+-> init_wait_for_inferior
| |
| `-> breakpoint_init_inferior //[2] loc->inserted = 0
|
`-> post_create_inferior
|
`-> breakpoint_re_set//[3] insert bp again, and
loc-inserted = 1

Supposing we have inserted an breakpoint B on address A1 before any
target is created. Then, we create a target remote. When gdbserver
replies qOffset packet with an offset map (usually, in uclinux,
gdbserver will do this), objfile_relocate is called, and
breakpoint_re_set [1] is called, so breakpoint B is inserted on address
A2, which is the relocated address of A1. Then, in [2],
breakpoint_init_inferior is called, so the bp loc corresponding to B is
reset, and flag `inserted' is set to zero. Finally, when goes to
post_create_inferior, breakpoint_re_set is called, since breakpoint B's
loc's flag `inserted' has been zero (cleaned in [2]), gdb will insert
breakpoint again at the same address A2, however breakpoint instruction
has on address A2. gdb will read breakpoint instruction into shadow,
and store the same breakpoint instruction on address A2. The original
instruction is lost!.

The cause of this problem is breakpoint_init_inferioris called later
than breakpoint_re_set. This patch is to move init_wait_for_inferior
forward, so that it can be called before breakpoint_re_set.

This problem is revealed by following fail in my uclinux port, and this
patch fixes this fail.

FAIL: gdb.base/break-always.exp: continue to breakpoint: bar

This patch is regression tested on x86_64-known-linux with board file
native-gdbserver.exp as well. Although there are some mi tests PASSes
and FAILs(timeout), they are caused by instable tests, instead of my patch.

I am still not very familiar with infrun.c and remote.c, so comments are
welcome.

--
Yao (é½å°§)

[-- Attachment #2: 0001-move-init_wait_for_inferior-forward.patch --]
[-- Type: text/x-patch, Size: 1023 bytes --]

2011-06-20 Yao Qi <yao@codesourcery.com>

* infrun.c (start_remote): Move call init_wait_for_inferior to ...
* remote.c (remote_start_remote): ... here.

---
gdb/infrun.c | 1 -
gdb/remote.c | 2 ++
2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 67a74cc..3d39080 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2128,7 +2128,6 @@ start_remote (int from_tty)
{
struct inferior *inferior;

- init_wait_for_inferior ();
inferior = current_inferior ();
inferior->stop_soon = STOP_QUIETLY_REMOTE;

diff --git a/gdb/remote.c b/gdb/remote.c
index 50e671d..559039d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3422,6 +3422,8 @@ remote_start_remote (struct ui_out *uiout, void *opaque)
/* Always add the main thread. */
add_thread_silent (inferior_ptid);

+ init_wait_for_inferior ();
+
get_offsets (); /* Get text, data & bss offsets. */

/* If we could not find a description using qXfer, and we know
--
1.7.0.4