From: Magne Hov <mhov@undo.io>
To: gdb-patches@sourceware.org
Subject: [RFC PATCH] gdb: fix eval.c assert during inferior exit event
Date: Wed, 5 May 2021 16:56:27 +0100 [thread overview]
Message-ID: <20210505155627.3850386-1-mhov@undo.io> (raw)
Evaluating expressions from within an inferior exit event handler can
cause a crash:
echo "int main() { return 0; }" > repro.cc
g++ -g repro.cc -o repro
./gdb -q --ex "start" --ex "python gdb.events.exited.connect(lambda _: gdb.execute('set \$_a=0'))" --ex "continue" repro
Reading symbols from repro...
Temporary breakpoint 1 at 0x1131: file repro.cc, line 1.
Starting program: /home/mhov/repos/binutils-gdb-master/install/bin/repro
Temporary breakpoint 1, main () at repro.cc:1
1 int main() { return 0; }
Continuing.
[Inferior 1 (process 75524) 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
This is the same assertion site as in
<https://sourceware.org/bugzilla/show_bug.cgi?id=26761>, but I'm not
sure if it is the same problem.
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 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
Diffing results before and after the patch shows a few XFAIL in
gdb.threads/attach-many-short-lived-threads.exp, and one unexpected
failure in gdb.base/run-attach-while-running.exp which seems to be
flaky on master as well.
gdb/ChangeLog:
2021-05-05 Magne Hov <mhov@undo.io>
* eval.c (expression::evaluate): Check inferior_ptid.
gdb/testsuite/ChangeLog:
2021-05-05 Magne Hov <mhov@undo.io>
* gdb.python/py-events.exp: Extend inferior exit tests.
* gdb.python/py-events.py: Print inferior exit PID.
* gdb.python/py-events.cc: New test.
---
gdb/eval.c | 2 +-
gdb/testsuite/gdb.python/py-events.cc | 21 ++++++++++++++
gdb/testsuite/gdb.python/py-events.exp | 39 ++++++++++++++++++++++++++
gdb/testsuite/gdb.python/py-events.py | 1 +
4 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.python/py-events.cc
diff --git a/gdb/eval.c b/gdb/eval.c
index 1b3c974009a..34c6c6160d4 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/gdb.python/py-events.cc b/gdb/testsuite/gdb.python/py-events.cc
new file mode 100644
index 00000000000..b865a5a9652
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-events.cc
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 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/>. */
+
+int main(void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e89cd8b021b..bec3c9f2cab 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
+ global process_id
+ gdb_test_multiple "info proc" $test {
+ -re "process (\\d+).*$gdb_prompt $" {
+ set process_id $expect_out(1,string)
+ pass $test
+ }
+ }
+}
+
+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.*"
+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,27 @@ 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 C++ expression were evaluated.
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+ return -1
+}
+
+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"
+gdb_test "run" "exited normally.*"
+gdb_test "print \$_foo" "= 1" "check foo after run"
+gdb_test "start" "Temporary breakpoint .* main .*" "stop on a C++ frame"
+gdb_test "continue" "exited normally.*"
+gdb_test "print \$_foo" "= 2" "check foo after start continue"
+
+>>>>>>> 2d04478f2ca... gdb: check inferior_ptid before calling inferior_thread in eval.c
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index bd1dd749452..53c7f9f20a4 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -44,6 +44,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)))
def continue_handler (event):
--
2.25.1
next reply other threads:[~2021-05-05 15:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 15:56 Magne Hov [this message]
2021-05-06 1:59 ` Simon Marchi
2021-05-10 16:57 ` Magne Hov
2021-05-10 17:33 ` Simon Marchi
2021-05-10 17:21 ` [PATCH v2] " Magne Hov
2021-05-10 17:44 ` Simon Marchi
2021-05-14 16:24 ` Magne Hov
2021-05-11 7:16 ` Aktemur, Tankut Baris
2021-05-14 16:22 ` Magne Hov
2021-05-24 19:23 ` Joel Brobecker
2021-05-26 13:02 ` [PATCH v3] " Magne Hov
2021-05-29 2:25 ` Simon Marchi
2021-06-02 11:34 ` Magne Hov
2021-06-03 21:07 ` Magne Hov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210505155627.3850386-1-mhov@undo.io \
--to=mhov@undo.io \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).