* [python] [patch] PR 13369/13374
@ 2011-11-21 11:42 Phil Muldoon
2011-11-21 11:50 ` Paul_Koning
2011-11-21 20:09 ` Tom Tromey
0 siblings, 2 replies; 7+ messages in thread
From: Phil Muldoon @ 2011-11-21 11:42 UTC (permalink / raw)
To: gdb-patches
This patch address two different bugs. There are some places where we
acquire the GIL where it is not needed. Also, in a similar vein we do
not properly handle some GDB exceptions.
OK?
Cheers,
Phil
--
2011-11-21 Phil Muldoon <pmuldoon@redhat.com>
PR python/13369
PR python 13374
* python/python.c (gdbpy_decode_line): Do not acquire GIL.
* python/py-inferior.c (inferior_to_inferior_object): Ditto.
* python/py-value.c (valpy_nonzero): Use TRY_CATCH to catch GDB
exceptions.
* python/py-type.c (typy_strip_typedefs): Ditto.
(typy_legacy_template_argument): Ditto.
* python/py-inferior.c (inferior_to_inferior_object): Ditto.
* python/py-breakpoint.c (bppy_set_ignore_count): Ditto.
--
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 3f4467a..6c6df2e 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -371,14 +371,20 @@ gdbpy_block_for_pc (PyObject *self, PyObject *args)
{
gdb_py_ulongest pc;
struct block *block;
- struct obj_section *section;
- struct symtab *symtab;
+ struct obj_section *section = NULL;
+ struct symtab *symtab = NULL;
+ volatile struct gdb_exception except;
if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
return NULL;
- section = find_pc_mapped_section (pc);
- symtab = find_pc_sect_symtab (pc, section);
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ section = find_pc_mapped_section (pc);
+ symtab = find_pc_sect_symtab (pc, section);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
+
if (!symtab || symtab->objfile == NULL)
{
PyErr_SetString (PyExc_RuntimeError,
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index f235bbc..11d60fe 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -334,6 +334,7 @@ bppy_set_ignore_count (PyObject *self, PyObject *newvalue, void *closure)
{
breakpoint_object *self_bp = (breakpoint_object *) self;
long value;
+ volatile struct gdb_exception except;
BPPY_SET_REQUIRE_VALID (self_bp);
@@ -355,7 +356,12 @@ bppy_set_ignore_count (PyObject *self, PyObject *newvalue, void *closure)
if (value < 0)
value = 0;
- set_ignore_count (self_bp->number, (int) value, 0);
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ set_ignore_count (self_bp->number, (int) value, 0);
+ }
+ GDB_PY_SET_HANDLE_EXCEPTION (except);
return 0;
}
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index d6086dc..be31f5d 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -157,15 +157,9 @@ inferior_to_inferior_object (struct inferior *inferior)
inf_obj = inferior_data (inferior, infpy_inf_data_key);
if (!inf_obj)
{
- struct cleanup *cleanup;
- cleanup = ensure_python_env (python_gdbarch, python_language);
-
inf_obj = PyObject_New (inferior_object, &inferior_object_type);
if (!inf_obj)
- {
- do_cleanups (cleanup);
return NULL;
- }
inf_obj->inferior = inferior;
inf_obj->threads = NULL;
@@ -173,7 +167,6 @@ inferior_to_inferior_object (struct inferior *inferior)
set_inferior_data (inferior, infpy_inf_data_key, inf_obj);
- do_cleanups (cleanup);
}
else
Py_INCREF ((PyObject *)inf_obj);
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index f76b1c7..b671cef 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -395,6 +395,13 @@ static PyObject *
typy_strip_typedefs (PyObject *self, PyObject *args)
{
struct type *type = ((type_object *) self)->type;
+ volatile struct gdb_exception except;
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ type = check_typedef (type);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
return type_to_type_object (check_typedef (type));
}
@@ -768,10 +775,11 @@ typy_legacy_template_argument (struct type *type, const struct block *block,
{
int i;
struct demangle_component *demangled;
- struct demangle_parse_info *info;
+ struct demangle_parse_info *info = NULL;
const char *err;
struct type *argtype;
struct cleanup *cleanup;
+ volatile struct gdb_exception except;
if (TYPE_NAME (type) == NULL)
{
@@ -779,8 +787,13 @@ typy_legacy_template_argument (struct type *type, const struct block *block,
return NULL;
}
- /* Note -- this is not thread-safe. */
- info = cp_demangled_name_to_comp (TYPE_NAME (type), &err);
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ /* Note -- this is not thread-safe. */
+ info = cp_demangled_name_to_comp (TYPE_NAME (type), &err);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
+
if (! info)
{
PyErr_SetString (PyExc_RuntimeError, err);
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 15c183a..05e592f 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -880,10 +880,10 @@ valpy_nonzero (PyObject *self)
struct type *type;
int nonzero = 0; /* Appease GCC warning. */
- type = check_typedef (value_type (self_value->value));
-
TRY_CATCH (except, RETURN_MASK_ALL)
{
+ type = check_typedef (value_type (self_value->value));
+
if (is_integral_type (type) || TYPE_CODE (type) == TYPE_CODE_PTR)
nonzero = !!value_as_long (self_value->value);
else if (TYPE_CODE (type) == TYPE_CODE_FLT)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 108e542..6022572 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -504,7 +504,7 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
if (! PyArg_ParseTuple (args, "|s", &arg))
return NULL;
- cleanups = ensure_python_env (get_current_arch (), current_language);
+ cleanups = make_cleanup (null_cleanup, NULL);
TRY_CATCH (except, RETURN_MASK_ALL)
{
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [python] [patch] PR 13369/13374 2011-11-21 11:42 [python] [patch] PR 13369/13374 Phil Muldoon @ 2011-11-21 11:50 ` Paul_Koning 2011-11-21 19:59 ` Tom Tromey 2011-11-21 20:09 ` Tom Tromey 1 sibling, 1 reply; 7+ messages in thread From: Paul_Koning @ 2011-11-21 11:50 UTC (permalink / raw) To: pmuldoon, gdb-patches >... >diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c >index f76b1c7..b671cef 100644 >--- a/gdb/python/py-type.c >+++ b/gdb/python/py-type.c >... >@@ -779,8 +787,13 @@ typy_legacy_template_argument (struct type *type, const struct block >*block, > return NULL; > } > >- /* Note -- this is not thread-safe. */ >- info = cp_demangled_name_to_comp (TYPE_NAME (type), &err); >+ TRY_CATCH (except, RETURN_MASK_ALL) >+ { >+ /* Note -- this is not thread-safe. */ Does the comment "not thread safe" indicate that this is a spot where the GIL should be acquired and isn't currently? Or doesn't the GIL help for that? paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [python] [patch] PR 13369/13374 2011-11-21 11:50 ` Paul_Koning @ 2011-11-21 19:59 ` Tom Tromey 0 siblings, 0 replies; 7+ messages in thread From: Tom Tromey @ 2011-11-21 19:59 UTC (permalink / raw) To: Paul_Koning; +Cc: pmuldoon, gdb-patches >>>>> "Paul" == <Paul_Koning@Dell.com> writes: >> + /* Note -- this is not thread-safe. */ Paul> Does the comment "not thread safe" indicate that this is a spot where Paul> the GIL should be acquired and isn't currently? Or doesn't the GIL Paul> help for that? The comment refers to the code being called here. I think the comment can be removed. It isn't ok to call gdb stuff from other Python threads anyway. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [python] [patch] PR 13369/13374 2011-11-21 11:42 [python] [patch] PR 13369/13374 Phil Muldoon 2011-11-21 11:50 ` Paul_Koning @ 2011-11-21 20:09 ` Tom Tromey 2011-11-22 12:16 ` Phil Muldoon 1 sibling, 1 reply; 7+ messages in thread From: Tom Tromey @ 2011-11-21 20:09 UTC (permalink / raw) To: pmuldoon; +Cc: gdb-patches >>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: Phil> @@ -157,15 +157,9 @@ inferior_to_inferior_object (struct inferior *inferior) Phil> inf_obj = inferior_data (inferior, infpy_inf_data_key); Phil> if (!inf_obj) Phil> { Phil> - struct cleanup *cleanup; Phil> - cleanup = ensure_python_env (python_gdbarch, python_language); Phil> - Phil> inf_obj = PyObject_New (inferior_object, &inferior_object_type); Phil> if (!inf_obj) Phil> - { Phil> - do_cleanups (cleanup); Phil> return NULL; Phil> - } This doesn't handle the call chain delete_thread_object -> find_inferior_object -> inferior_to_inferior_object Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [python] [patch] PR 13369/13374 2011-11-21 20:09 ` Tom Tromey @ 2011-11-22 12:16 ` Phil Muldoon 2011-11-22 18:01 ` Tom Tromey 0 siblings, 1 reply; 7+ messages in thread From: Phil Muldoon @ 2011-11-22 12:16 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Tom Tromey <tromey@redhat.com> writes: >>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: > > Phil> @@ -157,15 +157,9 @@ inferior_to_inferior_object (struct inferior *inferior) > Phil> inf_obj = inferior_data (inferior, infpy_inf_data_key); > Phil> if (!inf_obj) > Phil> { > Phil> - struct cleanup *cleanup; > Phil> - cleanup = ensure_python_env (python_gdbarch, python_language); > Phil> - > Phil> inf_obj = PyObject_New (inferior_object, &inferior_object_type); > Phil> if (!inf_obj) > Phil> - { > Phil> - do_cleanups (cleanup); > Phil> return NULL; > Phil> - } > > This doesn't handle the call chain > > delete_thread_object -> find_inferior_object -> inferior_to_inferior_object I see what I forgot. I forgot to move the late GIL acquisition up too a little earlier in the function body. Thanks for spotting! Cheers, Phil -- diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c index 3f4467a..6c6df2e 100644 --- a/gdb/python/py-block.c +++ b/gdb/python/py-block.c @@ -371,14 +371,20 @@ gdbpy_block_for_pc (PyObject *self, PyObject *args) { gdb_py_ulongest pc; struct block *block; - struct obj_section *section; - struct symtab *symtab; + struct obj_section *section = NULL; + struct symtab *symtab = NULL; + volatile struct gdb_exception except; if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc)) return NULL; - section = find_pc_mapped_section (pc); - symtab = find_pc_sect_symtab (pc, section); + TRY_CATCH (except, RETURN_MASK_ALL) + { + section = find_pc_mapped_section (pc); + symtab = find_pc_sect_symtab (pc, section); + } + GDB_PY_HANDLE_EXCEPTION (except); + if (!symtab || symtab->objfile == NULL) { PyErr_SetString (PyExc_RuntimeError, diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index f235bbc..11d60fe 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -334,6 +334,7 @@ bppy_set_ignore_count (PyObject *self, PyObject *newvalue, void *closure) { breakpoint_object *self_bp = (breakpoint_object *) self; long value; + volatile struct gdb_exception except; BPPY_SET_REQUIRE_VALID (self_bp); @@ -355,7 +356,12 @@ bppy_set_ignore_count (PyObject *self, PyObject *newvalue, void *closure) if (value < 0) value = 0; - set_ignore_count (self_bp->number, (int) value, 0); + + TRY_CATCH (except, RETURN_MASK_ALL) + { + set_ignore_count (self_bp->number, (int) value, 0); + } + GDB_PY_SET_HANDLE_EXCEPTION (except); return 0; } diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c index d6086dc..7b2981e 100644 --- a/gdb/python/py-inferior.c +++ b/gdb/python/py-inferior.c @@ -157,15 +157,9 @@ inferior_to_inferior_object (struct inferior *inferior) inf_obj = inferior_data (inferior, infpy_inf_data_key); if (!inf_obj) { - struct cleanup *cleanup; - cleanup = ensure_python_env (python_gdbarch, python_language); - inf_obj = PyObject_New (inferior_object, &inferior_object_type); if (!inf_obj) - { - do_cleanups (cleanup); return NULL; - } inf_obj->inferior = inferior; inf_obj->threads = NULL; @@ -173,7 +167,6 @@ inferior_to_inferior_object (struct inferior *inferior) set_inferior_data (inferior, infpy_inf_data_key, inf_obj); - do_cleanups (cleanup); } else Py_INCREF ((PyObject *)inf_obj); @@ -266,10 +259,15 @@ delete_thread_object (struct thread_info *tp, int ignore) inferior_object *inf_obj; thread_object *thread_obj; struct threadlist_entry **entry, *tmp; + + cleanup = ensure_python_env (python_gdbarch, python_language); inf_obj = (inferior_object *) find_inferior_object (PIDGET(tp->ptid)); if (!inf_obj) - return; + { + do_cleanups (cleanup); + return; + } /* Find thread entry in its inferior's thread_list. */ for (entry = &inf_obj->threads; *entry != NULL; entry = @@ -280,11 +278,10 @@ delete_thread_object (struct thread_info *tp, int ignore) if (!*entry) { Py_DECREF (inf_obj); + do_cleanups (cleanup); return; } - cleanup = ensure_python_env (python_gdbarch, python_language); - tmp = *entry; tmp->thread_obj->thread = NULL; diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c index f76b1c7..b671cef 100644 --- a/gdb/python/py-type.c +++ b/gdb/python/py-type.c @@ -395,6 +395,13 @@ static PyObject * typy_strip_typedefs (PyObject *self, PyObject *args) { struct type *type = ((type_object *) self)->type; + volatile struct gdb_exception except; + + TRY_CATCH (except, RETURN_MASK_ALL) + { + type = check_typedef (type); + } + GDB_PY_HANDLE_EXCEPTION (except); return type_to_type_object (check_typedef (type)); } @@ -768,10 +775,11 @@ typy_legacy_template_argument (struct type *type, const struct block *block, { int i; struct demangle_component *demangled; - struct demangle_parse_info *info; + struct demangle_parse_info *info = NULL; const char *err; struct type *argtype; struct cleanup *cleanup; + volatile struct gdb_exception except; if (TYPE_NAME (type) == NULL) { @@ -779,8 +787,13 @@ typy_legacy_template_argument (struct type *type, const struct block *block, return NULL; } - /* Note -- this is not thread-safe. */ - info = cp_demangled_name_to_comp (TYPE_NAME (type), &err); + TRY_CATCH (except, RETURN_MASK_ALL) + { + /* Note -- this is not thread-safe. */ + info = cp_demangled_name_to_comp (TYPE_NAME (type), &err); + } + GDB_PY_HANDLE_EXCEPTION (except); + if (! info) { PyErr_SetString (PyExc_RuntimeError, err); diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c index 15c183a..05e592f 100644 --- a/gdb/python/py-value.c +++ b/gdb/python/py-value.c @@ -880,10 +880,10 @@ valpy_nonzero (PyObject *self) struct type *type; int nonzero = 0; /* Appease GCC warning. */ - type = check_typedef (value_type (self_value->value)); - TRY_CATCH (except, RETURN_MASK_ALL) { + type = check_typedef (value_type (self_value->value)); + if (is_integral_type (type) || TYPE_CODE (type) == TYPE_CODE_PTR) nonzero = !!value_as_long (self_value->value); else if (TYPE_CODE (type) == TYPE_CODE_FLT) diff --git a/gdb/python/python.c b/gdb/python/python.c index 108e542..6022572 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -504,7 +504,7 @@ gdbpy_decode_line (PyObject *self, PyObject *args) if (! PyArg_ParseTuple (args, "|s", &arg)) return NULL; - cleanups = ensure_python_env (get_current_arch (), current_language); + cleanups = make_cleanup (null_cleanup, NULL); TRY_CATCH (except, RETURN_MASK_ALL) { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [python] [patch] PR 13369/13374 2011-11-22 12:16 ` Phil Muldoon @ 2011-11-22 18:01 ` Tom Tromey 2011-11-28 16:02 ` Phil Muldoon 0 siblings, 1 reply; 7+ messages in thread From: Tom Tromey @ 2011-11-22 18:01 UTC (permalink / raw) To: pmuldoon; +Cc: gdb-patches >>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: Phil> I see what I forgot. I forgot to move the late GIL acquisition up too a Phil> little earlier in the function body. Thanks for spotting! This is ok. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [python] [patch] PR 13369/13374 2011-11-22 18:01 ` Tom Tromey @ 2011-11-28 16:02 ` Phil Muldoon 0 siblings, 0 replies; 7+ messages in thread From: Phil Muldoon @ 2011-11-28 16:02 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Tom Tromey <tromey@redhat.com> writes: >>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: > > Phil> I see what I forgot. I forgot to move the late GIL acquisition up too a > Phil> little earlier in the function body. Thanks for spotting! > > This is ok. So committed, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-11-28 16:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-11-21 11:42 [python] [patch] PR 13369/13374 Phil Muldoon 2011-11-21 11:50 ` Paul_Koning 2011-11-21 19:59 ` Tom Tromey 2011-11-21 20:09 ` Tom Tromey 2011-11-22 12:16 ` Phil Muldoon 2011-11-22 18:01 ` Tom Tromey 2011-11-28 16:02 ` Phil Muldoon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox