From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7875 invoked by alias); 3 Jul 2015 15:44:57 -0000 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 Received: (qmail 7865 invoked by uid 89); 3 Jul 2015 15:44:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 03 Jul 2015 15:44:55 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 0B980923AE; Fri, 3 Jul 2015 15:44:54 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t63Fip4J022095; Fri, 3 Jul 2015 11:44:51 -0400 Message-ID: <5596ADF2.7060602@redhat.com> Date: Fri, 03 Jul 2015 15:44:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Gary Benson , gdb-patches@sourceware.org CC: Joel Brobecker , Philippe Waroquiers , Doug Evans , Don Breazeal Subject: Re: [PATCH v3] Make only user-specified executable and symbol filenames sticky References: <1433677265.2933.20.camel@soleil> <1433754079-10395-1-git-send-email-gbenson@redhat.com> In-Reply-To: <1433754079-10395-1-git-send-email-gbenson@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-07/txt/msg00097.txt.bz2 Sorry for dropping the ball on this... Comments below. On 06/08/2015 10:01 AM, Gary Benson wrote: > > PR gdb/17626 > * progspace.h (struct program_space) > : New field. > : Likewise. > (symfile_objfile_is_user_supplied): New macro. > * exec.h (exec_file_is_user_supplied): Likewise. > * exec.c (exec_close): Clear exec_file_is_user_supplied. > (exec_file_locate_attach): Remove get_exec_file check. > Do not replace user-supplied executable or symbol files. > Warn if user-supplied executable or symbol files do not > match discovered file. > (exec_file_command): Set exec_file_is_user_supplied. > * symfile.c (symbol_file_clear): Clear > symfile_objfile_is_user_supplied. > (symbol_file_command): Set symfile_objfile_is_user_supplied. > * inferior.c (add_inferior_command): Set > exec_file_is_user_supplied and symfile_objfile_is_user_supplied. > * main.c (captured_main): Likewise. > * infcmd.c (attach_command_post_wait): Always call > exec_file_locate_attach. Only call reopen_exec_file and > reread_symbols if exec_file_is_user_supplied. > * remote.c (remote_add_inferior): Remove get_exec_file check > around exec_file_locate_attach. > --- > gdb/ChangeLog | 26 ++++++++++++++++++++++++++ > gdb/exec.c | 30 +++++++++++++++++++++++------- > gdb/exec.h | 2 ++ > gdb/infcmd.c | 13 ++++++++----- > gdb/inferior.c | 3 +++ > gdb/main.c | 24 ++++++++++++++++-------- > gdb/progspace.h | 17 +++++++++++++++++ > gdb/remote.c | 9 ++++++--- > gdb/symfile.c | 3 +++ > 9 files changed, 104 insertions(+), 23 deletions(-) > > diff --git a/gdb/exec.c b/gdb/exec.c > index 8a4ab6f..11e5db0 100644 > --- a/gdb/exec.c > +++ b/gdb/exec.c > @@ -102,6 +102,7 @@ exec_close (void) > > xfree (exec_filename); > exec_filename = NULL; > + exec_file_is_user_supplied = 0; > } > } > > @@ -142,11 +143,6 @@ exec_file_locate_attach (int pid, int from_tty) > { > char *exec_file, *full_exec_path = NULL; > > - /* Do nothing if we already have an executable filename. */ > - exec_file = (char *) get_exec_file (0); > - if (exec_file != NULL) > - return; > - > /* Try to determine a filename from the process itself. */ > exec_file = target_pid_to_exec_file (pid); > if (exec_file == NULL) > @@ -171,8 +167,27 @@ exec_file_locate_attach (int pid, int from_tty) > full_exec_path = xstrdup (exec_file); > } > > - exec_file_attach (full_exec_path, from_tty); > - symbol_file_add_main (full_exec_path, from_tty); > + if (exec_file_is_user_supplied) > + { > + if (strcmp (full_exec_path, exec_filename) != 0) > + warning (_("Process %d has executable file %s," > + " but executable file is currently set to %s"), > + pid, full_exec_path, exec_filename); > + } > + else > + exec_file_attach (full_exec_path, from_tty); > + > + if (symfile_objfile_is_user_supplied) > + { > + const char *symbol_filename = objfile_filename (symfile_objfile); > + > + if (strcmp (full_exec_path, symbol_filename) != 0) > + warning (_("Process %d has symbol file %s," > + " but symbol file is currently set to %s"), > + pid, full_exec_path, symbol_filename); > + } > + else > + symbol_file_add_main (full_exec_path, from_tty); > } > This function is called while the passed in PID is not the current inferior. E.g., the remote_add_inferior path. Therefore seems to me that these symbol_file_add_main / exec_file_attach calls can change the symbols of the wrong inferior. (I still suspect that if we reverse the sense of the flag, then its management ends up being more centralized, as then the place that sets it is also the place that needs to check it, instead of doing that in multiple places. But, see below.) I also notice that reread_symbols has an exec_file_attach call which seems to me results in losing the is_user_supplied flag if the executable's timestamp changed, at least here: > + /* Attempt to open the file that was executed to create this > + inferior. If the user has explicitly specified executable > + and/or symbol files then warn the user if their choices do > + not match. Otherwise, set exec_file and symfile_objfile to > + the new file. */ > + exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); > + > + if (exec_file_is_user_supplied) > { > reopen_exec_file (); > reread_symbols (); I also notice that the clone-inferior command ends up with an exec_file_attach call in clone_program_space. That one should probably be setting the is_user_supplied flag too, I'd think. Or at least, copying it from the original. At this point, I'm wondering about adding a parameter to exec_file_attach to force considering (now and in future) the right value to put in the flag in each case. > @@ -2490,11 +2490,14 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) > inferior = current_inferior (); > inferior->control.stop_soon = NO_STOP_QUIETLY; > > - /* If no exec file is yet known, try to determine it from the > - process itself. */ > - if (get_exec_file (0) == NULL) > - exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); > - else > + /* Attempt to open the file that was executed to create this > + inferior. If the user has explicitly specified executable > + and/or symbol files then warn the user if their choices do > + not match. Otherwise, set exec_file and symfile_objfile to > + the new file. */ > + exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); > + > + if (exec_file_is_user_supplied) > { > reopen_exec_file (); > reread_symbols (); It seems to me that we should be able to move these reopen/reread calls inside exec_file_locate_attach. Thanks, Pedro Alves