Re: [PATCH] Handle partially optimized out values similarly to unavailable values (Re: [patchv2] Fix crash on optimized-out entry data values)

[Pedro Alves <palves at redhat dot com>]

On 07/16/2014 10:58 PM, Jan Kratochvil wrote:
> 
> 
> This patch is apparently not suitable for gdb-7.8 which is I guess often
> crashing on -O2 -g entry values so there could be some rather minimal crash
> avoiding patch instead.
> 

Yeah.

So this was originally "caused" (more exposed) by 4f14910f:
    
        gdb/ChangeLog
        2013-11-26  Andrew Burgess  <aburgess@broadcom.com>
    
            * value.c (allocate_optimized_out_value): Mark value as non-lazy.

I tried a few approaches in value_available_contents_eq
today, and ended up thinking that the simplest should be to
just revert that patch until we have the fuller fix in place.

While doing just that fixes the crash, it surprisingly causes
one of your new tests to FAIL:

 (gdb) frame
 #0  bar (ref=ref@entry=@0x7fffffffd184: 10) at gdb.arch/amd64-entry-value-paramref.cc:23
 23        vv++; /* break-here */
 (gdb) FAIL: gdb.arch/amd64-entry-value-paramref.exp: frame

If I instead make allocate_optimized_out_value not allocate
a lazy value, like:

 struct value *
 allocate_optimized_out_value (struct type *type)
 {
 -  struct value *retval = allocate_value_lazy (type);
 +  struct value *retval = allocate_value (type);
 
   set_value_optimized_out (retval, 1);
 -  set_value_lazy (retval, 0);
   return retval;
 }

Then the test passes.

 (gdb) PASS: gdb.arch/amd64-entry-value-paramref.exp: continue to breakpoint: break-here
 frame
 #0  bar (ref=@0x7fffffffd184: 10, ref@entry=@0x7fffffffd184: <optimized out>) at gdb.arch/amd64-entry-value-paramref.cc:23
 23        vv++; /* break-here */
 (gdb) PASS: gdb.arch/amd64-entry-value-paramref.exp: frame

The difference is that if we let the value be lazy, value_fetch_lazy
fetches contents from memory, and then those contents compare
equal with the non-entry value.

Hmm.  But that's odd, why would value_fetch_lazy fetch the value
from memory at all, it it started out as an optimized-out / not_lval
value?  What made it be lval_memory ?

Turns out it's the code disabled in value_of_dwarf_reg_entry:

  target_val = dwarf_entry_parameter_to_value (parameter,
					       TYPE_LENGTH (target_type),
					       target_type, caller_frame,
					       caller_per_cu);

  /* value_as_address dereferences TYPE_CODE_REF.  */
  addr = extract_typed_address (value_contents (outer_val), checked_type);

  /* The target entry value has artificial address of the entry value
     reference.  */
  VALUE_LVAL (target_val) = lval_memory;
  set_value_address (target_val, addr);

It looks quite wrong to me to just change a value's lval like that.

I ran the testsuite with that code disabled (like in the patch below),
and that caused no regressions.  I can't say I really understand the
intention here though.  What would we be missing if we removed that code?

------------------
>From 91d05d6af2b36a701429219f37712e7c6f944c4f Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 17 Jul 2014 11:06:18 +0100
Subject: [PATCH] Fix crash on optimized-out entry data values

The tests at
<https://sourceware.org/ml/gdb-patches/2014-07/msg00277.html> show
that comparing a fully optimized out value's contents with a value
that has not been optimized out, or is partially optimized out crashes
GDB:

 (gdb) bt
 #0  __memcmp_sse4_1 () at ../sysdeps/x86_64/multiarch/memcmp-sse4.S:816
 #1  0x00000000005a1914 in memcmp_with_bit_offsets (ptr1=0x202b2f0 "\n", offset1_bits=0, ptr2=0x0, offset2_bits=0, length_bits=32)
     at /home/pedro/gdb/mygit/build/../src/gdb/value.c:678
 #2  0x00000000005a1a05 in value_available_contents_bits_eq (val1=0x2361ad0, offset1=0, val2=0x23683b0, offset2=0, length=32)
     at /home/pedro/gdb/mygit/build/../src/gdb/value.c:717
 #3  0x00000000005a1c09 in value_available_contents_eq (val1=0x2361ad0, offset1=0, val2=0x23683b0, offset2=0, length=4)
     at /home/pedro/gdb/mygit/build/../src/gdb/value.c:769
 #4  0x00000000006033ed in read_frame_arg (sym=0x1b78d20, frame=0x19bca50, argp=0x7fff4aba82b0, entryargp=0x7fff4aba82d0)
     at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:416
 #5  0x0000000000603abb in print_frame_args (func=0x1b78cb0, frame=0x19bca50, num=-1, stream=0x1aea450) at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:671
 #6  0x0000000000604ae8 in print_frame (frame=0x19bca50, print_level=0, print_what=SRC_AND_LOC, print_args=1, sal=...)
     at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:1205
 #7  0x0000000000604050 in print_frame_info (frame=0x19bca50, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1)
     at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:857
 #8  0x00000000006029b3 in print_stack_frame (frame=0x19bca50, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1)
     at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:169
 #9  0x00000000005fc4b8 in print_stop_event (ws=0x7fff4aba8790) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6068
 #10 0x00000000005fc830 in normal_stop () at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6214

The 'ptr2=0x0' in frame #1 is val2->contents, and since git 4f14910f:

    gdb/ChangeLog
    2013-11-26  Andrew Burgess  <aburgess@broadcom.com>

        * value.c (allocate_optimized_out_value): Mark value as non-lazy.

... a fully optimized-out value can have it's value contents buffer
NULL.

As a spotgap fix, revert 4f14910f, with a comment.  A full fix would
be too invasive for 7.8.

gdb/
2014-07-17  Pedro Alves  <palves@redhat.com>

	* value.c (allocate_optimized_out_value): Don't mark value as
          non-lazy.
---
 gdb/dwarf2loc.c | 2 ++
 gdb/value.c     | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index fcab9b9..8ed3be0 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1335,6 +1335,7 @@ value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
 					       target_type, caller_frame,
 					       caller_per_cu);
 
+#if 0
   /* value_as_address dereferences TYPE_CODE_REF.  */
   addr = extract_typed_address (value_contents (outer_val), checked_type);
 
@@ -1342,6 +1343,7 @@ value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
      reference.  */
   VALUE_LVAL (target_val) = lval_memory;
   set_value_address (target_val, addr);
+#endif
 
   release_value (target_val);
   val = allocate_computed_value (type, &entry_data_value_funcs,
diff --git a/gdb/value.c b/gdb/value.c
index 29abe5f..8b44a6d 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -906,7 +906,10 @@ allocate_optimized_out_value (struct type *type)
   struct value *retval = allocate_value_lazy (type);
 
   set_value_optimized_out (retval, 1);
-  set_value_lazy (retval, 0);
+  /* FIXME: we should be able to avoid allocating the value's contents
+     buffer, but value_available_contents_bits_eq can't handle
+     that.  */
+  /* set_value_lazy (retval, 0); */
   return retval;
 }
 
-- 
1.9.3