From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by sourceware.org (Postfix) with ESMTPS id 34CC83A4DC36 for ; Wed, 5 May 2021 15:56:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 34CC83A4DC36 Received: by mail-wm1-x334.google.com with SMTP id b19-20020a05600c06d3b029014258a636e8so1435868wmn.2 for ; Wed, 05 May 2021 08:56:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=uGPgZua8o6N+Sep8I9rbVu4NO73a3csDZFG2i+JDGbA=; b=pxej40RlqBx26pMazdrQdqAXKmqQcNuQRqVUA3HdLRAN5pEHc2b8pngd7KFl9zGly0 IeMgJFrvF3ET5fJyWXDO6E8hiE3FMi3w3WYSJ+Os/3C7pSnqfUZOvSiXeXE2u9jQnp/i sRnggq/kGbjiatCwF9BeTggDD25SBdhfckw0iR1GXEq0wlywQCPXkghh0ThFZr8kIsTx tE9NnPxVBRyy/p1sbFg7Kzo5Hfm27Hlk2jrXIPiJdNKSM335X6WKzISCU1daesfVvIjM 8WDUmZZN/r1KtUz3BY5IdOnjTMWcBqxXg/JILhbZ1XXHdXj4ZUG6MBz3pTQbzarLVurw mOig== X-Gm-Message-State: AOAM532iR7+C3leOErGqo//fU6EKKr3QRZHGZqqNK2ibkgNy0fwNZ3aV 0cvlyeMMIazdFmTAl0vCATCxbFn66rzTf9xsHajhFRSYkRgHG7+Wpq15h2g+yt8obT7f2jRoT8F x478swcfd4TjVcdAlX07IinSxlOLtTbm45i6TG9w0yhAzGU3QKb2GfqTlW4U354bMJQ== X-Google-Smtp-Source: ABdhPJxrAs5MJHgVF9g3S7/sOT9A09nRk21lotEWX9yh2bugyyqNVJxCVuDXKE/7sVIDvVYHdfA8qA== X-Received: by 2002:a7b:cc11:: with SMTP id f17mr34713695wmh.159.1620230190188; Wed, 05 May 2021 08:56:30 -0700 (PDT) Received: from bucheron-thinkpad.undoers.io (cpc120850-nrwh12-2-0-cust139.4-4.cable.virginm.net. [82.32.180.140]) by smtp.gmail.com with ESMTPSA id q19sm5959563wmc.44.2021.05.05.08.56.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 May 2021 08:56:29 -0700 (PDT) From: Magne Hov 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 Message-Id: <20210505155627.3850386-1-mhov@undo.io> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 May 2021 15:56:33 -0000 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: . 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 , 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 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 * eval.c (expression::evaluate): Check inferior_ptid. gdb/testsuite/ChangeLog: 2021-05-05 Magne Hov * 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 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 . */ + +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