This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Fix execl.exp sporadic failures
On Monday 07 July 2008 20:10:36, Daniel Jacobowitz wrote:
> On Thu, Jul 03, 2008 at 01:43:08AM +0100, Pedro Alves wrote:
> It's OK.
> On Thu, Jul 03, 2008 at 01:43:08AM +0100, Pedro Alves wrote:
> > + /* 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. */
> > +
>
> Please remove this comment, or write one that describes the current
> state (bonus points for an assertion). Comments that describe how GDB
> used to be grow more confusing with their age.
/me wants bonus points.
Attached is what I checked in then.
Thanks!
--
Pedro Alves
2008-07-08 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
@@ -904,6 +904,12 @@ inf_ttrace_wait (ptid_t ptid, struct tar
tts.tts_u.tts_exec.tts_pathlen, 0) == -1)
perror_with_name (("ttrace"));
ourstatus->value.execd_pathname[tts.tts_u.tts_exec.tts_pathlen] = 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. */
+ mark_breakpoints_out ();
break;
case TTEVT_EXIT: