Fix execl.exp sporadic failures

Pedro Alves
2008-07-03T00:43:28Z

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

Sporadically the new execl.exp test fails with:

Executing new program: /home/pedro/gdb/execl_hunt/build32/gdb/testsuite/gdb.threads/execl1
Error in re-setting breakpoint 2: No source file named ../../../src/gdb/testsuite/gdb.threads/execl.c.
warning: Cannot initialize thread debugging library: versions of libpthread and libthread_db do not match
/home/pedro/gdb/execl_hunt/build32/gdb/testsuite/gdb.threads/execl1: error while loading shared libraries: libc.so.6:
failed to map segment from shared object: Cannot allocate memory

Program exited with code 0177.
(gdb) FAIL: gdb.threads/execl.exp: continue across exec (the program exited)

I traced to problem into the fact that when thread_db is loaded, GDB
will delete a breakpoint that was inserted in the old image from the
new image, which is a bad thing. The breakpoint shadows of the old
image are meaningless for the new image.

There's a call to mark_breakpoints_out in update_breakpoints_after_exec, which is
called by follow_exec, which in turn is called by handle_inferior_event on
a TARGET_WAITKIND_EXECD, that takes care of marking all breakpoints as not
inserted, to present this case.

The problem starts here:

static ptid_t
thread_db_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
{
ptid = target_beneath->to_wait (ptid, ourstatus);

(...)

if (ourstatus->kind == TARGET_WAITKIND_EXECD)
{
remove_thread_event_breakpoints (); <<<<<
unpush_target (&thread_db_ops);
using_thread_db = 0;

return ptid;
}

remove_thread_event_breakpoints calls delete_breakpoint,
and this happens before reaching handle_inferior_event, hence at this
point, the breakpoints that were inserted in the old image are still
mark as such, which triggers the breakpoint shadows to be inserted
into the new exec image.

It is my opition, that defering the mark_breakpoints_out call to so
late out of the target is not safe. It is better to calls it as
soon as we detect that the inferior execed.

That is what the attached patch does.

Tested on x86_64-unknown-linux-gnu. I no longer get sporadic
failures on execl.exp. They were much easier to trigger with
-m32 for some reason).

breakpoints always-inserted mode requires yet another fix,
which I'll post in a sec.

--
Pedro Alves

[-- Attachment #2: mark_breakpoints_out_sooner.diff --]
[-- Type: text/x-diff, Size: 4502 bytes --]

2008-07-03 Pedro Alves <pedro@codesourcery.com>

* breakpoint.c (mark_breakpoints_out): Make public.
(update_breakpoints_after_exec): Don't call mark_breakpoints_out
here. Update comment.
* breakpoint.h (mark_breakpoints_out): Declare.

* linux-nat.c (linux_handle_extended_wait): On
TARGET_WAITKIND_EXECD, call mark_breakpoints_out.
* inf-ttrace.c (inf_ttrace_wait): Likewise.

---
gdb/breakpoint.c | 21 ++++++++++++++-------
gdb/breakpoint.h | 3 +++
gdb/inf-ttrace.c | 6 ++++++
gdb/linux-nat.c | 10 ++++++++++
4 files changed, 33 insertions(+), 7 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2008-07-03 01:24:21.000000000 +0100
+++ src/gdb/breakpoint.c 2008-07-03 01:41:11.000000000 +0100
@@ -188,8 +188,6 @@ static int single_step_breakpoint_insert

static void free_bp_location (struct bp_location *loc);

-static void mark_breakpoints_out (void);
-
static struct bp_location *
allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type);

@@ -1445,10 +1443,19 @@ update_breakpoints_after_exec (void)
struct breakpoint *temp;
struct cleanup *cleanup;

- /* Doing this first prevents the badness of having delete_breakpoint()
- write a breakpoint's current "shadow contents" to lift the bp. That
- shadow is NOT valid after an exec()! */
- mark_breakpoints_out ();
+ /* There used to be a call to mark_breakpoints_out here with the
+ following comment:
+
+ Doing this first prevents the badness of having
+ delete_breakpoint() write a breakpoint's current "shadow
+ contents" to lift the bp. That shadow is NOT valid after an
+ exec()!
+
+ The concern is valid, but it was found that there are logical
+ places to delete breakpoints after detecting an exec and before
+ reaching here. The call has since moved closer to where the each
+ target detects an exec. */
+

/* The binary we used to debug is now gone, and we're updating
breakpoints for the new binary. Until we're done, we should not
@@ -1699,7 +1706,7 @@ remove_breakpoint (struct bp_location *b

/* Clear the "inserted" flag in all breakpoints. */

-static void
+void
mark_breakpoints_out (void)
{
struct bp_location *bpt;
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h 2008-07-03 01:24:21.000000000 +0100
+++ src/gdb/breakpoint.h 2008-07-03 01:40:14.000000000 +0100
@@ -826,6 +826,9 @@ extern void disable_breakpoint (struct b

extern void enable_breakpoint (struct breakpoint *);

+/* Clear the "inserted" flag in all breakpoints. */
+extern void mark_breakpoints_out (void);
+
extern void make_breakpoint_permanent (struct breakpoint *);

extern struct breakpoint *create_solib_event_breakpoint (CORE_ADDR);
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2008-07-03 01:24:21.000000000 +0100
+++ src/gdb/linux-nat.c 2008-07-03 01:40:14.000000000 +0100
@@ -1758,6 +1758,16 @@ linux_handle_extended_wait (struct lwp_i
linux_parent_pid = 0;
}

+ /* At this point, all inserted breakpoints are gone. Doing this
+ as soon as we detect an exec prevents the badness of deleting
+ a breakpoint writing the current "shadow contents" to lift
+ the bp. That shadow is NOT valid after an exec.
+
+ Note that we have to do this after the detach_breakpoints
+ call above, otherwise breakpoints wouldn't be lifted from the
+ parent on a vfork, because detach_breakpoints would think
+ that breakpoints are not inserted. */
+ mark_breakpoints_out ();
return 0;
}

Index: src/gdb/inf-ttrace.c
===================================================================
--- src.orig/gdb/inf-ttrace.c 2008-07-03 01:24:21.000000000 +0100
+++ src/gdb/inf-ttrace.c 2008-07-03 01:40:14.000000000 +0100