* [PATCH] Fix crash when TUI window creation fails
@ 2020-06-13 20:27 Tom Tromey
2020-06-16 16:03 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2020-06-13 20:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
If a TUI window is written in Python, and if the window construction
function fails, then gdb will crash. This patch fixes the crash.
gdb/ChangeLog
2020-06-13 Tom Tromey <tom@tromey.com>
* python/py-tui.c (tui_py_window::~tui_py_window): Handle case
where m_window==nullptr.
gdb/testsuite/ChangeLog
2020-06-13 Tom Tromey <tom@tromey.com>
* gdb.python/tui-window.py (failwin): New function. Register it
as a TUI window type.
* gdb.python/tui-window.exp: Create new "fail" layout. Test it.
---
gdb/ChangeLog | 5 +++++
gdb/python/py-tui.c | 4 +++-
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.python/tui-window.exp | 3 +++
gdb/testsuite/gdb.python/tui-window.py | 6 ++++++
5 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index ca88f85eb9f..375b7b47acd 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -133,7 +133,9 @@ tui_py_window::~tui_py_window ()
{
gdbpy_enter enter_py (get_current_arch (), current_language);
- if (PyObject_HasAttrString (m_window.get (), "close"))
+ /* This can be null if the Python function failed. */
+ if (m_window != nullptr
+ && PyObject_HasAttrString (m_window.get (), "close"))
{
gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close",
nullptr));
diff --git a/gdb/testsuite/gdb.python/tui-window.exp b/gdb/testsuite/gdb.python/tui-window.exp
index 1a4feebe22b..878d09c1428 100644
--- a/gdb/testsuite/gdb.python/tui-window.exp
+++ b/gdb/testsuite/gdb.python/tui-window.exp
@@ -36,6 +36,7 @@ gdb_test_no_output "source ${remote_python_file}" \
"source ${testfile}.py"
gdb_test_no_output "tui new-layout test test 1 status 0 cmd 1"
+gdb_test_no_output "tui new-layout fail fail 1 status 0 cmd 1"
if {![Term::enter_tui]} {
unsupported "TUI not supported"
@@ -49,3 +50,5 @@ Term::check_contents "Window display" "Test: 0"
Term::resize 51 51
# Remember that a resize request actually does two resizes...
Term::check_contents "Window was updated" "Test: 2"
+
+Term::command "layout fail"
diff --git a/gdb/testsuite/gdb.python/tui-window.py b/gdb/testsuite/gdb.python/tui-window.py
index 4deb585f138..f362b8768ae 100644
--- a/gdb/testsuite/gdb.python/tui-window.py
+++ b/gdb/testsuite/gdb.python/tui-window.py
@@ -35,3 +35,9 @@ class TestWindow:
self.count = self.count + 1
gdb.register_window_type("test", TestWindow)
+
+# A TUI window "constructor" that always fails.
+def failwin(win):
+ raise RuntimeError("Whoops")
+
+gdb.register_window_type("fail", failwin)
--
2.17.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix crash when TUI window creation fails
2020-06-13 20:27 [PATCH] Fix crash when TUI window creation fails Tom Tromey
@ 2020-06-16 16:03 ` Pedro Alves
2020-06-16 20:26 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2020-06-16 16:03 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 6/13/20 9:27 PM, Tom Tromey wrote:
> If a TUI window is written in Python, and if the window construction
> function fails, then gdb will crash. This patch fixes the crash.
>
> gdb/ChangeLog
> 2020-06-13 Tom Tromey <tom@tromey.com>
>
> * python/py-tui.c (tui_py_window::~tui_py_window): Handle case
> where m_window==nullptr.
>
> gdb/testsuite/ChangeLog
> 2020-06-13 Tom Tromey <tom@tromey.com>
>
> * gdb.python/tui-window.py (failwin): New function. Register it
> as a TUI window type.
> * gdb.python/tui-window.exp: Create new "fail" layout. Test it.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/python/py-tui.c | 4 +++-
> gdb/testsuite/ChangeLog | 6 ++++++
> gdb/testsuite/gdb.python/tui-window.exp | 3 +++
> gdb/testsuite/gdb.python/tui-window.py | 6 ++++++
> 5 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
> index ca88f85eb9f..375b7b47acd 100644
> --- a/gdb/python/py-tui.c
> +++ b/gdb/python/py-tui.c
> @@ -133,7 +133,9 @@ tui_py_window::~tui_py_window ()
> {
> gdbpy_enter enter_py (get_current_arch (), current_language);
>
> - if (PyObject_HasAttrString (m_window.get (), "close"))
> + /* This can be null if the Python function failed. */
> + if (m_window != nullptr
> + && PyObject_HasAttrString (m_window.get (), "close"))
Is this "the Python function" referring to this PyObject_CallFunctionObjArgs
call (*)?
std::unique_ptr<tui_py_window> window
(new tui_py_window (win_name, wrapper));
gdbpy_ref<> user_window
(PyObject_CallFunctionObjArgs (m_constr.get (),
(PyObject *) wrapper.get (),
nullptr));
if (user_window == nullptr)
{
gdbpy_print_stack ();
return nullptr;
}
window->set_user_window (std::move (user_window));
Wouldn't it make sense to to move the tui_py_window
allocation until after the Python call instead?
(*) - I think the comment would gain from mentioning
it more specifically, like 'the Python "contructor" function'
or something like that.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix crash when TUI window creation fails
2020-06-16 16:03 ` Pedro Alves
@ 2020-06-16 20:26 ` Tom Tromey
2020-06-16 22:08 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2020-06-16 20:26 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Is this "the Python function" referring to this PyObject_CallFunctionObjArgs
Pedro> call (*)?
Yep.
Pedro> std::unique_ptr<tui_py_window> window
Pedro> (new tui_py_window (win_name, wrapper));
Pedro> gdbpy_ref<> user_window
Pedro> (PyObject_CallFunctionObjArgs (m_constr.get (),
Pedro> (PyObject *) wrapper.get (),
Pedro> nullptr));
Pedro> if (user_window == nullptr)
Pedro> {
Pedro> gdbpy_print_stack ();
Pedro> return nullptr;
Pedro> }
window-> set_user_window (std::move (user_window));
Pedro> Wouldn't it make sense to to move the tui_py_window
Pedro> allocation until after the Python call instead?
No, because the TUI window object is passed to the user's constructor,
and that constructor can write text there, etc.
Pedro> (*) - I think the comment would gain from mentioning
Pedro> it more specifically, like 'the Python "contructor" function'
Pedro> or something like that.
I'll update it.
Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix crash when TUI window creation fails
2020-06-16 20:26 ` Tom Tromey
@ 2020-06-16 22:08 ` Pedro Alves
0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2020-06-16 22:08 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 6/16/20 9:26 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> Is this "the Python function" referring to this PyObject_CallFunctionObjArgs
> Pedro> call (*)?
>
> Yep.
>
> Pedro> std::unique_ptr<tui_py_window> window
> Pedro> (new tui_py_window (win_name, wrapper));
>
> Pedro> gdbpy_ref<> user_window
> Pedro> (PyObject_CallFunctionObjArgs (m_constr.get (),
> Pedro> (PyObject *) wrapper.get (),
> Pedro> nullptr));
> Pedro> if (user_window == nullptr)
> Pedro> {
> Pedro> gdbpy_print_stack ();
> Pedro> return nullptr;
> Pedro> }
>
> window-> set_user_window (std::move (user_window));
>
> Pedro> Wouldn't it make sense to to move the tui_py_window
> Pedro> allocation until after the Python call instead?
>
> No, because the TUI window object is passed to the user's constructor,
> and that constructor can write text there, etc.
>
> Pedro> (*) - I think the comment would gain from mentioning
> Pedro> it more specifically, like 'the Python "contructor" function'
> Pedro> or something like that.
>
> I'll update it.
Thanks for the clarification. Feel free to merge with that
update then.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-16 22:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-13 20:27 [PATCH] Fix crash when TUI window creation fails Tom Tromey
2020-06-16 16:03 ` Pedro Alves
2020-06-16 20:26 ` Tom Tromey
2020-06-16 22:08 ` Pedro Alves
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).