RFA: attach to a PID using a different exec

RFA: attach to a PID using a different exec

[Tom Tromey]

Sometimes I will start gdb like "gdb /some/path/to/gdb" and then
"attach PID" -- where PID is actually running "/some/other/gdb".
That is, the current exec file and the attached process disagree.

Right now this usually yields an error along the lines of:
"Cannot access memory at address 0x2e".

This patch changes gdb to look at the target's notion of the exec
file, and then switch to it if it differs from the current exec.

I looked a little and didn't see an existing "are these two files 'the
same'" predicate in gdb, so I just compare the real paths.

Built and regtested on x86-64 (compile farm).
New test included -- it fails before the patch and passes after it.

Please review.

thanks,
Tom

2008-11-13  Tom Tromey  <tromey@redhat.com>

	* infcmd.c (attach_command_post_wait): Compare current exec file
	with target's exec file.

2008-11-13  Tom Tromey  <tromey@redhat.com>

	* gdb.base/attach.exp (do_attach_tests): Test attaching to a PID
	using a different exec file.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index b3af31f..2efdd14 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2114,37 +2114,63 @@ proceed_after_attach (int pid)
 static void
 attach_command_post_wait (char *args, int from_tty, int async_exec)
 {
-  char *exec_file;
-  char *full_exec_path = NULL;
+  char *exec_file, *targ_exec_file;
+  char *full_targ_exec_path = NULL;
   struct inferior *inferior;
+  int must_attach;
 
   inferior = current_inferior ();
   inferior->stop_soon = NO_STOP_QUIETLY;
 
-  /* If no exec file is yet known, try to determine it from the
-     process itself.  */
+  /* Figure out what to do about the exec.  There are four cases to
+     consider -- we may or may not have a current exec file, and the
+     target may or may not be able to determine the exec file of the
+     attached process.  */
   exec_file = (char *) get_exec_file (0);
-  if (!exec_file)
+
+  /* Get the target's notion of the current exec.  */
+  targ_exec_file = target_pid_to_exec_file (PIDGET (inferior_ptid));
+  if (targ_exec_file)
     {
-      exec_file = target_pid_to_exec_file (PIDGET (inferior_ptid));
-      if (exec_file)
+      /* It's possible we don't have a full path, but rather just a
+	 filename.  Some targets, such as HP-UX, don't provide the
+	 full path, sigh.
+
+	 Attempt to qualify the filename against the source path.
+	 (If that fails, we'll just fall back on the original
+	 filename.  Not much more we can do...)
+      */
+      if (!source_full_path_of (targ_exec_file, &full_targ_exec_path))
+	full_targ_exec_path = xstrdup (targ_exec_file);
+    }
+
+  must_attach = 0;
+  /* If the target knows the exec, check to see if it matches our
+     current exec.  */
+  if (full_targ_exec_path)
+    {
+      /* If we don't have a current exec, just go with what the target
+	 reported.  */
+      if (!exec_file)
+	must_attach = 1;
+      else
 	{
-	  /* It's possible we don't have a full path, but rather just a
-	     filename.  Some targets, such as HP-UX, don't provide the
-	     full path, sigh.
-
-	     Attempt to qualify the filename against the source path.
-	     (If that fails, we'll just fall back on the original
-	     filename.  Not much more we can do...)
-	   */
-	  if (!source_full_path_of (exec_file, &full_exec_path))
-	    full_exec_path = savestring (exec_file, strlen (exec_file));
-
-	  exec_file_attach (full_exec_path, from_tty);
-	  symbol_file_add_main (full_exec_path, from_tty);
+	  char *real_path = gdb_realpath (exec_file);
+	  char *targ_real_path = gdb_realpath (full_targ_exec_path);
+	  must_attach = strcmp (real_path, targ_real_path) != 0;
+	  if (!must_attach)
+	    xfree (full_targ_exec_path);
+	  xfree (real_path);
+	  xfree (targ_real_path);
 	}
     }
-  else
+
+  if (must_attach)
+    {
+      exec_file_attach (full_targ_exec_path, from_tty);
+      symbol_file_add_main (full_targ_exec_path, from_tty);
+    }
+  else if (exec_file)
     {
       reopen_exec_file ();
       reread_symbols ();
@@ -2163,7 +2189,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
       /* The user requested an `attach&', so be sure to leave threads
 	 that didn't get a signal running.  */
 
-      /* Immediatelly resume all suspended threads of this inferior,
+      /* Immediately resume all suspended threads of this inferior,
 	 and this inferior only.  This should have no effect on
 	 already running threads.  If a thread has been stopped with a
 	 signal, leave it be.  */
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 4529715..fd59c0f 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -48,6 +48,7 @@ set srcfile2 ${testfile}2.c
 set binfile  ${objdir}/${subdir}/${testfile}
 set binfile2 ${objdir}/${subdir}/${testfile}2
 set escapedbinfile  [string_to_regexp ${objdir}/${subdir}/${testfile}]
+set escapedbinfile2 [string_to_regexp ${objdir}/${subdir}/${testfile}2]
 
 #execute_anywhere "rm -f ${binfile} ${binfile2}"
 remote_exec build "rm -f ${binfile} ${binfile2}"
@@ -75,8 +76,8 @@ if [get_compiler_info ${binfile}] {
 
 proc do_attach_tests {} {
     global gdb_prompt
-    global binfile
-    global escapedbinfile
+    global binfile binfile2
+    global escapedbinfile escapedbinfile2
     global srcfile
     global testfile
     global objdir
@@ -313,7 +314,53 @@ proc do_attach_tests {} {
 	"Attaching to process $testpid.*Reading symbols from $escapedbinfile.*main.*at .*" \
 	"attach when process' a.out not in cwd"
 
-    set test "after attach3, exit"
+    # Detach the process.
+   
+    gdb_test "detach" \
+	"Detaching from program: .*$escapedbinfile, process $testpid" \
+	"before attach4, detach"
+
+    # Wait a bit for gdb to finish detaching
+    
+    exec sleep 5
+
+    # Use symbols from one file, then attach to a process executing
+    # from another file.
+    gdb_test "file $binfile2" \
+	"Reading symbols from $escapedbinfile2\.\.\.*done." \
+	"attach4, select binfile2" \
+	"Load new symbol table from .*? .y or n. " \
+	"y"
+
+    set test "attach to PID using different exec"
+    send_gdb "attach $testpid\n"
+    gdb_expect {
+	-re "Attaching to program.*, process $testpid.*Load new symbol table from \"$escapedbinfile\".*y or n. $" {
+	    send_gdb "y\n"
+	    gdb_expect {
+		-re "main.*at.*$srcfile:.*$gdb_prompt $" {
+		    pass "$test"
+		}
+		-re "$gdb_prompt $" {
+		    fail "$test"
+		}
+		timeout {
+		    fail "$test (timeout)"
+		}
+	    }
+	}
+	-re "Attaching to program.*, process $testpid.*Cannot access memory at .*$" {
+	    fail "$test"
+	}
+	-re "Attaching to program.*, process $testpid.*$gdb_prompt $" {
+	    fail "$test"
+	}
+	timeout {
+	    fail "$test (timeout)"
+	}
+    }
+
+    set test "after attach4, exit"
     gdb_test_multiple "kill" "$test" {
 	-re "Kill the program being debugged.*y or n. $" {
 	    gdb_test "y" "" "$test"

Re: RFA: attach to a PID using a different exec

[Daniel Jacobowitz]

On Thu, Nov 13, 2008 at 12:18:20PM -0700, Tom Tromey wrote:
> Sometimes I will start gdb like "gdb /some/path/to/gdb" and then
> "attach PID" -- where PID is actually running "/some/other/gdb".
> That is, the current exec file and the attached process disagree.
> 
> Right now this usually yields an error along the lines of:
> "Cannot access memory at address 0x2e".
> 
> This patch changes gdb to look at the target's notion of the exec
> file, and then switch to it if it differs from the current exec.

This seems like a bad idea to me.  The canonical example is:

/usr/bin/prog.stripped &
gdb /home/drow/prog.debug $(pidof prog.stripped)

I don't want GDB to prompt my to switch to the stripped program...

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: attach to a PID using a different exec

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

>> This patch changes gdb to look at the target's notion of the exec
>> file, and then switch to it if it differs from the current exec.

Daniel> This seems like a bad idea to me.  The canonical example is:
Daniel> /usr/bin/prog.stripped &
Daniel> gdb /home/drow/prog.debug $(pidof prog.stripped)
Daniel> I don't want GDB to prompt my to switch to the stripped program...

Well, you can answer "no".  So while it is inconvenient, it seems
minor.  On the other hand, the current situation can leave one with a
nonsensical setup.  This bites me a lot more than I should probably
admit.

Tom

Re: RFA: attach to a PID using a different exec

[Stan Shebs]

Tom Tromey wrote:
>>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
>>>>>>             
>>> This patch changes gdb to look at the target's notion of the exec
>>> file, and then switch to it if it differs from the current exec.
>>>       
>
> Daniel> This seems like a bad idea to me.  The canonical example is:
> Daniel> /usr/bin/prog.stripped &
> Daniel> gdb /home/drow/prog.debug $(pidof prog.stripped)
> Daniel> I don't want GDB to prompt my to switch to the stripped program...
>
> Well, you can answer "no".  So while it is inconvenient, it seems
> minor.  On the other hand, the current situation can leave one with a
> nonsensical setup.  This bites me a lot more than I should probably
> admit.
>   
For multi-exec GDB, I added a set-exec command for just this situation - 
if you have multiple execs in the session, it's extremely difficult to 
tell which (if any) corresponds to the attached process. In practice so 
far, it's better than nothing, but we need more. We should have as many 
heuristics as possible, and ask for a confirmation too. (I wonder if 
comparing a handful of bytes, or sections, from each exec would be a 
useful check...)

Stan

Re: RFA: attach to a PID using a different exec

[Daniel Jacobowitz]

On Thu, Nov 13, 2008 at 12:09:03PM -0800, Stan Shebs wrote:
> For multi-exec GDB, I added a set-exec command for just this situation -  
> if you have multiple execs in the session, it's extremely difficult to  
> tell which (if any) corresponds to the attached process. In practice so  
> far, it's better than nothing, but we need more. We should have as many  
> heuristics as possible, and ask for a confirmation too. (I wonder if  
> comparing a handful of bytes, or sections, from each exec would be a  
> useful check...)

We do basically this for libraries, in solib-svr4.c - there's a few
very useful warnings there.  I wouldn't mind one here...

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: attach to a PID using a different exec

[Joel Brobecker]

> This seems like a bad idea to me.  The canonical example is:
> 
> /usr/bin/prog.stripped &
> gdb /home/drow/prog.debug $(pidof prog.stripped)
> 
> I don't want GDB to prompt my to switch to the stripped program...

In my case, it's usually a procedural mistake on my end. The typical
scenario is when I want to debugger GDB, so in one terminal, I start
"gdb my_process", and then on another terminal is start "gdb gdb"
followed by "attach inferior_gdb_pid". The problem is often that
the inferior GDB I started was the one in the path instead of
the one I just built. In all the cases when this occured, GDB switching
binaries from under me would not have been helpful. The error that
occurs is a little cryptic, so this can be improved, but the only
solution to my mistake is a restart of the inferior GDB.

-- 
Joel

Re: RFA: attach to a PID using a different exec

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

> On Thu, Nov 13, 2008 at 12:18:20PM -0700, Tom Tromey wrote:
>> Sometimes I will start gdb like "gdb /some/path/to/gdb" and then
>> "attach PID" -- where PID is actually running "/some/other/gdb".
>> That is, the current exec file and the attached process disagree.
>> 
>> Right now this usually yields an error along the lines of:
>> "Cannot access memory at address 0x2e".
>> 
>> This patch changes gdb to look at the target's notion of the exec
>> file, and then switch to it if it differs from the current exec.
>
> This seems like a bad idea to me.  The canonical example is:
>
> /usr/bin/prog.stripped &
> gdb /home/drow/prog.debug $(pidof prog.stripped)
>
> I don't want GDB to prompt my to switch to the stripped program...

I think a warning would still be helpful, similar to what you get when
you load a core that does not match.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: RFA: attach to a PID using a different exec

[Tom Tromey]

>>>>> "Andreas" == Andreas Schwab <schwab@suse.de> writes:

Daniel> I don't want GDB to prompt my to switch to the stripped
Daniel> program...

Andreas> I think a warning would still be helpful, similar to what you
Andreas> get when you load a core that does not match.

I did consider a warning.

My patch was tailored to my use case, where basically I made an early
mistake in selecting the exec file.  Suppose in this situation that
gdb warns about the non-matching exec.  After that, gdb is left in an
un-useful state, and furthermore the user has to resort to
cut-and-paste (or a lot of typing) to get back to the exec file that
gdb already knows.  That seems unfriendly.

I did not think of either Daniel's or Joel's use cases.

I would classify Daniel's as advanced usage -- but again, perhaps
tinged by my experience.  That is, I have never once done what he
describes, but then I almost always have separate debug info available
for those few stripped programs I must debug.

Joel's is an interesting case of making the "opposite" mistake
(starting the wrong program), but I think it doesn't really have an
effect on the outcome of this discussion (either thing gdb does will
yield the right answer when he restarts the inferior and reattaches).


Another oddity in this area, btw, is that at least the linux native
target gives a funny report if the attached process' exec file has
been deleted.


I don't like to add new options, in general, but I would add one for
this case if that would help.

Tom

Re: RFA: attach to a PID using a different exec

[Michael Snyder]

No way should we do this without asking.
It's just as possible that the first file was the one
the user wanted, and the one attached to was a mistake.

Tom Tromey wrote:
> Sometimes I will start gdb like "gdb /some/path/to/gdb" and then
> "attach PID" -- where PID is actually running "/some/other/gdb".
> That is, the current exec file and the attached process disagree.
> 
> Right now this usually yields an error along the lines of:
> "Cannot access memory at address 0x2e".
> 
> This patch changes gdb to look at the target's notion of the exec
> file, and then switch to it if it differs from the current exec.
> 
> I looked a little and didn't see an existing "are these two files 'the
> same'" predicate in gdb, so I just compare the real paths.
> 
> Built and regtested on x86-64 (compile farm).
> New test included -- it fails before the patch and passes after it.
> 
> Please review.
> 
> thanks,
> Tom
> 
> 2008-11-13  Tom Tromey  <tromey@redhat.com>
> 
> 	* infcmd.c (attach_command_post_wait): Compare current exec file
> 	with target's exec file.
> 
> 2008-11-13  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/attach.exp (do_attach_tests): Test attaching to a PID
> 	using a different exec file.
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index b3af31f..2efdd14 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -2114,37 +2114,63 @@ proceed_after_attach (int pid)
>  static void
>  attach_command_post_wait (char *args, int from_tty, int async_exec)
>  {
> -  char *exec_file;
> -  char *full_exec_path = NULL;
> +  char *exec_file, *targ_exec_file;
> +  char *full_targ_exec_path = NULL;
>    struct inferior *inferior;
> +  int must_attach;
>  
>    inferior = current_inferior ();
>    inferior->stop_soon = NO_STOP_QUIETLY;
>  
> -  /* If no exec file is yet known, try to determine it from the
> -     process itself.  */
> +  /* Figure out what to do about the exec.  There are four cases to
> +     consider -- we may or may not have a current exec file, and the
> +     target may or may not be able to determine the exec file of the
> +     attached process.  */
>    exec_file = (char *) get_exec_file (0);
> -  if (!exec_file)
> +
> +  /* Get the target's notion of the current exec.  */
> +  targ_exec_file = target_pid_to_exec_file (PIDGET (inferior_ptid));
> +  if (targ_exec_file)
>      {
> -      exec_file = target_pid_to_exec_file (PIDGET (inferior_ptid));
> -      if (exec_file)
> +      /* It's possible we don't have a full path, but rather just a
> +	 filename.  Some targets, such as HP-UX, don't provide the
> +	 full path, sigh.
> +
> +	 Attempt to qualify the filename against the source path.
> +	 (If that fails, we'll just fall back on the original
> +	 filename.  Not much more we can do...)
> +      */
> +      if (!source_full_path_of (targ_exec_file, &full_targ_exec_path))
> +	full_targ_exec_path = xstrdup (targ_exec_file);
> +    }
> +
> +  must_attach = 0;
> +  /* If the target knows the exec, check to see if it matches our
> +     current exec.  */
> +  if (full_targ_exec_path)
> +    {
> +      /* If we don't have a current exec, just go with what the target
> +	 reported.  */
> +      if (!exec_file)
> +	must_attach = 1;
> +      else
>  	{
> -	  /* It's possible we don't have a full path, but rather just a
> -	     filename.  Some targets, such as HP-UX, don't provide the
> -	     full path, sigh.
> -
> -	     Attempt to qualify the filename against the source path.
> -	     (If that fails, we'll just fall back on the original
> -	     filename.  Not much more we can do...)
> -	   */
> -	  if (!source_full_path_of (exec_file, &full_exec_path))
> -	    full_exec_path = savestring (exec_file, strlen (exec_file));
> -
> -	  exec_file_attach (full_exec_path, from_tty);
> -	  symbol_file_add_main (full_exec_path, from_tty);
> +	  char *real_path = gdb_realpath (exec_file);
> +	  char *targ_real_path = gdb_realpath (full_targ_exec_path);
> +	  must_attach = strcmp (real_path, targ_real_path) != 0;
> +	  if (!must_attach)
> +	    xfree (full_targ_exec_path);
> +	  xfree (real_path);
> +	  xfree (targ_real_path);
>  	}
>      }
> -  else
> +
> +  if (must_attach)
> +    {
> +      exec_file_attach (full_targ_exec_path, from_tty);
> +      symbol_file_add_main (full_targ_exec_path, from_tty);
> +    }
> +  else if (exec_file)
>      {
>        reopen_exec_file ();
>        reread_symbols ();
> @@ -2163,7 +2189,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
>        /* The user requested an `attach&', so be sure to leave threads
>  	 that didn't get a signal running.  */
>  
> -      /* Immediatelly resume all suspended threads of this inferior,
> +      /* Immediately resume all suspended threads of this inferior,
>  	 and this inferior only.  This should have no effect on
>  	 already running threads.  If a thread has been stopped with a
>  	 signal, leave it be.  */
> diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
> index 4529715..fd59c0f 100644
> --- a/gdb/testsuite/gdb.base/attach.exp
> +++ b/gdb/testsuite/gdb.base/attach.exp
> @@ -48,6 +48,7 @@ set srcfile2 ${testfile}2.c
>  set binfile  ${objdir}/${subdir}/${testfile}
>  set binfile2 ${objdir}/${subdir}/${testfile}2
>  set escapedbinfile  [string_to_regexp ${objdir}/${subdir}/${testfile}]
> +set escapedbinfile2 [string_to_regexp ${objdir}/${subdir}/${testfile}2]
>  
>  #execute_anywhere "rm -f ${binfile} ${binfile2}"
>  remote_exec build "rm -f ${binfile} ${binfile2}"
> @@ -75,8 +76,8 @@ if [get_compiler_info ${binfile}] {
>  
>  proc do_attach_tests {} {
>      global gdb_prompt
> -    global binfile
> -    global escapedbinfile
> +    global binfile binfile2
> +    global escapedbinfile escapedbinfile2
>      global srcfile
>      global testfile
>      global objdir
> @@ -313,7 +314,53 @@ proc do_attach_tests {} {
>  	"Attaching to process $testpid.*Reading symbols from $escapedbinfile.*main.*at .*" \
>  	"attach when process' a.out not in cwd"
>  
> -    set test "after attach3, exit"
> +    # Detach the process.
> +   
> +    gdb_test "detach" \
> +	"Detaching from program: .*$escapedbinfile, process $testpid" \
> +	"before attach4, detach"
> +
> +    # Wait a bit for gdb to finish detaching
> +    
> +    exec sleep 5
> +
> +    # Use symbols from one file, then attach to a process executing
> +    # from another file.
> +    gdb_test "file $binfile2" \
> +	"Reading symbols from $escapedbinfile2\.\.\.*done." \
> +	"attach4, select binfile2" \
> +	"Load new symbol table from .*? .y or n. " \
> +	"y"
> +
> +    set test "attach to PID using different exec"
> +    send_gdb "attach $testpid\n"
> +    gdb_expect {
> +	-re "Attaching to program.*, process $testpid.*Load new symbol table from \"$escapedbinfile\".*y or n. $" {
> +	    send_gdb "y\n"
> +	    gdb_expect {
> +		-re "main.*at.*$srcfile:.*$gdb_prompt $" {
> +		    pass "$test"
> +		}
> +		-re "$gdb_prompt $" {
> +		    fail "$test"
> +		}
> +		timeout {
> +		    fail "$test (timeout)"
> +		}
> +	    }
> +	}
> +	-re "Attaching to program.*, process $testpid.*Cannot access memory at .*$" {
> +	    fail "$test"
> +	}
> +	-re "Attaching to program.*, process $testpid.*$gdb_prompt $" {
> +	    fail "$test"
> +	}
> +	timeout {
> +	    fail "$test (timeout)"
> +	}
> +    }
> +
> +    set test "after attach4, exit"
>      gdb_test_multiple "kill" "$test" {
>  	-re "Kill the program being debugged.*y or n. $" {
>  	    gdb_test "y" "" "$test"

Re: RFA: attach to a PID using a different exec

[Tom Tromey]

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> No way should we do this without asking.
Michael> It's just as possible that the first file was the one
Michael> the user wanted, and the one attached to was a mistake.

It does ask, at least if the current exec has debug symbols:

(gdb) attach 16970
Attaching to program: /home/tromey/gnu/git-gdb/build/gdb/gdb, process 16970
Load new symbol table from "/home/tromey/gnu/archer/build/gdb/gdb"? (y or n) 

I suppose it could ask something more specific, though.

Tom