[PATCH v2 0/2] Debugging without a symbol file

[PATCH v2 0/2] Debugging without a symbol file

[Luis Machado]

This is an updated version of the previous couple patches i sent.

Changes in v2 include fixes based on previous comments and improved testcase
logic.

Luis Machado (2):
  Debugging without a binary (regression)
  Test GDB connection to GDBserver with no symbol files

 gdb/exec.c                                         |  61 +++++++++++-
 .../gdb.server/connect-with-no-symbol-file.c       |  25 +++++
 .../gdb.server/connect-with-no-symbol-file.exp     | 102 +++++++++++++++++++++
 3 files changed, 186 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/connect-with-no-symbol-file.c
 create mode 100644 gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp

-- 
1.9.1

[PATCH v2 1/2] Debugging without a binary (regression)

[Luis Machado]

When we attempt to debug a process using GDBserver in standard remote mode
without a symbol file on GDB's end, we may run into an issue where GDB cuts
the connection attempt short due to an error. The error is caused by not
being able to open a symbol file, like so:

--

(gdb) set sysroot
(gdb) tar rem :2345
Remote debugging using :2345
/proc/23769/exe: Permission denied.
(gdb) i r
The program has no registers now.
(gdb)

It should've been like this:

(gdb) set sysroot
(gdb) tar rem :2345
Remote debugging using :2345
0xf7ddb2d0 in ?? ()
(gdb) i r
eax            0x0  0
ecx            0x0  0
edx            0x0  0
ebx            0x0  0
esp            0xffffdfa0 0xffffdfa0
ebp            0x0  0x0
esi            0x0  0
edi            0x0  0
eip            0xf7ddb2d0 0xf7ddb2d0
eflags         0x200  [ IF ]
cs             0x33 51
ss             0x2b 43
ds             0x0  0
es             0x0  0
fs             0x0  0
gs             0x0  0
(gdb)

This is caused by a couple of function calls within exec_file_locate_attach
that can potentially throw errors.

The following patch guards both exec_file_attach and symbol_file_add_main to
prevent the errors from disrupting the connection process.

There was also a case where native GDB tripped on this problem, but it was
mostly fixed by bf74e428bca61022bd5cdf6bf28789a184748b4d.

Regression-tested on x86-64/Ubuntu.

Changes in v2:

- We are using two TRY/CATCH blocks to guard both exec_file_attach and
symbol_file_add_main.
- Adjusted code to display only a single warning message if both TRY/CATCH
blocks throw with the same error message.
- I had to duplicate the exception message string because the second CATCH
block will overwrite the contents of the pointer we saved.

gdb/ChangeLog:

2016-04-12  Luis Machado  <lgustavo@codesourcery.com>

	* exec.c (exec_file_locate_attach): Guard a couple functions
	that can throw errors.
	(exception_print_same): New helper function.
---
 gdb/exec.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index a10ab9b..051a585 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -135,6 +135,25 @@ exec_file_clear (int from_tty)
     printf_unfiltered (_("No executable file now.\n"));
 }
 
+/* Returns non-zero if exceptions E1 and E2 are equal.  Returns zero
+   otherwise.  */
+
+static int
+exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
+{
+  const char *msg1 = e1.message;
+  const char *msg2 = e2.message;
+
+  if (msg1 == NULL)
+    msg1 = "";
+  if (msg2 == NULL)
+    msg2 = "";
+
+  return (e1.reason == e2.reason
+	  && e1.error == e2.error
+	  && strcmp (e1.message, e2.message) == 0);
+}
+
 /* See gdbcore.h.  */
 
 void
