From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4979 invoked by alias); 3 Jul 2008 00:43:28 -0000 Received: (qmail 4969 invoked by uid 22791); 3 Jul 2008 00:43:27 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 03 Jul 2008 00:43:10 +0000 Received: (qmail 19818 invoked from network); 3 Jul 2008 00:43:08 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Jul 2008 00:43:08 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Fix execl.exp sporadic failures Date: Thu, 03 Jul 2008 00:43:00 -0000 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_cCCbIqqMhwlpOsd" Message-Id: <200807030143.08682.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-07/txt/msg00025.txt.bz2 --Boundary-00=_cCCbIqqMhwlpOsd Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 2275 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 --Boundary-00=_cCCbIqqMhwlpOsd Content-Type: text/x-diff; charset="us-ascii"; name="mark_breakpoints_out_sooner.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="mark_breakpoints_out_sooner.diff" Content-length: 4502 2008-07-03 Pedro Alves * 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: --Boundary-00=_cCCbIqqMhwlpOsd--