aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMagne Hov <mhov@undo.io>2021-06-03 21:20:30 +0100
committerMagne Hov <mhov@undo.io>2021-06-03 21:22:08 +0100
commitdf5bc734f2dcd07ed3261cd679743924e8962439 (patch)
tree0c3b5b7edff73fe5a41a28df057f07c31fda7ed5
parent415c8100a28d56628c01730ef279c61f99578518 (diff)
downloadgdb-df5bc734f2dcd07ed3261cd679743924e8962439.zip
gdb-df5bc734f2dcd07ed3261cd679743924e8962439.tar.gz
gdb-df5bc734f2dcd07ed3261cd679743924e8962439.tar.bz2
gdb: fix eval.c assert during inferior exit event
Evaluating expressions from within an inferior exit event handler can cause a crash: echo "int main() { return 0; }" > repro.c gcc -g repro.c -o repro ./gdb -q --ex "set language c++" --ex "python gdb.events.exited.connect(lambda _: gdb.execute('set \$_a=0'))" --ex "run" repro Reading symbols from repro... Starting program: /home/mhov/repos/binutils-gdb-master/install-bad/bin/repro [Inferior 1 (process 1974779) exited normally] ../../gdb/thread.c:72: internal-error: thread_info* inferior_thread(): Assertion `current_thread_ != nullptr' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) [answered Y; input not from terminal] This is a bug, please report it. For instructions, see: <https://www.gnu.org/software/gdb/bugs/>. Backtrace 0 in internal_error of ../../gdbsupport/errors.cc:51 1 in inferior_thread of ../../gdb/thread.c:72 2 in expression::evaluate of ../../gdb/eval.c:98 3 in evaluate_expression of ../../gdb/eval.c:115 4 in set_command of ../../gdb/printcmd.c:1502 5 in do_const_cfunc of ../../gdb/cli/cli-decode.c:101 6 in cmd_func of ../../gdb/cli/cli-decode.c:2181 7 in execute_command of ../../gdb/top.c:670 ... 22 in python_inferior_exit of ../../gdb/python/py-inferior.c:182 In `expression::evaluate (...)' there is a call to `inferior_thread ()' that is guarded by `target_has_execution ()': struct value * expression::evaluate (struct type *expect_type, enum noside noside) { gdb::optional<enable_thread_stack_temporaries> stack_temporaries; if (target_has_execution () && language_defn->la_language == language_cplus && !thread_stack_temporaries_enabled_p (inferior_thread ())) stack_temporaries.emplace (inferior_thread ()); The `target_has_execution ()' guard maps onto `inf->pid' and the `inferior_thread ()' call assumes that `current_thread_' is set to something meaningful: struct thread_info* inferior_thread (void) { gdb_assert (current_thread_ != nullptr); return current_thread_; } In other words, it is assumed that if `inf->pid' is set then `current_thread_' must also be set. This does not hold at the point where inferior exit observers are notified: - `generic_mourn_inferior (...)' - `switch_to_no_thread ()' - `current_thread_ = nullptr;' - `exit_inferior (...)' - `gdb::observers::inferior_exit.notify (...)' - `inf->pid = 0' The inferior exit notification means that a Python handler can get a chance to run while `current_thread' has been cleared and the `inf->pid' has not been cleared. Since the Python handler can call any GDB command with `gdb.execute(...)' (in my case `gdb.execute("set $_a=0")' we can end up evaluating expressions and asserting in `evaluate_subexp (...)'. This patch adds a test in `evaluate_subexp (...)' to check the global `inferior_ptid' which is reset at the same time as `current_thread_'. Checking `inferior_ptid' at the same time as `target_has_execution ()' seems to be a common pattern: $ git grep -n -e inferior_ptid --and -e target_has_execution gdb/breakpoint.c:2998: && (inferior_ptid == null_ptid || !target_has_execution ())) gdb/breakpoint.c:3054: && (inferior_ptid == null_ptid || !target_has_execution ())) gdb/breakpoint.c:4587: if (inferior_ptid == null_ptid || !target_has_execution ()) gdb/infcmd.c:360: if (inferior_ptid != null_ptid && target_has_execution ()) gdb/infcmd.c:2380: /* FIXME: This should not really be inferior_ptid (or target_has_execution). gdb/infrun.c:3438: if (!target_has_execution () || inferior_ptid == null_ptid) gdb/remote.c:11961: if (!target_has_execution () || inferior_ptid == null_ptid) gdb/solib.c:725: if (target_has_execution () && inferior_ptid != null_ptid) The testsuite has been run on 5.4.0-59-generic x86_64 GNU/Linux: - Ubuntu 20.04.1 LTS - gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0 - DejaGnu version 1.6.2 - Expect version 5.45.4 - Tcl version 8.6 - Native configuration: x86_64-pc-linux-gnu - Target: unix Results show a few XFAIL in gdb.threads/attach-many-short-lived-threads.exp. The existing py-events.exp tests are skipped for native-gdbserver and fail for native-extended-gdbserver, but the new tests pass with native-extended-gdbserver when run without the existing tests. gdb/ChangeLog: 2021-06-03 Magne Hov <mhov@undo.io> PR python/27841 * eval.c (expression::evaluate): Check inferior_ptid. gdb/testsuite/ChangeLog: 2021-06-03 Magne Hov <mhov@undo.io> PR python/27841 * gdb.python/py-events.exp: Extend inferior exit tests. * gdb.python/py-events.py: Print inferior exit PID.
-rw-r--r--gdb/ChangeLog5
-rw-r--r--gdb/eval.c2
-rw-r--r--gdb/testsuite/ChangeLog6
-rw-r--r--gdb/testsuite/gdb.python/py-events.exp41
-rw-r--r--gdb/testsuite/gdb.python/py-events.py1
5 files changed, 54 insertions, 1 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4a624f2..1dd4d3e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2021-06-03 Magne Hov <mhov@undo.io>
+
+ PR python/27841
+ * eval.c (expression::evaluate): Check inferior_ptid.
+
2021-06-03 Pedro Alves <pedro@palves.net>
* MAINTAINERS (The Official FSF-appointed GDB Maintainers): Remove
diff --git a/gdb/eval.c b/gdb/eval.c
index 1b3c974..659493c 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -93,7 +93,7 @@ struct value *
expression::evaluate (struct type *expect_type, enum noside noside)
{
gdb::optional<enable_thread_stack_temporaries> stack_temporaries;
- if (target_has_execution ()
+ if (target_has_execution () && inferior_ptid != null_ptid
&& language_defn->la_language == language_cplus
&& !thread_stack_temporaries_enabled_p (inferior_thread ()))
stack_temporaries.emplace (inferior_thread ());
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 642c18c..afa9ebf 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2021-06-03 Magne Hov <mhov@undo.io>
+
+ PR python/27841
+ * gdb.python/py-events.exp: Extend inferior exit tests.
+ * gdb.python/py-events.py: Print inferior exit PID.
+
2021-06-03 Hannes Domani <ssbssa@yahoo.de>
* gdb.python/py-symbol.exp: Test symbol constants.
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e89cd8b..7537093 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -197,18 +197,33 @@ gdb_test_multiple "continue" $test {
gdb_test_no_output "delete $second_breakpoint"
#test exited event.
+proc get_process_id {test} {
+ global gdb_prompt
+ gdb_test_multiple "info proc" $test {
+ -re "process (\\d+).*$gdb_prompt $" {
+ set process_id $expect_out(1,string)
+ pass $gdb_test_name
+ }
+ }
+ return ${process_id}
+}
+
+set process_id [get_process_id "get inferior 1 process id"]
gdb_test "continue" ".*event type: continue.*
.*clear_objfiles\[\r\n\]*progspace: .*py-events.*
.*event type: exit.*
.*exit code: 12.*
.*exit inf: 1.*
+.*exit pid: $process_id.*
dir ok: True.*" "Inferior 1 terminated."
gdb_test "inferior 2" ".*Switching to inferior 2.*"
+set process_id [get_process_id "get inferior 2 process id"]
gdb_test "continue" ".*event type: continue.*
.*event type: exit.*
.*exit code: 12.*
.*exit inf: 2.*
+.*exit pid: $process_id.*
dir ok: True.*" "Inferior 2 terminated."
@@ -235,3 +250,29 @@ gdb_test "python print(count)" 2 "check for before_prompt event"
gdb_test_no_output "xxz" "run a canned sequence"
gdb_test "python print(count)" 4 "check for before_prompt event again"
+
+# Test evaluating expressions from within an inferior exit event handler. This
+# used to cause a crash when expression were evaluated as C++.
+gdb_test_no_output "set language c++"
+
+gdb_test_multiline "add exited listener" \
+ "python" "" \
+ "def increment_foo(_):" "" \
+ " gdb.execute('set \$_foo=\$_foo+1')" "" \
+ "gdb.events.exited.connect(increment_foo)" "" \
+ "end" ""
+gdb_test "set \$_foo=0" "" "initialize foo variable"
+gdb_test "print \$_foo" "= 0" "check foo initialized"
+
+with_test_prefix "inferior run exit" {
+ gdb_run_cmd
+ gdb_test "" "exited with code.*" "run to exit"
+ gdb_test "print \$_foo" "= 1" "check foo after run"
+}
+
+with_test_prefix "inferior continue exit" {
+ gdb_start_cmd
+ gdb_test "" "Temporary breakpoint .* main .*" "stop on a frame"
+ gdb_test "continue" "exited with code.*" "continue to exit"
+ gdb_test "print \$_foo" "= 2" "check foo after start continue"
+}
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 6a67627..1524267 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -47,6 +47,7 @@ def exit_handler(event):
print("event type: exit")
print("exit code: %d" % (event.exit_code))
print("exit inf: %d" % (event.inferior.num))
+ print("exit pid: %d" % (event.inferior.pid))
print("dir ok: %s" % str("exit_code" in dir(event)))