* [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo
@ 2023-11-15 13:52 Tom Tromey
2023-11-15 15:47 ` John Baldwin
2023-11-16 10:18 ` Andrew Burgess
0 siblings, 2 replies; 5+ messages in thread
From: Tom Tromey @ 2023-11-15 13:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
If you run gdb in the build tree without --data-directory, on a
program that does not have debug info, it will crash, because
gdbpy_handle_missing_debuginfo unconditionally uses gdb_python_module.
Other code in gdb using gdb_python_module checks it first. I'm not
really sure why gdb_python_initialized does not cover this case (maybe
so that python can be partially initialized and still somewhat work?),
but it seemes harmless to do the same thing here.
---
gdb/python/python.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4523e30ed24..7e48165db21 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1703,7 +1703,7 @@ gdbpy_handle_missing_debuginfo (const struct extension_language_defn *extlang,
struct objfile *objfile)
{
/* Early exit if Python is not initialised. */
- if (!gdb_python_initialized)
+ if (!gdb_python_initialized || gdb_python_module == nullptr)
return {};
struct gdbarch *gdbarch = objfile->arch ();
--
2.41.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo
2023-11-15 13:52 [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo Tom Tromey
@ 2023-11-15 15:47 ` John Baldwin
2023-11-15 16:38 ` Tom Tromey
2023-11-16 10:18 ` Andrew Burgess
1 sibling, 1 reply; 5+ messages in thread
From: John Baldwin @ 2023-11-15 15:47 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 11/15/23 5:52 AM, Tom Tromey wrote:
> If you run gdb in the build tree without --data-directory, on a
> program that does not have debug info, it will crash, because
> gdbpy_handle_missing_debuginfo unconditionally uses gdb_python_module.
>
> Other code in gdb using gdb_python_module checks it first. I'm not
> really sure why gdb_python_initialized does not cover this case (maybe
> so that python can be partially initialized and still somewhat work?),
> but it seemes harmless to do the same thing here.
> ---
> gdb/python/python.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 4523e30ed24..7e48165db21 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1703,7 +1703,7 @@ gdbpy_handle_missing_debuginfo (const struct extension_language_defn *extlang,
> struct objfile *objfile)
> {
> /* Early exit if Python is not initialised. */
> - if (!gdb_python_initialized)
> + if (!gdb_python_initialized || gdb_python_module == nullptr)
> return {};
>
> struct gdbarch *gdbarch = objfile->arch ();
Looks good to me. I suspect you can only check gdb_python_module == nullptr
here and assume that if it is non-null, gdb_python_initialized is also non-zero
as we set gdb_python_module in do_initialize() IFF do_start_initialize()
(which sets gdb_python_initialized) succeeds.
The error in do_initialize when the gdb module fails to load does seem to
explain why this condition exists and lines up with what you guess in the
commit log:
gdb_python_module = PyImport_ImportModule ("gdb");
if (gdb_python_module == NULL)
{
gdbpy_print_stack ();
/* This is passed in one call to warning so that blank lines aren't
inserted between each line of text. */
warning (_("\n"
"Could not load the Python gdb module from `%s'.\n"
"Limited Python support is available from the _gdb module.\n"
"Suggest passing --data-directory=/path/to/gdb/data-directory."),
gdb_pythondir.c_str ());
/* We return "success" here as we've already emitted the
warning. */
return true;
}
--
John Baldwin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo
2023-11-15 15:47 ` John Baldwin
@ 2023-11-15 16:38 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2023-11-15 16:38 UTC (permalink / raw)
To: John Baldwin; +Cc: Tom Tromey, gdb-patches
>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
John> Looks good to me. I suspect you can only check gdb_python_module == nullptr
John> here and assume that if it is non-null, gdb_python_initialized is also non-zero
John> as we set gdb_python_module in do_initialize() IFF do_start_initialize()
John> (which sets gdb_python_initialized) succeeds.
Yeah. I think you're right, but other spots also check
gdb_python_initialized, so I think we might as well leave it as-is.
John> The error in do_initialize when the gdb module fails to load does seem to
John> explain why this condition exists and lines up with what you guess in the
John> commit log:
I'll update the commit message to reflect this.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo
2023-11-15 13:52 [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo Tom Tromey
2023-11-15 15:47 ` John Baldwin
@ 2023-11-16 10:18 ` Andrew Burgess
2023-11-16 15:14 ` Tom Tromey
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2023-11-16 10:18 UTC (permalink / raw)
To: Tom Tromey, gdb-patches; +Cc: Tom Tromey
Tom Tromey <tromey@adacore.com> writes:
> If you run gdb in the build tree without --data-directory, on a
> program that does not have debug info, it will crash, because
> gdbpy_handle_missing_debuginfo unconditionally uses gdb_python_module.
>
> Other code in gdb using gdb_python_module checks it first. I'm not
> really sure why gdb_python_initialized does not cover this case (maybe
> so that python can be partially initialized and still somewhat work?),
> but it seemes harmless to do the same thing here.
Gah. I was dropping the ball all over the place with those patches.
Thanks for fixing this (with whatever updated commit message you use).
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
> ---
> gdb/python/python.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 4523e30ed24..7e48165db21 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1703,7 +1703,7 @@ gdbpy_handle_missing_debuginfo (const struct extension_language_defn *extlang,
> struct objfile *objfile)
> {
> /* Early exit if Python is not initialised. */
> - if (!gdb_python_initialized)
> + if (!gdb_python_initialized || gdb_python_module == nullptr)
> return {};
>
> struct gdbarch *gdbarch = objfile->arch ();
> --
> 2.41.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo
2023-11-16 10:18 ` Andrew Burgess
@ 2023-11-16 15:14 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2023-11-16 15:14 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> Gah. I was dropping the ball all over the place with those patches.
Don't worry about it, and especially don't feel bad about this case,
which is a weird corner case that only affects gdb developers and people
who messed up their gdb install.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-16 15:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-15 13:52 [PATCH] Check gdb_python_module in gdbpy_handle_missing_debuginfo Tom Tromey
2023-11-15 15:47 ` John Baldwin
2023-11-15 16:38 ` Tom Tromey
2023-11-16 10:18 ` Andrew Burgess
2023-11-16 15:14 ` Tom Tromey
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).