From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9929 invoked by alias); 14 Aug 2006 15:07:30 -0000 Received: (qmail 9464 invoked by uid 22791); 14 Aug 2006 15:07:26 -0000 X-Spam-Check-By: sourceware.org Received: from ip-160-218-142-157.eurotel.cz (HELO host0.dyn.jankratochvil.net) (160.218.142.157) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 14 Aug 2006 16:07:18 +0100 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.13.7/8.13.7) with ESMTP id k7EF6UIY003585; Mon, 14 Aug 2006 17:06:30 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.13.7/8.13.7/Submit) id k7EF6SXR003584; Mon, 14 Aug 2006 17:06:28 +0200 Date: Mon, 14 Aug 2006 15:07:00 -0000 From: Jan Kratochvil To: gdb@sourceware.org Cc: Daniel Jacobowitz Subject: Debugging through exec() (Linux MAY_FOLLOW_EXEC) Message-ID: <20060814150628.GA24544@host0.dyn.jankratochvil.net> References: <20060614142552.GA15021@nevyn.them.org> <20060615203519.GA9603@host0.dyn.jankratochvil.net> <20060721181556.GA9150@lace.redhat.com> <20060721184421.GA22820@nevyn.them.org> <20060722123102.GA1936@lace.redhat.com> <20060724190332.GA13612@nevyn.them.org> <20060729185317.GA16200@host0.dyn.jankratochvil.net> <200607312038.k6VKchKj018729@elgar.sibelius.xs4all.nl> <20060805164144.GA23819@host0.dyn.jankratochvil.net> <20060808160113.GC21032@nevyn.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="SLDf9lqlvOQaIe6s" Content-Disposition: inline In-Reply-To: <20060808160113.GC21032@nevyn.them.org> User-Agent: Mutt/1.4.2.1i X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2006-08/txt/msg00115.txt.bz2 --SLDf9lqlvOQaIe6s Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 4682 Hi, please review the proposed functionality to be able to debug though exec(). In fact I am not aware of any change until you type "catch exec". Daniel Jacobowitz has objections to the correctness of the patch. Some demo of the attached patch functionality: (gdb) tcatch exec Catchpoint 1 (exec) (gdb) run Starting program: /tmp/execve Executing new program: /tmp/hello [Switching to process 1588] ### Notice here the symbol is no longer resolved as gdb still did not detect ### right after exec() that it has shared library loaded. 0x44031840 in ?? () (gdb) tbreak main Breakpoint 2 at 0x8048395: file hello.c, line 6. (gdb) continue Continuing. [Switching to process 1588] main () at hello.c:6 6 puts("hello world"); (gdb) continue Continuing. hello world Program exited normally. (gdb) run ### New messages notifying the loaded executable changed. Restoring the program name before exec(). Restoring the symbol table before exec(). Starting program: /tmp/execve Executing new program: /tmp/hello hello world Program exited normally. (gdb) Original behavior was: (gdb) tcatch exec Catchpoint 1 (exec) (gdb) run Starting program: /tmp/execve ### Symbol is resolved here but the shared library is in fact no longer valid. 0x44031840 in _start () from /lib/ld-linux.so.2 (gdb) tbreak main ### Break address/sourcefile is stale here and thus disfunctional. Breakpoint 2 at 0x80483b4: file execve.c, line 7. (gdb) continue Continuing. hello world Program exited normally. (gdb) Please read the mails from Daniel Jacobowitz below: On Tue, 08 Aug 2006 18:01:13 +0200, Daniel Jacobowitz wrote: > On Sat, Aug 05, 2006 at 06:41:44PM +0200, Jan Kratochvil wrote: > > Hi Mark, > > > > On Mon, 31 Jul 2006 22:38:43 +0200, Mark Kettenis wrote: > > ... > > > That WNOHANG is wrong; > > > > In fact yes, the patch is more correct without that WNOHANG hack there. > > > > > > 2006-07-29 Jan Kratochvil > > > > * inf-ptrace.c (inf_ptrace_mourn_inferior): waitpid(2) only if there > > is valid inferior_ptid to wait for. > > * linux-fork.c (linux_fork_mourn_inferior): Ditto. > > * infrun.c (follow_exec): Unconditionally enabled by MAY_FOLLOW_EXEC. > > Provide restoration of exec_bfd and symfile_objfile for any new "run". > > * linux-thread-db.c (thread_db_wait): Handle TARGET_WAITKIND_EXECD. > > * linux-thread-db.c (thread_db_mourn_inferior): Turn off threading. > > * foll-exec.exp: Uncoditionally enabled for all platforms. > > Relaxed regex to apply besides HP-UX also for GNU/Linux backtrace. > > Sorry, but I can't approve this patch. I think someone needs to > discuss the concept and interface on gdb@ first. > > You're using make_run_cleanup to restore the inferior file. This means > it happens before "run". Any time we choose to change the file > silently will be surprising (especially to front ends like Eclipse) but > that time will be pretty surprising to the user too: > > ... info files shows first prog > (gdb) catch exec > (gdb) run > ... info files shows second prog > (gdb) continue > ... exits. > ... info files shows second prog > (gdb) run > ... starts first prog! > > I also think that changing inferior_ptid before mourning is pretty > strange. The whole way follow_exec works is a hack and not > particularly well defined. > > This is a hard problem to solve; I think a half-finished solution would > be worse than leaving it unsolved. > > -- > Daniel Jacobowitz > CodeSourcery Daniel's earlier mail: On Mon, 24 Jul 2006 21:03:32 +0200, Daniel Jacobowitz wrote: > Jan Kratochvil wrote: > > On Fri, 21 Jul 2006 20:44:21 +0200, Daniel Jacobowitz wrote: > > ... > > > Turning on MAY_FOLLOW_EXEC is not a good idea. No one really knows how > > > that behavior works, a lot of it doesn't, and the way it implicitly > > > changes the symbol file is very disorienting. Please don't mix it up > > > with the fix for your current bug. > > > > Still I am for MAY_FOLLOW_EXEC as it improves the user experience and makes > > debugging of exec()ing processes much more convenient - without having to find > > out how each child gets executed and replay such conditions by hand. > > > > As gdb-6.5 has been released and the MAY_FOLLOW_EXEC feature IMO generally > > works for GNU/Linux - isn't appropriate to enable it and settle it down? > > I would even like to fix any issues possibly roaring its head. > > Does anyone else have an opinion on this? I'm starting to think you're > right - we should turn it on, invite people to use it, and see what > happens. > > The reason I find it so disorienting is this: [ silent changing of the loaded exec + symbol file ] Thanks, Jan Kratochvil --SLDf9lqlvOQaIe6s Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="gdb-6.5-follow-exec-v3.patch" Content-length: 7662 Index: gdb/inf-ptrace.c =================================================================== RCS file: /cvs/src/src/gdb/inf-ptrace.c,v retrieving revision 1.32 diff -u -p -r1.32 inf-ptrace.c --- gdb/inf-ptrace.c 24 Jan 2006 22:34:34 -0000 1.32 +++ gdb/inf-ptrace.c 14 Aug 2006 15:04:53 -0000 @@ -167,7 +167,8 @@ inf_ptrace_mourn_inferior (void) Do not check whether this succeeds though, since we may be dealing with a process that we attached to. Such a process will only report its exit status to its original parent. */ - waitpid (ptid_get_pid (inferior_ptid), &status, 0); + if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution) + waitpid (ptid_get_pid (inferior_ptid), &status, 0); unpush_target (ptrace_ops_hack); generic_mourn_inferior (); Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.213 diff -u -p -r1.213 infrun.c --- gdb/infrun.c 22 Jul 2006 14:48:03 -0000 1.213 +++ gdb/infrun.c 14 Aug 2006 15:04:56 -0000 @@ -47,6 +47,7 @@ #include "language.h" #include "solib.h" #include "main.h" +#include "objfiles.h" #include "gdb_assert.h" #include "mi/mi-common.h" @@ -109,10 +110,10 @@ int sync_execution = 0; static ptid_t previous_inferior_ptid; /* This is true for configurations that may follow through execl() and - similar functions. At present this is only true for HP-UX native. */ + similar functions. */ #ifndef MAY_FOLLOW_EXEC -#define MAY_FOLLOW_EXEC (0) +#define MAY_FOLLOW_EXEC (1) #endif static int may_follow_exec = MAY_FOLLOW_EXEC; @@ -375,6 +376,45 @@ follow_inferior_reset_breakpoints (void) insert_breakpoints (); } +static void +follow_exec_restore_execfile (void *filename_new_untyped) +{ + char *filename_new = filename_new_untyped; + + /* filename_new == NULL is not expected. */ + if (filename_new == NULL && exec_bfd != NULL) + exec_file_clear (0); + /* exec_bfd == NULL is not expected. */ + if (filename_new != NULL && + (exec_bfd == NULL || strcmp (get_exec_file (0), filename_new))) + { + /* The filename will be printed already below: Starting program: %s */ + printf_unfiltered (_("Restoring the program name before exec().\n")); + exec_file_attach (filename_new, 0); + } + + free (filename_new); +} + +static void +follow_exec_restore_symfile (void *filename_new_untyped) +{ + char *filename_new = filename_new_untyped; + + /* symfile_objfile == NULL is not expected. */ + if (filename_new == NULL && symfile_objfile != NULL) + symbol_file_clear (0); + if (filename_new != NULL && (symfile_objfile == NULL + || strcmp (symfile_objfile->name, filename_new))) + { + /* The filename will be printed already below: Starting program: %s */ + printf_unfiltered (_("Restoring the symbol table before exec().\n")); + symbol_file_add_main (filename_new, 0); + } + + free (filename_new); +} + /* EXECD_PATHNAME is assumed to be non-NULL. */ static void @@ -382,6 +422,7 @@ follow_exec (int pid, char *execd_pathna { int saved_pid = pid; struct target_ops *tgt; + struct objfile *objfile, *objfile_temp; if (!may_follow_exec) return; @@ -427,6 +468,32 @@ follow_exec (int pid, char *execd_pathna error (_("Could find run target to save before following exec")); gdb_flush (gdb_stdout); + + /* During the common "run" bare command we should run again the original + * program spawning us. Stacking order is correct this way. */ + make_run_cleanup (follow_exec_restore_symfile, (!symfile_objfile ? NULL : + xstrdup (symfile_objfile->name))); + make_run_cleanup (follow_exec_restore_execfile, + (!exec_bfd ? NULL : xstrdup (exec_bfd->filename))); + + /* As symbol_file_add_main()->thread_db_new_objfile()->check_for_thread_db() + * would fine already loaded libpthread.so while the threading structures + * would not be yet initialized for this early inferior. + * Call before target_mourn_inferior() as it will breakpoint_re_set(). */ +#ifdef CLEAR_SOLIB + CLEAR_SOLIB (); +#else + clear_solib (); +#endif + /* Do not: symbol_file_clear()->clear_symtab_users()->breakpoint_re_set(). */ + ALL_OBJFILES_SAFE (objfile, objfile_temp) + { + free_objfile (objfile); + } + symfile_objfile = NULL; + + /* Avoid stucked waitpid(2) as PID inferior_ptid is still running. */ + inferior_ptid = null_ptid; target_mourn_inferior (); inferior_ptid = pid_to_ptid (saved_pid); /* Because mourn_inferior resets inferior_ptid. */ Index: gdb/linux-fork.c =================================================================== RCS file: /cvs/src/src/gdb/linux-fork.c,v retrieving revision 1.7 diff -u -p -r1.7 linux-fork.c --- gdb/linux-fork.c 27 Apr 2006 23:03:41 -0000 1.7 +++ gdb/linux-fork.c 14 Aug 2006 15:04:56 -0000 @@ -366,7 +366,8 @@ linux_fork_mourn_inferior (void) only report its exit status to its original parent. */ int status; - waitpid (ptid_get_pid (inferior_ptid), &status, 0); + if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution) + waitpid (ptid_get_pid (inferior_ptid), &status, 0); /* OK, presumably inferior_ptid is the one who has exited. We need to delete that one from the fork_list, and switch Index: gdb/linux-thread-db.c =================================================================== RCS file: /cvs/src/src/gdb/linux-thread-db.c,v retrieving revision 1.19 diff -u -p -r1.19 linux-thread-db.c --- gdb/linux-thread-db.c 2 Aug 2006 10:24:00 -0000 1.19 +++ gdb/linux-thread-db.c 14 Aug 2006 15:04:57 -0000 @@ -884,6 +884,10 @@ thread_db_wait (ptid_t ptid, struct targ return pid_to_ptid (GET_PID (ptid)); } + /* Threading structures got reset. Return as nonthreaded. */ + if (ourstatus->kind == TARGET_WAITKIND_EXECD) + return pid_to_ptid (GET_PID (ptid)); + if (ourstatus->kind == TARGET_WAITKIND_STOPPED && ourstatus->value.sig == TARGET_SIGNAL_TRAP) /* Check for a thread event. */ @@ -975,6 +979,9 @@ thread_db_mourn_inferior (void) the inferior, so that we don't try to uninsert them. */ remove_thread_event_breakpoints (); + /* Destroy thread info; it's no longer valid. */ + init_thread_list (); + /* Detach thread_db target ops. */ unpush_target (&thread_db_ops); using_thread_db = 0; Index: gdb/testsuite/gdb.base/foll-exec.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/foll-exec.exp,v retrieving revision 1.3 diff -u -p -r1.3 foll-exec.exp --- gdb/testsuite/gdb.base/foll-exec.exp 10 Aug 2006 05:27:20 -0000 1.3 +++ gdb/testsuite/gdb.base/foll-exec.exp 14 Aug 2006 15:04:58 -0000 @@ -47,12 +47,6 @@ if { [gdb_compile "${srcdir}/${subdir}/ } -# Until "catch exec" is implemented on other targets... -# -if ![istarget "hppa*-hp-hpux*"] then { - continue -} - proc zap_session {} { global gdb_prompt global binfile @@ -214,7 +208,9 @@ proc do_exec_tests {} { setup_xfail hppa2.0w-hp-hpux* CLLbs16760 send_gdb "continue\n" gdb_expect { - -re ".*Executing new program:.*${testfile2}.*Catchpoint .*(exec\'d .*${testfile2}).*in .START..*$gdb_prompt $"\ + # It is OS dependent and no symbols may be found, GNU/Linux has "_start" + # while HP-UX has " in .START..*$gdb_prompt" etc. + -re ".*Executing new program:.*${testfile2}.*Catchpoint .*(exec\'d .*${testfile2}).*in .*$gdb_prompt $"\ {pass "hit catch exec"} -re "$gdb_prompt $" {fail "hit catch exec"} timeout {fail "(timeout) hit catch exec"} --SLDf9lqlvOQaIe6s--