@@ -142,6 +161,7 @@ exec_file_locate_attach (int pid, int from_tty)
 {
   char *exec_file, *full_exec_path = NULL;
   struct cleanup *old_chain;
+  struct gdb_exception prev_err;
 
   /* Do nothing if we already have an executable filename.  */
   exec_file = (char *) get_exec_file (0);
@@ -182,9 +202,46 @@ exec_file_locate_attach (int pid, int from_tty)
 
   old_chain = make_cleanup (xfree, full_exec_path);
 
-  exec_file_attach (full_exec_path, from_tty);
-  symbol_file_add_main (full_exec_path, from_tty);
+  /* exec_file_attach and symbol_file_add_main may throw an error if the file
+     cannot be opened either locally or remotely.
+
+     This happens for example, when the file is first found in the local
+     sysroot (above), and then disappears (a TOCTOU race), or when it doesn't
+     exist in the target filesystem, or when the file does exist, but
+     is not readable.
+
+     Even without a symbol file, the remote-based debugging session should
+     continue normally instead of ending abruptly.  Hence we catch thrown
+     errors/exceptions in the following code.  */
+  TRY
+    {
+      exec_file_attach (full_exec_path, from_tty);
+    }
+  CATCH (err, RETURN_MASK_ERROR)
+    {
+      if (err.message != NULL)
+	warning ("%s", err.message);
+
+      prev_err = err;
+
+      /* Save message so it doesn't get trashed by the catch problem
+	 below.  */
+      prev_err.message = xstrdup (err.message);
+    }
+  END_CATCH
+
+  TRY
+    {
+      symbol_file_add_main (full_exec_path, from_tty);
+    }
+  CATCH (err, RETURN_MASK_ERROR)
+    {
+      if (!exception_print_same (prev_err, err))
+	warning ("%s", err.message);
+    }
+  END_CATCH
 
+  xfree ((void *) prev_err.message);
   do_cleanups (old_chain);
 }
 
-- 
1.9.1

Re: [PATCH v2 1/2] Debugging without a binary (regression)

[Pedro Alves]

On 04/12/2016 08:24 PM, Luis Machado wrote:
> When we attempt to debug a process using GDBserver in standard remote mode
> without a symbol file on GDB's end, we may run into an issue where GDB cuts
> the connection attempt short due to an error. The error is caused by not
> being able to open a symbol file, like so:
> 
> --
> 
> (gdb) set sysroot
> (gdb) tar rem :2345
> Remote debugging using :2345
> /proc/23769/exe: Permission denied.
> (gdb) i r
> The program has no registers now.
> (gdb)
> 
> It should've been like this:
> 
> (gdb) set sysroot
> (gdb) tar rem :2345
> Remote debugging using :2345
> 0xf7ddb2d0 in ?? ()

The warning output is missing here, right?

> (gdb) i r
> eax            0x0  0
> ecx            0x0  0
> edx            0x0  0
> ebx            0x0  0
> esp            0xffffdfa0 0xffffdfa0
> ebp            0x0  0x0
> esi            0x0  0
> edi            0x0  0
> eip            0xf7ddb2d0 0xf7ddb2d0
> eflags         0x200  [ IF ]
> cs             0x33 51
> ss             0x2b 43
> ds             0x0  0
> es             0x0  0
> fs             0x0  0
> gs             0x0  0
> (gdb)
> 

