This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy


On 07/04/2013 05:40 PM, Andrew Burgess wrote:
> On 03/07/2013 8:57 PM, Pedro Alves wrote:
>> On 07/01/2013 07:06 PM, Andrew Burgess wrote:
>>> On 19/06/2013 4:50 PM, Pedro Alves wrote:
>>>> On 06/10/2013 11:24 AM, Andrew Burgess wrote:
>>>>
>>>>> I ran into an issue with gdb that appears to be caused by an incorrect
>>>>> use of value_optimized_out.
>>>>>
>>>>
>>>> I'm finding the patch a bit hard to read though.  Could you
>>>> split it up?  
>>>
>>> This tiny patch notices that when we mark a value as optimized out
>>> we can also mark the value as no longer lazy.
>>> This patch is not required, but felt like a good thing to me, not
>>> sure if everyone will agree though.
>>>
>>> Should I apply?
>>>
>>> Thanks
>>> Andrew
>>>
>>>
>>>
>>>
>>> gdb/ChangeLog
>>>
>>> 2013-07-01  Andrew Burgess  <aburgess@broadcom.com>
>>>
>>> 	* value.c (set_value_optimized_out): A value that is optimized out
>>> 	is no longer lazy.
>>
>> Hmm.
>>
>> I tried going a step further.  If the value is optimized
>> out, then we don't really need its contents buffer.
>>
>> That is:
>>
>>  gdb/value.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/gdb/value.c b/gdb/value.c
>> index 970fb66..50f8889 100644
>> --- a/gdb/value.c
>> +++ b/gdb/value.c
>> @@ -690,6 +690,8 @@ allocate_value_lazy (struct type *type)
>>  void
>>  allocate_value_contents (struct value *val)
>>  {
>> +  gdb_assert (!val->optimized_out);
>> +
>>    if (!val->contents)
>>      val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
>>  }
>> @@ -1064,6 +1066,13 @@ void
>>  set_value_optimized_out (struct value *value, int val)
>>  {
>>    value->optimized_out = val;
>> +
>> +  if (val)
>> +    {
>> +      set_value_lazy (value, 0);
>> +      xfree (value->contents);
>> +      value->contents = NULL;
>> +    }
>>  }
>>
>> and then ran the testsuite.  Sure enough, that catches a gdb crash.
>> In gdb.dwarf2/dw2-op-out-param.exp:
>>
>> #0  0x000000000056bed0 in extract_signed_integer (addr=0x0, len=8, byte_order=BFD_ENDIAN_LITTLE) at ../../src/gdb/findvar.c:78
>> #1  0x000000000057e761 in unpack_long (type=0x1793f80, valaddr=0x0) at ../../src/gdb/value.c:2453
>> #2  0x000000000059baae in print_scalar_formatted (valaddr=0x0, type=0x1793f80, options=0x7fffdb2376c0, size=0, stream=0x1929a00) at ../../src/gdb/printcmd.c:404
>> #3  0x000000000059763b in val_print_scalar_formatted (type=0x1793f80, valaddr=0x0, embedded_offset=0, val=0x19dfcd0, options=0x7fffdb2376c0, size=0, stream=0x1929a00)
>>     at ../../src/gdb/valprint.c:970
>> #4  0x00000000006d1f2f in c_val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, original_value=0x19dfcd0, options=
>>     0x7fffdb237860) at ../../src/gdb/c-valprint.c:407
>> #5  0x0000000000596f4a in val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, val=0x19dfcd0, options=0x7fffdb237940,
>>     language=0x8d34a0) at ../../src/gdb/valprint.c:778
>> #6  0x00000000006d3491 in cp_print_value_fields (type=0x1796110, real_type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0,
>>     options=0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:363
>> #7  0x00000000006d37ef in cp_print_value_fields_rtti (type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=
>>     0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:458
>> #8  0x00000000006d1e45 in c_val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, original_value=0x19dfcd0, options=
>>     0x7fffdb237d40) at ../../src/gdb/c-valprint.c:393
>> #9  0x0000000000596f4a in val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=0x7fffdb237e60,
>>     language=0x8d34a0) at ../../src/gdb/valprint.c:778
>> #10 0x0000000000597157 in common_val_print (val=0x19dfcd0, stream=0x1929a00, recurse=2, options=0x7fffdb237e60, language=0x8d34a0) at ../../src/gdb/valprint.c:840
>> #11 0x00000000005d9ea6 in print_frame_arg (arg=0x7fffdb237f30) at ../../src/gdb/stack.c:284
>> #12 0x00000000005daa49 in print_frame_args (func=0x1796af0, frame=0x1b5b800, num=-1, stream=0x168fd10) at ../../src/gdb/stack.c:645
>> #13 0x00000000005db9df in print_frame (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1, sal=...) at ../../src/gdb/stack.c:1172
>> #14 0x00000000005daf95 in print_frame_info (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1) at ../../src/gdb/stack.c:824
>> #15 0x00000000005dcdc5 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at ../../src/gdb/stack.c:1763
>> #16 0x00000000005dd1b6 in backtrace_command (arg=0x0, from_tty=1) at ../../src/gdb/stack.c:1860
>> #17 0x00000000004dc53f in do_cfunc (c=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:113
>> #18 0x00000000004df5d4 in cmd_func (cmd=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1888
>> #19 0x00000000006e43d1 in execute_command (p=0x14623a2 "", from_tty=1) at ../../src/gdb/top.c:478
>>
>> Odd that this bit of val_print_scalar_formatted:
>>
>>   /* A scalar object that does not have all bits available can't be
>>      printed, because all bits contribute to its representation.  */
>>   if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
>> 			      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
>>     val_print_optimized_out (stream);
>>
>> didn't catch this.  I then added this further.
>>
>> --- c/gdb/value.c
>> +++ w/gdb/value.c
>> @@ -1078,6 +1078,8 @@ set_value_optimized_out (struct value *value, int val)
>>  int
>>  value_entirely_optimized_out (const struct value *value)
>>  {
>> +  gdb_assert (value->optimized_out || !value->lazy);
>> +
>>    if (!value->optimized_out)
>>      return 0;
>>    if (value->lval != lval_computed
>> @@ -1089,6 +1091,8 @@ value_entirely_optimized_out (const struct value *value)
>>  int
>>  value_bits_valid (const struct value *value, int offset, int length)
>>  {
>> +  gdb_assert (value->optimized_out || !value->lazy);
>> +
>>    if (!value->optimized_out)
>>      return 1;
>>    if (value->lval != lval_computed
>>
>> And I see:
>>
>> Breakpoint 2, 0x000000000040058f in breakpt ()
>> (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_two_reg_pieces
>> bt
>> #0  0x000000000040058f in breakpt ()
>> #1  0x00000000004005c2 in struct_param_two_reg_pieces (operand0=../../src/gdb/value.c:1081: internal-error: value_entirely_optimized_out: Assertion `value->optimized_out || !value->lazy' failed.
>> A problem internal to GDB has been detected,
>> further debugging may prove unreliable.
>> Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_two_reg_pieces (GDB internal error)
>> Resyncing due to internal error.
>>
>> ... and then I ran out of time.  Gotta run an errand.  :-)
>>
> 
> The issue here is that optimized_out does not mean "the value is
> optimized out", but instead (in the general case) means this value is at
> least partially optimized out.
> 
> The problem case are computed values, where if part of the value is
> missing then the optimized_out flag is set, but we still rely on being
> able to access the contents of the value, with the result that your
> patch above, freeing the contents, causes problems.

Right.

> I don't include the patch here, but as an experiment I made a quick hack
> where all the calls to set_value_optimized_out inside dwarf2loc.c call a
> new method set_value_partially_optimized_out, this new method sets the
> optimized_out flag but does not free the contents buffer, all the tests
> now pass.  I know this is a hack, we create computed values in other
> situations, but the case of computed value and partially optimized out
> values only appear to be tested for the dwarf case.

Yep.  I'm actually about to post a patch in this direction.  I'm just
splitting it into smaller unrelated chunks.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]