diff mbox series

[PATCHv2,2/5] gdbserver: fix handling of trailing empty argument

Message ID 3485c21f838f6df48b865663aa637d21d6656661.1695835626.git.aburgess@redhat.com
State New
Headers
Series Fixes for passing arguments to gdbserver |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Andrew Burgess Sept. 27, 2023, 5:27 p.m. UTC 
  When I posted the previous patch for review Andreas Schwab pointed out
that passing a trailing empty argument also doesn't work.

The fix for this is in the same area of code as the previous patch,
but is sufficiently different that I felt it deserved a patch of its
own.

I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:

  gdb -ex 'set sysroot' --args /tmp/show-args
  Reading symbols from /tmp/show-args...
  (gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
  Remote debugging using | gdbserver --once --multi - /tmp/show-args
  stdin/stdout redirected
  Process /tmp/show-args created; pid = 176054
  Remote debugging using stdio
  Reading symbols from /lib64/ld-linux-x86-64.so.2...
  (No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
  0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
  (gdb) set args abc ""
  (gdb) run
  The program being debugged has been started already.
  Start it from the beginning? (y or n) y
  Starting program: /tmp/show-args \'
  stdin/stdout redirected
  Process /tmp/show-args created; pid = 176088
  2 args are:
    /tmp/show-args
    abc
  Done.
  [Inferior 1 (process 176088) exited normally]
  (gdb) target native
  Done.  Use the "run" command to start a process.
  (gdb) run
  Starting program: /tmp/show-args \'
  2 args are:
    /tmp/show-args
    abc

  Done.
  [Inferior 1 (process 176095) exited normally]
  (gdb) q

The 'shows-args' program used here just prints the arguments passed to
the inferior.

Notice that when starting the inferior using the extended-remote
target there is only a single argument 'abc', while when using the
native target there is a second argument, the blank line, representing
the empty argument.

The problem here is that the vRun packet coming from GDB looks like
this (I've removing the trailing checksum):

  $vRun;PROGRAM_NAME;616263;

If we compare this to a packet with only a single argument and no
trailing empty argument:

  $vRun;PROGRAM_NAME;616263

Notice the lack of the trailing ';' character here.

The problem is that gdbserver processes this string in a loop.  At
each point we maintain a pointer to the character just after a ';',
and then we process everything up to either the next ';' character, or
to the end of the string.

We break out of this loop when the character we start with (in that
loop iteration) is the null-character.  This means in the trailing
empty argument case, we abort the loop before doing anything with the
empty argument.

In this commit I've updated the loop, we now break out using a 'break'
statement at the end of the loop if the (sub-)string we just processed
was empty, with this change we now notice the trailing empty
argument.

I've updated the test case to cover this issue.
---
 gdb/testsuite/gdb.base/inferior-args.exp | 9 ++++++---
 gdbserver/server.cc                      | 8 +++++---
 2 files changed, 11 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.base/inferior-args.exp b/gdb/testsuite/gdb.base/inferior-args.exp
index 3d3cd39a706..2c920ab14ec 100644
--- a/gdb/testsuite/gdb.base/inferior-args.exp
+++ b/gdb/testsuite/gdb.base/inferior-args.exp
@@ -28,8 +28,10 @@  if {[build_executable "failed to prepare" $testfile $srcfile \
 proc do_test { method } {
     global binfile hex
 
-    # The second arg is an empty string on purpose.
-    set inferior_args { "first arg" "" "third-arg" "'" "\"" " " }
+    # The second arg is an empty string on purpose.  The last argument
+    # must be the empty argument -- we once had a bug where that
+    # wouldn't work!
+    set inferior_args { "first arg" "" "third-arg" "'" "\"" " " "" }
 
     clean_restart $binfile
 
@@ -109,7 +111,7 @@  proc do_test { method } {
     }
 
     # Now that we are stopped at main, inspect argc/argv.
-    gdb_test "print argc" " = 7"
+    gdb_test "print argc" " = 8"
     gdb_test "print argv\[0\]" " = $hex \".*\""
     gdb_test "print argv\[1\]" " = $hex \"first arg\""
     gdb_test "print argv\[2\]" " = $hex \"\""
@@ -117,6 +119,7 @@  proc do_test { method } {
     gdb_test "print argv\[4\]" " = $hex \"'\""
     gdb_test "print argv\[5\]" " = $hex \"\\\\\"\""
     gdb_test "print argv\[6\]" " = $hex \" \""
+    gdb_test "print argv\[7\]" " = $hex \"\""
 }
 
 foreach_with_prefix method { "start" "starti" "run" "set args" } {
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 496b9bebb7d..d78eb5a7d94 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -2969,7 +2969,9 @@  handle_v_run (char *own_buf)
   char *new_program_name = NULL;
   int i;
 
-  for (i = 0, p = own_buf + strlen ("vRun;"); *p; p = next_p, ++i)
+  for (i = 0, p = own_buf + strlen ("vRun;");
+       /* Exit condition is at the end of the loop.  */;
+       p = next_p + 1, ++i)
     {
       next_p = strchr (p, ';');
       if (next_p == NULL)
@@ -3032,8 +3034,8 @@  handle_v_run (char *own_buf)
 	    new_argv.push_back (full_arg);
 	  xfree (arg);
 	}
-      if (*next_p)
-	next_p++;
+      if (*next_p == '\0')
+	break;
     }
 
   if (new_program_name == NULL)