>  void
> @@ -142,6 +161,7 @@ exec_file_locate_attach (int pid, int from_tty)
>  {
>    char *exec_file, *full_exec_path = NULL;
>    struct cleanup *old_chain;
> +  struct gdb_exception prev_err;
>  
>    /* Do nothing if we already have an executable filename.  */
>    exec_file = (char *) get_exec_file (0);
> @@ -182,9 +202,46 @@ exec_file_locate_attach (int pid, int from_tty)
>  
>    old_chain = make_cleanup (xfree, full_exec_path);
>  
> -  exec_file_attach (full_exec_path, from_tty);
> -  symbol_file_add_main (full_exec_path, from_tty);
> +  /* exec_file_attach and symbol_file_add_main may throw an error if the file
> +     cannot be opened either locally or remotely.
> +
> +     This happens for example, when the file is first found in the local
> +     sysroot (above), and then disappears (a TOCTOU race), or when it doesn't
> +     exist in the target filesystem, or when the file does exist, but
> +     is not readable.
> +
> +     Even without a symbol file, the remote-based debugging session should
> +     continue normally instead of ending abruptly.  Hence we catch thrown
> +     errors/exceptions in the following code.  */
> +  TRY
> +    {
> +      exec_file_attach (full_exec_path, from_tty);
> +    }
> +  CATCH (err, RETURN_MASK_ERROR)
> +    {
> +      if (err.message != NULL)
> +	warning ("%s", err.message);
> +
> +      prev_err = err;
> +
> +      /* Save message so it doesn't get trashed by the catch problem

"the catch problem below" should be either the "the caught
problem below", or "the catch below", I think.

> +	 below.  */
> +      prev_err.message = xstrdup (err.message);
> +    }
> +  END_CATCH
> +
> +  TRY
> +    {
> +      symbol_file_add_main (full_exec_path, from_tty);
> +    }
> +  CATCH (err, RETURN_MASK_ERROR)
> +    {
> +      if (!exception_print_same (prev_err, err))
> +	warning ("%s", err.message);
> +    }
> +  END_CATCH
>  
> +  xfree ((void *) prev_err.message);

This will reference an uninitialized prev_err.message if the
first TRY doesn't throw.  Initialize it with:

 struct gdb_exception prev_err = exception_none;


Also, it will leak prev_err.message on Ctrl-C/QUIT, which is
not caught by RETURN_MASK_ERROR.  prev_err.message needs to be
freed with a cleanup.  Say:

  struct gdb_exception prev_err = exception_none;

  old_chain = make_cleanup (xfree, full_exec_path);

  make_cleanup (free_current_contents, &prev_err.message);

  TRY
    {
       ...
    }
  CATCH (err, RETURN_MASK_ERROR)
    {
      if (err.message != NULL)
	warning ("%s", err.message);

      prev_err = err;
      prev_err.message = xstrdup (err.message);
    }
   ...

  do_cleanups (old_chain);

Thanks,
Pedro Alves

Re: [PATCH v2 1/2] Debugging without a binary (regression)

[Luis Machado]

On 04/13/2016 10:00 AM, Pedro Alves wrote:
> On 04/12/2016 08:24 PM, Luis Machado wrote:
>> When we attempt to debug a process using GDBserver in standard remote mode
>> without a symbol file on GDB's end, we may run into an issue where GDB cuts
>> the connection attempt short due to an error. The error is caused by not
>> being able to open a symbol file, like so:
>>
>> --
>>
>> (gdb) set sysroot
>> (gdb) tar rem :2345
>> Remote debugging using :2345
>> /proc/23769/exe: Permission denied.
>> (gdb) i r
>> The program has no registers now.
>> (gdb)
>>
>> It should've been like this:
>>
>> (gdb) set sysroot
>> (gdb) tar rem :2345
>> Remote debugging using :2345
>> 0xf7ddb2d0 in ?? ()
>
> The warning output is missing here, right?
>

It is, because i did not update it after the patch. :-)

Should be fixed in v3.

