[PATCHv2,2/2] gdb: allow core file containing special characters on the command line

Message ID 522ad1692a5cc56c41d693112c2be6d37c3dc897.1732544034.git.aburgess@redhat.com
State New
Headers
Series Fix regression passing core files on GDB command line |

Checks

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

Commit Message

Andrew Burgess Nov. 25, 2024, 2:16 p.m. UTC
  After the commit:

  commit 03ad29c86c232484f9090582bbe6f221bc87c323
  Date:   Wed Jun 19 11:14:08 2024 +0100

      gdb: 'target ...' commands now expect quoted/escaped filenames

it was no longer possible to pass GDB the name of a core file
containing any special characters (white space or quote characters) on
the command line.  For example:

  $ gdb -c /tmp/core\ file.core
  Junk after filename "/tmp/core": file.core
  (gdb)

The problem is that the above commit changed the 'target core' command
to expect quoted filenames, so before the above commit a user could
write:

  (gdb) target core /tmp/core file.core
  [New LWP 2345783]
  Core was generated by `./mkcore'.
  Program terminated with signal SIGSEGV, Segmentation fault.
  #0  0x0000000000401111 in ?? ()
  (gdb)

But after the above commit the user must write:

  (gdb) target core /tmp/core\ file.core

or

  (gdb) target core "/tmp/core file.core"

This is part of a move to make GDB's filename argument handling
consistent.

Anyway, the problem with the '-c' command line flag is that it
forwards the filename unmodified through to the 'core-file' command,
which in turn forwards to the 'target core' command.

So when the user, at a shell writes:

  $ gdb -c "core file.core"

this arrives in GDB as the unquoted string 'core file.core' (without
the single quotes).  GDB then forwards this to the 'core-file'
command as if the user had written this at a GDB prompt:

  (gdb) core-file core file.core

Which then fails to parse due to the unquoted white space between
'core' and 'file.core'.

The solution I propose is to escape any special characters in the core
file name passed from the command line before calling 'core-file'
command from main.c.

I've updated the corefile.exp test to include a test for passing a
core file containing a white space character.  While I was at it I've
modernised the part of corefile.exp that I was touching.
---
 gdb/main.c                          |  17 ++--
 gdb/testsuite/gdb.base/corefile.exp | 116 +++++++++++++---------------
 gdbsupport/common-utils.cc          |  21 +++++
 gdbsupport/common-utils.h           |   5 ++
 4 files changed, 88 insertions(+), 71 deletions(-)
  

Patch

diff --git a/gdb/main.c b/gdb/main.c
index 4370e95ada4..14337fbda22 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1235,7 +1235,8 @@  captured_main_1 (struct captured_main_args *context)
 
   if (corearg != NULL)
     {
-      ret = catch_command_errors (core_file_command, corearg,
+      ret = catch_command_errors (core_file_command,
+				  make_quoted_string (corearg).c_str (),
 				  !batch_flag);
     }
   else if (pidarg != NULL)
@@ -1253,16 +1254,18 @@  captured_main_1 (struct captured_main_args *context)
 	  ret = catch_command_errors (attach_command, pid_or_core_arg,
 				      !batch_flag);
 	  if (ret == 0)
-	    ret = catch_command_errors (core_file_command,
-					pid_or_core_arg,
-					!batch_flag);
+	    ret = catch_command_errors
+	      (core_file_command,
+	       make_quoted_string (pid_or_core_arg).c_str (),
+	       !batch_flag);
 	}
       else
 	{
 	  /* Can't be a pid, better be a corefile.  */
-	  ret = catch_command_errors (core_file_command,
-				      pid_or_core_arg,
-				      !batch_flag);
+	  ret = catch_command_errors
+	    (core_file_command,
+	     make_quoted_string (pid_or_core_arg).c_str (),
+	     !batch_flag);
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index dc3c8b1dfc8..039f1894bc0 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -34,81 +34,69 @@  if {$corefile == ""} {
     return 0
 }
 
-# Test that we can simply startup with a "-core=$corefile" command line arg
-# and recognize that the core file is a valid, usable core file.
-# To do this, we must shutdown the currently running gdb and restart
-# with the -core args.  We can't use gdb_start because it looks for
-# the first gdb prompt, and the message we are looking for occurs
-# before the first prompt.
+# Start GDB with COREFILE passed as a command line argument.  COREOPT
+# is prefixed before COREFILE and is the command line flag to specify
+# the corefile, i.e. one of '--core=', '-core=', '-c '.
 #
-# Another problem is that on some systems (solaris for example), there
-# is apparently a limit on the length of a fully specified path to 
-# the corefile executable, at about 80 chars.  For this case, consider
-# it a pass, but note that the program name is bad.
-
-gdb_exit
-if {$verbose>1} {
-    send_user "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS -core=$corefile\n"
-}
-
-set oldtimeout $timeout
-set timeout [expr "$timeout + 60"]
-verbose "Timeout is now $timeout seconds" 2
-eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS -core=$corefile"
-expect {
-    -re "Couldn't find .* registers in core file.*$gdb_prompt $" {
-        fail "args: -core=[file tail $corefile] (couldn't find regs)"
-    }
-    -re "Core was generated by .*corefile.*\r\n\#0  .*\(\).*\r\n$gdb_prompt $" {
-	pass "args: -core=[file tail $corefile]"
-    }
-    -re "Core was generated by .*\r\n\#0  .*\(\).*\r\n$gdb_prompt $" {
-	pass "args: -core=[file tail $corefile] (with bad program name)"
-    }
-    -re ".*registers from core file: File in wrong format.* $" {
-	fail "args: -core=[file tail $corefile] (could not read registers from core file)"
-    }
-    -re ".*$gdb_prompt $"	{ fail "args: -core=[file tail $corefile]" }
-    timeout 		{ fail "(timeout) starting with -core" }
-}
-
-
+# If BINFILE is not the empty string then it is also added as a
+# command line argument and is the executable to load.
 #
-# Test that startup with both an executable file and -core argument.
-# See previous comments above, they are still applicable.
-#
-
-close
+# TESTNAME is used for naming the tests.
+proc start_gdb_with_corefile { testname coreopt corefile {binfile ""} } {
+    gdb_exit
 
-if {$verbose>1} {
-    send_user "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS $binfile -core=$corefile\n"
-}
+    global GDBFLAGS
 
+    save_vars { GDBFLAGS } {
+	append GDBFLAGS " $binfile $coreopt$corefile"
+	set res [gdb_spawn]
+	if { $res != 0 } {
+	    fail "$testname (start GDB)"
+	    return
+	}
 
-eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS $binfile -core=$corefile"
-expect {
-    -re "Core was generated by .*corefile.*\r\n\#0  .*\(\).*\r\n$gdb_prompt $" {
-	pass "args: execfile -core=[file tail $corefile]"
-    }
-    -re "Core was generated by .*\r\n\#0  .*\(\).*\r\n$gdb_prompt $"	 {
-	pass "args: execfile -core=[file tail $corefile] (with bad program name)"
-    }
-    -re ".*registers from core file: File in wrong format.* $" {
-	fail "args: execfile -core=[file tail $corefile] (could not read registers from core file)"
+	gdb_test_multiple "" $testname {
+	    -re -wrap "Couldn't find .* registers in core file.*" {
+		fail "$gdb_test_name (couldn't find regs)"
+	    }
+	    -re -wrap "Core was generated by `[string_to_regexp $corefile]'\\.\r\n.*\#0  \[^\r\n\]+\(\).*" {
+		pass $gdb_test_name
+	    }
+	    -re -wrap "Core was generated by .*\r\n\#0  .*\(\).*" {
+		# This case is hit when the executable name is
+		# truncated in the output.
+		pass "$gdb_test_name (with bad program name)"
+	    }
+	    -re -wrap ".*registers from core file: File in wrong format.*" {
+		fail "$gdb_test_name (could not read registers from core file)"
+	    }
+	    -re -wrap "" {
+		fail "$gdb_test_name (core not loaded)"
+	    }
+	}
     }
-    -re ".*$gdb_prompt $"	{ fail "args: execfile -core=[file tail $corefile]" }
-    timeout 		{ fail "(timeout) starting with -core" }
 }
-set timeout $oldtimeout
-verbose "Timeout is now $timeout seconds" 2
 
-close
+# Create a copy of the corefile, but with a space in the filename.
+set alt_corefile [standard_output_file "core\\ file.core"]
+remote_exec host "cp $corefile $alt_corefile"
+
+# Test that we can start GDB with a corefile command line argument and
+# recognize that the core file is a valid, usable core file.  We test
+# using '--core=...', '-core=...', and '-c ...' style arguments.  We
+# also test with, and without an executable.
+foreach_with_prefix coreopt {--core= -core= "-c "} {
+    start_gdb_with_corefile "just core file" $coreopt $corefile
+    start_gdb_with_corefile "core file and executable" $coreopt $corefile $binfile
+    start_gdb_with_corefile "core file with white space in name" \
+	$coreopt $alt_corefile
+    start_gdb_with_corefile "core file with white space in name and executable" \
+	$coreopt $alt_corefile $binfile
+}
 
 # Now restart normally.
 
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+clean_restart $binfile
 
 # Test basic corefile recognition via core-file command.
 
diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
index 91c14a0f139..af25bf0ee9c 100644
--- a/gdbsupport/common-utils.cc
+++ b/gdbsupport/common-utils.cc
@@ -222,6 +222,27 @@  extract_string_maybe_quoted (const char **arg)
   return result;
 }
 
+/* See gdbsupport/common-utils.h.  */
+
+std::string
+make_quoted_string (const char *str)
+{
+  gdb_assert (str != nullptr);
+
+  std::string result;
+
+  for (; *str != '\0'; ++str)
+    {
+      const char ch = *str;
+
+      if (strchr ("\"' \t\n", ch) != nullptr)
+       result.push_back ('\\');
+      result.push_back (ch);
+    }
+
+  return result;
+}
+
 /* The bit offset of the highest byte in a ULONGEST, for overflow
    checking.  */
 
diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h
index 2fb22916409..65158388881 100644
--- a/gdbsupport/common-utils.h
+++ b/gdbsupport/common-utils.h
@@ -84,6 +84,11 @@  char *savestring (const char *ptr, size_t len);
 
 std::string extract_string_maybe_quoted (const char **arg);
 
+/* Return a copy of STR, but with any white space, single quote, or
+   double quote characters escaped with a backslash.  */
+
+std::string make_quoted_string (const char *str);
+
 /* The strerror() function can return NULL for errno values that are
    out of range.  Provide a "safe" version that always returns a
    printable string.  This version is also thread-safe.  */