This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail
- From: Tom Tromey <tromey at redhat dot com>
- To: Hui Zhu <hui_zhu at mentor dot com>
- Cc: gdb-patches ml <gdb-patches at sourceware dot org>, Phil Muldoon <pmuldoon at redhat dot com>
- Date: Tue, 14 Jan 2014 10:41:38 -0700
- Subject: Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail
- Authentication-results: sourceware.org; auth=none
- References: <52974146 dot 70805 at mentor dot com> <8761r7w85h dot fsf at fleche dot redhat dot com> <529D8865 dot 80503 at mentor dot com> <87li01smua dot fsf at fleche dot redhat dot com> <529EDCA7 dot 1090903 at mentor dot com> <87eh5j2vw0 dot fsf at fleche dot redhat dot com> <52AAF43B dot 5060603 at mentor dot com> <87vbxmwnn3 dot fsf at fleche dot redhat dot com>
>>>>> "Hui" == Tom Tromey <tromey@redhat.com> writes:
Hui> With this patch, GDB will output a lot of "Python not initialized"
Hui> that output by function "ensure_python_env".
Hui> For example:
Hui> (gdb) start
Hui> Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Hui> Starting program: /home/teawater/tmp/2
Hui> Python not initialized
Tom> I think this may be an independent bug.
Tom> I'll have a better patch soon.
Ok, I have looked into the problem some more.
I'm sorry this has been taking so long.
I've appended my current patch. It arranges to clear
gdb_python_initialized if the second stage of initialization fails.
And, it adds a check on gdb_python_initialized in a couple of spots that
we missed before.
However, it still isn't quite right, due to this code in
finish_python_initialization:
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.\n"),
gdb_pythondir);
do_cleanups (cleanup);
return;
}
The question is -- what should we really do here?
One option is to set gdb_python_initialized = 0.
I think this makes the warning text obsolete, though, since the
"limited" support would not actually be available at all (e.g., the
"python" command wouldn't work at all).
Another option is to try to split the initialization global. One of
your patches in this thread did that. It wasn't clear to me that your
patch went far enough, though -- it changed some spots to check the new
flag, but not every spot. This approach would seem to involve auditing
all uses of gdb_python_initialized and deciding (how?) whether or not
the particular spot needs "full" initialization.
I lean toward the simpler solution of changing the warning message and
clearing the flag.
Tom
2014-01-14 Tom Tromey <tromey@redhat.com>
* python/py-finishbreakpoint.c (bpfinishpy_handle_stop): Check
gdb_python_initialized.
(bpfinishpy_handle_exit): Likewise.
* python/python.c (_initialize_python): Don't release the GIL.
(release_gil): New function.
(finish_python_initialization): Use release_gil. Don't call
ensure_python_env. Possibly clear gdb_python_initialized.
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 712a9ee..4e052d7 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -382,8 +382,12 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
static void
bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
{
- struct cleanup *cleanup = ensure_python_env (get_current_arch (),
- current_language);
+ struct cleanup *cleanup;
+
+ if (!gdb_python_initialized)
+ return;
+
+ cleanup = ensure_python_env (get_current_arch (), current_language);
iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb,
bs == NULL ? NULL : bs->breakpoint_at);
@@ -397,8 +401,12 @@ bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
static void
bpfinishpy_handle_exit (struct inferior *inf)
{
- struct cleanup *cleanup = ensure_python_env (target_gdbarch (),
- current_language);
+ struct cleanup *cleanup;
+
+ if (!gdb_python_initialized)
+ return;
+
+ cleanup = ensure_python_env (target_gdbarch (), current_language);
iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb, NULL);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 337c170..8a6389f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1712,10 +1712,6 @@ message == an error message without a stack will be printed."),
if (gdbpy_value_cst == NULL)
goto fail;
- /* Release the GIL while gdb runs. */
- PyThreadState_Swap (NULL);
- PyEval_ReleaseLock ();
-
make_final_cleanup (finalize_python, NULL);
gdb_python_initialized = 1;
@@ -1731,6 +1727,15 @@ message == an error message without a stack will be printed."),
#ifdef HAVE_PYTHON
+/* A cleanup function that releases the GIL. */
+
+static void
+release_gil (void *ignore)
+{
+ PyThreadState_Swap (NULL);
+ PyEval_ReleaseLock ();
+}
+
/* Perform the remaining python initializations.
These must be done after GDB is at least mostly initialized.
E.g., The "info pretty-printer" command needs the "info" prefix
@@ -1744,7 +1749,13 @@ finish_python_initialization (void)
PyObject *sys_path;
struct cleanup *cleanup;
- cleanup = ensure_python_env (get_current_arch (), current_language);
+ /* If the first phase of initialization failed, don't bother doing
+ anything here. */
+ if (!gdb_python_initialized)
+ return;
+
+ /* Release the GIL while gdb runs. */
+ cleanup = make_cleanup (release_gil, NULL);
/* Add the initial data-directory to sys.path. */
@@ -1814,6 +1825,7 @@ finish_python_initialization (void)
gdbpy_print_stack ();
warning (_("internal error: Unhandled Python exception"));
do_cleanups (cleanup);
+ gdb_python_initialized = 0;
}
#endif /* HAVE_PYTHON */