>> (gdb) i r
>> eax            0x0  0
>> ecx            0x0  0
>> edx            0x0  0
>> ebx            0x0  0
>> esp            0xffffdfa0 0xffffdfa0
>> ebp            0x0  0x0
>> esi            0x0  0
>> edi            0x0  0
>> eip            0xf7ddb2d0 0xf7ddb2d0
>> eflags         0x200  [ IF ]
>> cs             0x33 51
>> ss             0x2b 43
>> ds             0x0  0
>> es             0x0  0
>> fs             0x0  0
>> gs             0x0  0
>> (gdb)
>>
>
>>   void
>> @@ -142,6 +161,7 @@ exec_file_locate_attach (int pid, int from_tty)
>>   {
>>     char *exec_file, *full_exec_path = NULL;
>>     struct cleanup *old_chain;
>> +  struct gdb_exception prev_err;
>>
>>     /* Do nothing if we already have an executable filename.  */
>>     exec_file = (char *) get_exec_file (0);
>> @@ -182,9 +202,46 @@ exec_file_locate_attach (int pid, int from_tty)
>>
>>     old_chain = make_cleanup (xfree, full_exec_path);
>>
>> -  exec_file_attach (full_exec_path, from_tty);
>> -  symbol_file_add_main (full_exec_path, from_tty);
>> +  /* exec_file_attach and symbol_file_add_main may throw an error if the file
>> +     cannot be opened either locally or remotely.
>> +
>> +     This happens for example, when the file is first found in the local
>> +     sysroot (above), and then disappears (a TOCTOU race), or when it doesn't
>> +     exist in the target filesystem, or when the file does exist, but
>> +     is not readable.
>> +
>> +     Even without a symbol file, the remote-based debugging session should
>> +     continue normally instead of ending abruptly.  Hence we catch thrown
>> +     errors/exceptions in the following code.  */
>> +  TRY
>> +    {
>> +      exec_file_attach (full_exec_path, from_tty);
>> +    }
>> +  CATCH (err, RETURN_MASK_ERROR)
>> +    {
>> +      if (err.message != NULL)
>> +	warning ("%s", err.message);
>> +
>> +      prev_err = err;
>> +
>> +      /* Save message so it doesn't get trashed by the catch problem
>
> "the catch problem below" should be either the "the caught
> problem below", or "the catch below", I think.
>

Yeah, i got things mixed up when coming up with it. "the catch below" is 
what i wanted to say.

>> +	 below.  */
>> +      prev_err.message = xstrdup (err.message);
>> +    }
>> +  END_CATCH
>> +
>> +  TRY
>> +    {
>> +      symbol_file_add_main (full_exec_path, from_tty);
>> +    }
>> +  CATCH (err, RETURN_MASK_ERROR)
>> +    {
>> +      if (!exception_print_same (prev_err, err))
>> +	warning ("%s", err.message);
>> +    }
>> +  END_CATCH
>>
>> +  xfree ((void *) prev_err.message);
>
> This will reference an uninitialized prev_err.message if the
> first TRY doesn't throw.  Initialize it with:
>
>   struct gdb_exception prev_err = exception_none;
>
>

Done.

> Also, it will leak prev_err.message on Ctrl-C/QUIT, which is
> not caught by RETURN_MASK_ERROR.  prev_err.message needs to be
> freed with a cleanup.  Say:
>
>    struct gdb_exception prev_err = exception_none;
>
>    old_chain = make_cleanup (xfree, full_exec_path);
>
>    make_cleanup (free_current_contents, &prev_err.message);
>
>    TRY
>      {
>         ...
>      }
>    CATCH (err, RETURN_MASK_ERROR)
>      {
>        if (err.message != NULL)
> 	warning ("%s", err.message);
>
>        prev_err = err;
>        prev_err.message = xstrdup (err.message);
>      }
>     ...
>
>    do_cleanups (old_chain);
>

Thanks. I've fixed these. I'll get v3 out once the testcase gets reviewed.

> Thanks,
> Pedro Alves
>

[PATCH v2 2/2] Test GDB connection to GDBserver with no symbol files

[Luis Machado]

This test exercises the scenarios where we attempt to connect GDB to GDBserver
in standard remote mode, query the symbol file path, attempt to open said
symbol file on GDB's end and fail, causing the connection to drop abruptly.

Regression-tested on x86-64/Ubuntu.

Does it look OK now?

Changes in v2:

- Addressed comments and moved the test to a proc that can be called multiple
times.
- The testcase now exercises these permutations:

* Unreadable file + empty sysroot
* Unreadable file + target: sysroot
* Removed file + empty sysroot
* Removed file + target: sysroot

With an unpatched GDB we should see this:

FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = permission: connection to GDBserver succeeded (the program is no longer running)
FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = delete: connection to GDBserver succeeded (the program is no longer running)
FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = permission: connection to GDBserver succeeded (the program is no longer running)
FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = delete: connection to GDBserver succeeded (the program is no longer running)

A patched GDB should have full passes.

gdb/testsuite/ChangeLog:

2016-04-12  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.server/connect-with-no-symbol-file.c: New file.
	* gdb.server/connect-with-no-symbol-file.exp: New file.
---
 .../gdb.server/connect-with-no-symbol-file.c       |  25 +++++
 .../gdb.server/connect-with-no-symbol-file.exp     | 102 +++++++++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100644 gdb/testsuite/gdb.server/connect-with-no-symbol-file.c
 create mode 100644 gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp

diff --git a/gdb/testsuite/gdb.server/connect-with-no-symbol-file.c b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.c
new file mode 100644
index 0000000..e5e4acc
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+
+int
+main (int argc, char **argv)
+{
+  printf ("Hello world!\n");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp
new file mode 100644
index 0000000..2d171b6
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp
@@ -0,0 +1,102 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's ability to properly connect to GDBserver with no pre-opened
+# symbol file.  If GDB is buggy, it will drop the connection when
+# it attempts to open the symbol file indicated by GDBserver and fails.
+#
+# This test is only meaningful for standard remote connections.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if { [skip_gdbserver_tests] } {
+    return 0
+}
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile debug] } {
+    return -1
+}
+
+# Test connecting GDB to GDBserver without loading a symbol file.
+#
+# SYSROOT is the desired sysroot string
+#
+# ACTION is the action to perform to the symbol file on the target.
+# It can be either "permission" to deny access to the file or "delete"
+# to remove the file.
+#
+proc connect_no_symbol_file { sysroot action } {
+    global binfile
+
+    with_test_prefix "setup" {
+	# Copy the symbol file to the target.
+	gdb_remote_download target $binfile.bak $binfile 
+
+	# Make sure we're disconnected, in case we're testing with an
+	# extended-remote board, therefore already connected.
+	gdb_test "disconnect" ".*" "disconnect"
+
+	# Discard any symbol files that we have opened.
+	gdb_test "file" ".*" "discard symbol table" \
+	    {Discard symbol table from `.*'\? \(y or n\) } "y"
+
+	# Set sysroot to something non-target and possibly also invalid so that
+	# GDB is unable to open the symbol file.
+	gdb_test_no_output "set sysroot $sysroot" "adjust sysroot"
+
+	set target_exec [gdbserver_download_current_prog]
+
+	# Start GDBserver.
+	set res [gdbserver_start "" $target_exec]
+
+	set gdbserver_protocol [lindex $res 0]
+	set gdbserver_gdbport [lindex $res 1]
+
+	# Perform test actions to the symbol file on the target.
+	if { $action == "delete" } then {
+	  remote_file target delete $binfile
+	} elseif { $action == "permission" } {
+	  remote_spawn target "chmod 000 $binfile"
+	}
+       
+	# Connect to GDBserver.
+	gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+    }
+
+    # Check if GDB succeeded connecting to GDBserver by attempting to detach
+    # from the process.
+    gdb_test "detach" \
+	".*Detaching from program: , process.*Ending remote debugging.*" \
+	"connection to GDBserver succeeded"
+}
+
+set sysroots {"" "target:"}
+set file_actions {"permission" "delete"}
+
+# Make sure we have the original symbol file in a safe place to copy from.
+gdb_remote_download host $binfile $binfile.bak
+
+# Run the test with different permutations.
+foreach sysroot $sysroots {
+    foreach action $file_actions {
+	with_test_prefix "test sysroot = \"$sysroot\" action = $action" {
+	    connect_no_symbol_file $sysroot $action
+	}
+    }
+}
-- 
1.9.1

Re: [PATCH v2 2/2] Test GDB connection to GDBserver with no symbol files

[Pedro Alves]

On 04/12/2016 08:24 PM, Luis Machado wrote:

> - Addressed comments and moved the test to a proc that can be called multiple
> times.
> - The testcase now exercises these permutations:
> 
> * Unreadable file + empty sysroot
> * Unreadable file + target: sysroot
> * Removed file + empty sysroot
> * Removed file + target: sysroot
> 
> With an unpatched GDB we should see this:
> 
> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = permission: connection to GDBserver succeeded (the program is no longer running)
> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = delete: connection to GDBserver succeeded (the program is no longer running)
> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = permission: connection to GDBserver succeeded (the program is no longer running)
> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = delete: connection to GDBserver succeeded (the program is no longer running)
> 
> A patched GDB should have full passes.

Excellent.  LGTM.  A couple minor comments below.

> +
> +#include <stdio.h>
> +
> +int
> +main (int argc, char **argv)
> +{
> +  printf ("Hello world!\n");
> +  return 0;
> +}

No need for stdio.h / printf, right?  As a general principle,
if the test doesn't need those, best just not to compile
them in either.

> +# Make sure we have the original symbol file in a safe place to copy from.
> +gdb_remote_download host $binfile $binfile.bak
> +
> +# Run the test with different permutations.
> +foreach sysroot $sysroots {
> +    foreach action $file_actions {
> +	with_test_prefix "test sysroot = \"$sysroot\" action = $action" {
> +	    connect_no_symbol_file $sysroot $action
> +	}
> +    }
> +}
> 

Note you can simplify using foreach_with_prefix.  No need
for the separate variables then:

foreach_with_prefix sysroot {"" "target:"} {
    foreach_with_prefix action {"permission" "delete"} {
       connect_no_symbol_file $sysroot $action
    }
}

Thanks,
Pedro Alves

Re: [PATCH v2 2/2] Test GDB connection to GDBserver with no symbol files

[Luis Machado]

On 04/13/2016 10:16 AM, Pedro Alves wrote:
> On 04/12/2016 08:24 PM, Luis Machado wrote:
>
>> - Addressed comments and moved the test to a proc that can be called multiple
>> times.
>> - The testcase now exercises these permutations:
>>
>> * Unreadable file + empty sysroot
>> * Unreadable file + target: sysroot
>> * Removed file + empty sysroot
>> * Removed file + target: sysroot
>>
>> With an unpatched GDB we should see this:
>>
>> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = permission: connection to GDBserver succeeded (the program is no longer running)
>> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "" action = delete: connection to GDBserver succeeded (the program is no longer running)
>> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = permission: connection to GDBserver succeeded (the program is no longer running)
>> FAIL: gdb.server/connect-with-no-symbol-file.exp: test sysroot = "target:" action = delete: connection to GDBserver succeeded (the program is no longer running)
>>
>> A patched GDB should have full passes.
>
> Excellent.  LGTM.  A couple minor comments below.
>
>> +
>> +#include <stdio.h>
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  printf ("Hello world!\n");
>> +  return 0;
>> +}
>
> No need for stdio.h / printf, right?  As a general principle,
> if the test doesn't need those, best just not to compile
> them in either.
>

Nope. Fixed.

>> +# Make sure we have the original symbol file in a safe place to copy from.
>> +gdb_remote_download host $binfile $binfile.bak
>> +
>> +# Run the test with different permutations.
>> +foreach sysroot $sysroots {
>> +    foreach action $file_actions {
>> +	with_test_prefix "test sysroot = \"$sysroot\" action = $action" {
>> +	    connect_no_symbol_file $sysroot $action
>> +	}
>> +    }
>> +}
>>
>
> Note you can simplify using foreach_with_prefix.  No need
> for the separate variables then:
>
> foreach_with_prefix sysroot {"" "target:"} {
>      foreach_with_prefix action {"permission" "delete"} {
>         connect_no_symbol_file $sysroot $action
>      }
> }
>

Ah, handy! Thanks for the suggestion.

> Thanks,
> Pedro Alves
>