[binutils-gdb] PR gdb/20604 - fix "quit" when an invalid expression is used

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=36cf1806a87fdd208c704d0768af232ce35aae9f

commit 36cf1806a87fdd208c704d0768af232ce35aae9f
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Sep 14 11:48:31 2016 -0600

    PR gdb/20604 - fix "quit" when an invalid expression is used
    
    This fixes PR gdb/20604.  The bug here is that passing an invalid
    expression to "quit" -- e.g., "quit()" -- causes gdb to enter a
    non-functioning state.
    
    The immediate problem is that quit_force resets the terminal before
    evaluating the expression.  However, it seemed to me that it doesn't
    really make sense to pass the quit_force argument to kill_or_detach
    (which passes it to to_detach), first because conflating the exit
    status for "quit" and the signal to pass when detaching doesn't make
    sense, and second because to_detach implementations generally only
    accept a constant here, while "quit" accepts an expression.  So, I
    removed that.
    
    As an aside, I think the "detach SIGNO" functionality is not
    documented.
    
    Built and regtested on x86-64 Fedora 24.
    
    2016-09-21  Tom Tromey  <tom@tromey.com>
    
    	PR gdb/20604:
    	* top.h (quit_force): Update.
    	* top.c (quit_force): Changed type of first argument.  Don't
    	evaluate expression.  Pass NULL to kill_or_detach.
    	* cli/cli-cmds.c (quit_command): Evaluate "args".
    
    2016-09-21  Tom Tromey  <tom@tromey.com>
    
    	PR gdb/20604:
    	* gdb.base/quit.exp: New file.

Diff:
---
 gdb/ChangeLog                   |  8 ++++++++
 gdb/cli/cli-cmds.c              | 13 ++++++++++++-
 gdb/testsuite/ChangeLog         |  5 +++++
 gdb/testsuite/gdb.base/quit.exp | 35 +++++++++++++++++++++++++++++++++++
 gdb/top.c                       | 12 ++++--------
 gdb/top.h                       |  2 +-
 6 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4f16b09..6753ebe 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2016-09-21  Tom Tromey  <tom@tromey.com>
+
+	PR gdb/20604:
+	* top.h (quit_force): Update.
+	* top.c (quit_force): Changed type of first argument.  Don't
+	evaluate expression.  Pass NULL to kill_or_detach.
+	* cli/cli-cmds.c (quit_command): Evaluate "args".
+
 2016-09-21  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* .gitignore: Ignore more files.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5ab4791..384a336 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -344,12 +344,23 @@ show_configuration (char *args, int from_tty)
 void
 quit_command (char *args, int from_tty)
 {
+  int exit_code = 0;
+
+  /* An optional expression may be used to cause gdb to terminate with
+     the value of that expression.  */
+  if (args)
+    {
+      struct value *val = parse_and_eval (args);
+
+      exit_code = (int) value_as_long (val);
+    }
+
   if (!quit_confirm ())
     error (_("Not confirmed."));
 
   query_if_trace_running (from_tty);
 
-  quit_force (args, from_tty);
+  quit_force (args ? &exit_code : NULL, from_tty);
 }
 
 static void
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3da0314..1b8bc1a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-09-21  Tom Tromey  <tom@tromey.com>
+
+	PR gdb/20604:
+	* gdb.base/quit.exp: New file.
+
 2016-09-21  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* .gitignore: New file.
diff --git a/gdb/testsuite/gdb.base/quit.exp b/gdb/testsuite/gdb.base/quit.exp
new file mode 100644
index 0000000..1c9db64
--- /dev/null
+++ b/gdb/testsuite/gdb.base/quit.exp
@@ -0,0 +1,35 @@
+# Copyright (C) 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/>.
+
+# Check the "quit" command.
+
+clean_restart
+
+# Test that a syntax error causes quit to abort.
+# Regression test for PR gdb/20604.
+gdb_test "quit()" "A syntax error in expression, near .*" \
+    "quit with syntax error"
+
+# Test that an expression can be used to set the error code.
+send_gdb "quit 22+1\n"
+set result [wait -i $gdb_spawn_id]
+verbose $result
+if {[lindex $result 2] == 0 && [lindex $result 3] == 23} {
+    pass "quit with expression"
+} else {
+    fail "quit with expression"
+}
+close $gdb_spawn_id
+clear_gdb_spawn_id
diff --git a/gdb/top.c b/gdb/top.c
index 320c296..3cfa113 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1625,7 +1625,7 @@ undo_terminal_modifications_before_exit (void)
 /* Quit without asking for confirmation.  */
 
 void
-quit_force (char *args, int from_tty)
+quit_force (int *exit_arg, int from_tty)
 {
   int exit_code = 0;
   struct qt_args qt;
@@ -1634,16 +1634,12 @@ quit_force (char *args, int from_tty)
 
   /* An optional expression may be used to cause gdb to terminate with the 
      value of that expression.  */
-  if (args)
-    {
-      struct value *val = parse_and_eval (args);
-
-      exit_code = (int) value_as_long (val);
-    }
+  if (exit_arg)
+    exit_code = *exit_arg;
   else if (return_child_result)
     exit_code = return_child_result_value;
 
-  qt.args = args;
+  qt.args = NULL;
   qt.from_tty = from_tty;
 
   /* We want to handle any quit errors and exit regardless.  */
diff --git a/gdb/top.h b/gdb/top.h
index ee664c1..acdb8e9 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -212,7 +212,7 @@ extern void read_command_file (FILE *);
 extern void init_history (void);
 extern void command_loop (void);
 extern int quit_confirm (void);
-extern void quit_force (char *, int);
+extern void quit_force (int *, int);
 extern void quit_command (char *, int);
 extern void quit_cover (void);
 extern void execute_command (char *, int);