diff mbox

Eliminate -var-create error for optzd ptr to struct

Message ID 1459882257-7029-1-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal April 5, 2016, 6:50 p.m. UTC 
Hi Yao,

On 4/5/2016 5:52 AM, Yao Qi wrote:
> Don Breazeal <donb@codesourcery.com> writes:

>> +      CATCH (ex, RETURN_MASK_ERROR)
>> +	{
>> +	  /* If we get an error, assume the value is not optimized out.  */
>> +	  return 0;
> 
> Why don't we fall back to checking value->optimized_out below?  Some
> bits/pieces of value are optimized out, but reading the rest of
> bits/piece may trigger the memory error.  In this case, the value is
> optimized out too.  We can do this...
> 
>          TRY
>          {
>            value_fetch_lazy (value);
>          }
>          CATCH (ex, RETURN_MASK_ERROR)
>          {
>            /* Fall back to checking value->optimized_out.  */
>          }
>          END_CATCH
> 
> What do you think?

Of course, that makes more sense, thanks.

> Note that, after this patch, value_optimized_out will no longer throw
> exceptions, some TRY/CATCH in value_optimized_out's callers can be
> removed, such as gdbscm_value_optimized_out_p and
> valpy_get_is_optimized_out.  This can be done in a follow-up patch.

I've done a more thorough audit of the call sites for value_optimized_out
than I did previously, and the only places where an enclosing TRY/CATCH can
be removed are the two you name.  There is one other place where it is
called inside a TRY/CATCH, but there are other functions that could throw
errors called there as well.

I will create a follow-up patch.

I changed this patch as you suggest above, as well as changing
RETURN_MASK_ERROR to RETURN_MASK_ALL.  The TRY/CATCH blocks that are
going to be removed use RETURN_MASK_ALL, and I thought that this patch
should maintain the same level of coverage.

Re-tested on native Linux x86_64.

Thanks,
--Don

----
This patch eliminates an error thrown when accessing the value of a
pointer to a structure where the pointer has been optimized out and
'set print object' is 'on'.  The error shows up as the rather ugly
value of the pointer variable in Eclipse.

If 'set print object' is 'on', GDB tries to determine the actual
(derived) type of the object rather than the declared type, which
requires dereferencing the pointer, which in this cases throws an
error because the pointer has been optimized out.

The fix is to simply ignore the 'print object on' setting for
pointers or references to structures when they have been optimized
out.  This means we just get the declared type instead of the actual
type, because in this case that's the best that we can do.

To implement the fix, value_optimized_out was modified so that it
no longer throws an error when it fails to fetch the specified
value.  If we can't definitively say that the value is optimized
out, then we assume it is not.

I'm working on setting things in motion for a patch to Eclipse that
recognizes optimized-out pointer-to-struct in this scenario and
prevents any subsequent attempt to dereference it from that end.

Tested on bare-metal powerpc board with Linux x86 host and Linux
native x86_64.

gdb/ChangeLog:
2016-04-05  Don Breazeal  <donb@codesourcery.com>

	* value.c (value_actual_type): Don't try to get rtti type
	of the value if it has been optimized out.
	(value_optimized_out): If a memory access error occurs,
	fall back to just checking value->optimized_out.

---
 gdb/value.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Pedro Alves April 5, 2016, 7 p.m. UTC | #1 
On 04/05/2016 07:50 PM, Don Breazeal wrote:
> Hi Yao,
> 
> On 4/5/2016 5:52 AM, Yao Qi wrote:
>> Don Breazeal <donb@codesourcery.com> writes:
> 
>>> +      CATCH (ex, RETURN_MASK_ERROR)
>>> +	{
>>> +	  /* If we get an error, assume the value is not optimized out.  */
>>> +	  return 0;
>>
>> Why don't we fall back to checking value->optimized_out below?  Some
>> bits/pieces of value are optimized out, but reading the rest of
>> bits/piece may trigger the memory error.  In this case, the value is
>> optimized out too.  We can do this...
>>
>>          TRY
>>          {
>>            value_fetch_lazy (value);
>>          }
>>          CATCH (ex, RETURN_MASK_ERROR)
>>          {
>>            /* Fall back to checking value->optimized_out.  */
>>          }
>>          END_CATCH
>>
>> What do you think?
> 
> Of course, that makes more sense, thanks.
> 
>> Note that, after this patch, value_optimized_out will no longer throw
>> exceptions, some TRY/CATCH in value_optimized_out's callers can be
>> removed, such as gdbscm_value_optimized_out_p and
>> valpy_get_is_optimized_out.  This can be done in a follow-up patch.
> 
> I've done a more thorough audit of the call sites for value_optimized_out
> than I did previously, and the only places where an enclosing TRY/CATCH can
> be removed are the two you name.  There is one other place where it is
> called inside a TRY/CATCH, but there are other functions that could throw
> errors called there as well.
> 
> I will create a follow-up patch.
> 
> I changed this patch as you suggest above, as well as changing
> RETURN_MASK_ERROR to RETURN_MASK_ALL.  The TRY/CATCH blocks that are
> going to be removed use RETURN_MASK_ALL, and I thought that this patch
> should maintain the same level of coverage.

Please don't.  A RETURN_MASK_ALL swallows Ctrl-C/QUIT, and that's almost
always a bug.  The cases you mention translate a QUIT to a python/scheme
exception, which is not the same as just swallowing the exception.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/value.c b/gdb/value.c
index 8268b08..8a210e7 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1205,7 +1205,8 @@  value_actual_type (struct value *value, int resolve_simple_types,
       if ((TYPE_CODE (result) == TYPE_CODE_PTR
 	   || TYPE_CODE (result) == TYPE_CODE_REF)
 	  && TYPE_CODE (check_typedef (TYPE_TARGET_TYPE (result)))
-	     == TYPE_CODE_STRUCT)
+	     == TYPE_CODE_STRUCT
+	  && !value_optimized_out (value))
         {
           struct type *real_type;
 
@@ -1433,7 +1434,17 @@  value_optimized_out (struct value *value)
   /* We can only know if a value is optimized out once we have tried to
      fetch it.  */
   if (VEC_empty (range_s, value->optimized_out) && value->lazy)
-    value_fetch_lazy (value);
+    {
+      TRY
+	{
+	  value_fetch_lazy (value);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  /* Fall back to checking value->optimized_out.  */
+	}
+      END_CATCH
+    }
 
   return !VEC_empty (range_s, value->optimized_out);
 }