diff mbox

Catch exception in value_rtti_indirect_type

Message ID 1411174953-8930-1-git-send-email-simon.marchi@ericsson.com
State Superseded
Headers

Commit Message

Simon Marchi Sept. 20, 2014, 1:02 a.m. UTC 
  In the situation described in bug 17416 [1], an exception thrown in
value_ind can propagate too far and leave an half-built variable object,
leading to a wrong state. This patch adds a TRY_CATCH to catch it and
makes value_rtti_indirect_type return NULL in that case, meaning
that the type of the pointed object could not be found.

If you want, I can also integrate the test case provided in the bug
description. I just don't know how to name it without giving it a
ridiculously long name such as
mi-var-list-children-with-print-object-on-and-a-null-pointer-to-a-structure-that-contains-a-pointer-to-a-structure.exp.

I tested the change on my machine, Ubuntu 14.10 x86-64.

gdb/Changelog:

	* valops.c (value_rtti_indirect_type): Catch exception thrown by
	value_ind.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=17416
---
 gdb/valops.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Simon Marchi Oct. 20, 2014, 5:50 p.m. UTC | #1 
On 2014-09-19 09:02 PM, Simon Marchi wrote:
> In the situation described in bug 17416 [1], an exception thrown in
> value_ind can propagate too far and leave an half-built variable object,
> leading to a wrong state. This patch adds a TRY_CATCH to catch it and
> makes value_rtti_indirect_type return NULL in that case, meaning
> that the type of the pointed object could not be found.
> 
> If you want, I can also integrate the test case provided in the bug
> description. I just don't know how to name it without giving it a
> ridiculously long name such as
> mi-var-list-children-with-print-object-on-and-a-null-pointer-to-a-structure-that-contains-a-pointer-to-a-structure.exp.
> 
> I tested the change on my machine, Ubuntu 14.10 x86-64.
> 
> gdb/Changelog:
> 
> 	* valops.c (value_rtti_indirect_type): Catch exception thrown by
> 	value_ind.
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=17416
> ---
>  gdb/valops.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/valops.c b/gdb/valops.c
> index e1decf0..c1a0c86 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -3609,7 +3609,18 @@ value_rtti_indirect_type (struct value *v, int *full,
>    if (TYPE_CODE (type) == TYPE_CODE_REF)
>      target = coerce_ref (v);
>    else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -    target = value_ind (v);
> +    {
> +      volatile struct gdb_exception except;
> +
> +      TRY_CATCH (except, RETURN_MASK_ERROR)
> +        {
> +	  target = value_ind (v);
> +        }
> +      if (except.error == MEMORY_ERROR)
> +	  return NULL;
> +      else if (except.error != GDB_NO_ERROR)
> +	throw_exception (except);
> +    }
>    else
>      return NULL;

Ping
Simon Marchi Dec. 1, 2014, 1:35 p.m. UTC | #2 
On 2014-10-20 01:50 PM, Simon Marchi wrote:
> On 2014-09-19 09:02 PM, Simon Marchi wrote:
>> In the situation described in bug 17416 [1], an exception thrown in
>> value_ind can propagate too far and leave an half-built variable object,
>> leading to a wrong state. This patch adds a TRY_CATCH to catch it and
>> makes value_rtti_indirect_type return NULL in that case, meaning
>> that the type of the pointed object could not be found.
>>
>> If you want, I can also integrate the test case provided in the bug
>> description. I just don't know how to name it without giving it a
>> ridiculously long name such as
>> mi-var-list-children-with-print-object-on-and-a-null-pointer-to-a-structure-that-contains-a-pointer-to-a-structure.exp.
>>
>> I tested the change on my machine, Ubuntu 14.10 x86-64.
>>
>> gdb/Changelog:
>>
>> 	* valops.c (value_rtti_indirect_type): Catch exception thrown by
>> 	value_ind.
>>
>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=17416
>> ---
>>  gdb/valops.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/valops.c b/gdb/valops.c
>> index e1decf0..c1a0c86 100644
>> --- a/gdb/valops.c
>> +++ b/gdb/valops.c
>> @@ -3609,7 +3609,18 @@ value_rtti_indirect_type (struct value *v, int *full,
>>    if (TYPE_CODE (type) == TYPE_CODE_REF)
>>      target = coerce_ref (v);
>>    else if (TYPE_CODE (type) == TYPE_CODE_PTR)
>> -    target = value_ind (v);
>> +    {
>> +      volatile struct gdb_exception except;
>> +
>> +      TRY_CATCH (except, RETURN_MASK_ERROR)
>> +        {
>> +	  target = value_ind (v);
>> +        }
>> +      if (except.error == MEMORY_ERROR)
>> +	  return NULL;
>> +      else if (except.error != GDB_NO_ERROR)
>> +	throw_exception (except);
>> +    }
>>    else
>>      return NULL;
> 
> Ping
> 

Ping.
Pedro Alves Dec. 4, 2014, 5:47 p.m. UTC | #3 
On 09/20/2014 02:02 AM, Simon Marchi wrote:
> In the situation described in bug 17416 [1], 

Could you paste that in the commit log please?

> an exception thrown in
> value_ind can propagate too far and leave an half-built variable object,
> leading to a wrong state. This patch adds a TRY_CATCH to catch it and
> makes value_rtti_indirect_type return NULL in that case, meaning
> that the type of the pointed object could not be found.
> 
> If you want, I can also integrate the test case provided in the bug

Yes please.

> description. I just don't know how to name it without giving it a
> ridiculously long name such as
> mi-var-list-children-with-print-object-on-and-a-null-pointer-to-a-structure-that-contains-a-pointer-to-a-structure.exp.

Perhaps mi-var-list-children-invalid-grandchild.exp.

> 
> I tested the change on my machine, Ubuntu 14.10 x86-64.
> 
> gdb/Changelog:
> 
> 	* valops.c (value_rtti_indirect_type): Catch exception thrown by
> 	value_ind.
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=17416
> ---
>  gdb/valops.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/valops.c b/gdb/valops.c
> index e1decf0..c1a0c86 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -3609,7 +3609,18 @@ value_rtti_indirect_type (struct value *v, int *full,
>    if (TYPE_CODE (type) == TYPE_CODE_REF)
>      target = coerce_ref (v);
>    else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -    target = value_ind (v);
> +    {
> +      volatile struct gdb_exception except;
> +
> +      TRY_CATCH (except, RETURN_MASK_ERROR)
> +        {
> +	  target = value_ind (v);
> +        }
> +      if (except.error == MEMORY_ERROR)
> +	  return NULL;
> +      else if (except.error != GDB_NO_ERROR)
> +	throw_exception (except);

Indentation looks odd.  But, could you write it like this instead ?

      volatile struct gdb_exception except;

      TRY_CATCH (except, RETURN_MASK_ERROR)
        {
	  target = value_ind (v);
        }
      if (except.reason < 0)
        {
          if (except.error == MEMORY_ERROR)
            {
              /* Add comment here.  */
	      return NULL;
            }
  	  throw_exception (except);
        }

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/valops.c b/gdb/valops.c
index e1decf0..c1a0c86 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3609,7 +3609,18 @@  value_rtti_indirect_type (struct value *v, int *full,
   if (TYPE_CODE (type) == TYPE_CODE_REF)
     target = coerce_ref (v);
   else if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    target = value_ind (v);
+    {
+      volatile struct gdb_exception except;
+
+      TRY_CATCH (except, RETURN_MASK_ERROR)
+        {
+	  target = value_ind (v);
+        }
+      if (except.error == MEMORY_ERROR)
+	  return NULL;
+      else if (except.error != GDB_NO_ERROR)
+	throw_exception (except);
+    }
   else
     return NULL;