[gdb/python] Eliminate catch(...) in type_to_type_object

Message ID 20240821150654.27255-1-tdevries@suse.de
State Committed
Headers
Series [gdb/python] Eliminate catch(...) in type_to_type_object |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Aug. 21, 2024, 3:06 p.m. UTC
  In type_to_type_object we have:
...
  try
    {
      if (type->is_stub ())
	type = check_typedef (type);
    }
  catch (...)
    {
      /* Just ignore failures in check_typedef.  */
    }
...

The catch is supposed to catch gdb_exception_error, but it catches any
exception.

Fix that by only ignoring gdb_exception_error.

Also handle gdb_exception_quit / gdb_exception_forced_quit.  I'm not sure if
they can occur here, but handling them is trivial using
GDB_PY_HANDLE_EXCEPTION, so why not?

Tested on x86_64-linux.
---
 gdb/python/py-type.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)


base-commit: 28c3bf55f0f9aca8619c6d01be34a02a887c5577
  

Comments

Tom Tromey Aug. 21, 2024, 5:03 p.m. UTC | #1
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The catch is supposed to catch gdb_exception_error, but it catches any
Tom> exception.

Tom> Fix that by only ignoring gdb_exception_error.

Tom> Also handle gdb_exception_quit / gdb_exception_forced_quit.  I'm not sure if
Tom> they can occur here, but handling them is trivial using
Tom> GDB_PY_HANDLE_EXCEPTION, so why not?

I think this is the right approach -- if a QUIT does happen in
check_typedef, and it propagates past here, it could cause a crash.

I suspect a failure like this is indeed possible because check_typedef
might result in CU expansion, which is maybe interruptible (or anyway we
want it to be someday).

Approved-By: Tom Tromey <tom@tromey.com>

Tom
  
Tom de Vries Aug. 22, 2024, 7:51 a.m. UTC | #2
On 8/21/24 19:03, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> The catch is supposed to catch gdb_exception_error, but it catches any
> Tom> exception.
> 
> Tom> Fix that by only ignoring gdb_exception_error.
> 
> Tom> Also handle gdb_exception_quit / gdb_exception_forced_quit.  I'm not sure if
> Tom> they can occur here, but handling them is trivial using
> Tom> GDB_PY_HANDLE_EXCEPTION, so why not?
> 
> I think this is the right approach -- if a QUIT does happen in
> check_typedef, and it propagates past here, it could cause a crash.
> 
> I suspect a failure like this is indeed possible because check_typedef
> might result in CU expansion, which is maybe interruptible (or anyway we
> want it to be someday).
> 

OK, thanks for the confirmation, I've made the commit message less 
tentative, and pushed.

Thanks,
- Tom
  

Patch

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 863e6f6175f..c13b8610d37 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1465,10 +1465,14 @@  type_to_type_object (struct type *type)
       if (type->is_stub ())
 	type = check_typedef (type);
     }
-  catch (...)
+  catch (const gdb_exception_error &)
     {
       /* Just ignore failures in check_typedef.  */
     }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
 
   type_obj = PyObject_New (type_object, &type_object_type);
   if (type_obj)