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

Message ID 73751e0b7adf9e015e32275d910ec1c3bd2edade.1730020661.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 Oct. 27, 2024, 9:19 a.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 command line flags is that they forward
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, which then fails to parse the full command name.

The solution I propose here is to escape any special characters in the
core file name passed from the command line before calling 'core-file'
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/pathstuff.cc             |  21 +++++
 gdbsupport/pathstuff.h              |   6 ++
 4 files changed, 89 insertions(+), 71 deletions(-)
  

Patch

diff --git a/gdb/main.c b/gdb/main.c
index e4a40c51023..7559941adba 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1233,7 +1233,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_filename (corearg).c_str (),
 				  !batch_flag);
     }
   else if (pidarg != NULL)
@@ -1251,16 +1252,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_filename (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_filename (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/pathstuff.cc b/gdbsupport/pathstuff.cc
index faed79b2cfb..d9b92c3cfc9 100644
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -395,3 +395,24 @@  make_temp_filename (const std::string &f)
   strcat (filename_temp.data () + f.size (), "-XXXXXX");
   return filename_temp;
 }
+
+/* See gdbsupport/pathstuff.h.  */
+
+std::string
+make_quoted_filename (const char *filename)
+{
+  gdb_assert (filename != nullptr);
+
+  std::string result;
+
+  for (; *filename != '\0'; ++filename)
+    {
+      const char ch = *filename;
+
+      if (strchr ("\"' \t\n", ch) != nullptr)
+	result.push_back ('\\');
+      result.push_back (ch);
+    }
+
+  return result;
+}
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
index 15c7872aa9a..bb54a167cb0 100644
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -180,4 +180,10 @@  extern const char *get_shell ();
 
 extern gdb::char_vector make_temp_filename (const std::string &f);
 
+/* Return a copy of FILENAME, but with any special characters escaped
+   using a backslash.  The special characters are the white space
+   characters and single and double quotes.  */
+
+extern std::string make_quoted_filename (const char *filename);
+
 #endif /* COMMON_PATHSTUFF_H */