[patch#2] fetch result of locdesc expressions as integer (not address)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Mon, 03 Oct 2011 23:10:14 +0200, Joel Brobecker wrote:
> The problem is that the debugger is treating the result of
> the DWARF location expressions as addresses, whereas this is
> just an offset in this case.

It is not an offset, the problem is it really is an address.

DWARF-4:
# The beginning address is pushed on the DWARF stack before the location
# description is evaluated; the result of the evaluation is the base address
# of the member entry.

Both former and current FSF GDB pushes bogus address 0 first simulating base
address of the struct.
-  stack[stacki] = 0;
-  stack[++stacki] = 0;
+  /* DW_AT_data_member_location expects the structure address to be pushed on
+     the stack.  Simulate the offset by address 0.  */
+  dwarf_expr_push_address (ctx, 0, 0);


Proposing to revert the patch of mine breaking it:
	[patch 2/2] Fix decode_locdesc for gcc-4.7.x optimized DWARF
	http://sourceware.org/ml/gdb-patches/2011-07/msg00762.html
	49c026948157691b949769c8c3365d18cf74b319

It was there for DWARF-2 compatibility with GCC-4.7.x.  It is unrelated to
DWARF-3+ as those use DW_FORM_udata as `constant' DWARF class is already
supported for DW_AT_data_member_location there).  Jakub removed this temporary
GCC-4.7.x optimization - it is in fact not useful, it is only incompatible:
	commit 946090338c2fdde793f035832d0b7544ed541132
	Author: jakub <jakub@138bc75d-0d04-0410-961f-82ee72b054a4>
	Date:   Thu Jul 28 16:23:20 2011 +0000
		* dwarf2out.c (resolve_addr): For -gdwarf-2 don't
		optimize DW_AT_data_member_location containing just
		DW_OP_plus_uconst.
		git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@176878 138bc75d-0d04-0410-961f-82ee72b054a4

While trying to fix it I faced for example the exception for address 0 - isn't
it broken for AVR?  Isn't SRAM address 0 a valid address?
	static CORE_ADDR
	avr_make_saddr (CORE_ADDR x)
	{
	  /* Return 0 for NULL.  */
	  if (x == 0)
	    return 0;

	  return ((x) | AVR_SMEM_START);
	}
Unfortunately I cannot argue about AVR arch issues.  Joel, do you run the
testsuite with iron AVR or is it OK to run it some way with sim/avr/ ?

I was trying to clean up the DWARF expression vs. location issues Ulrich
noted.  The problem is the combination of (a) the AVR address 0 hack above and
(b) inability of GDB to do proper dynamic type with DWARF block as its dynamic
DW_AT_data_member_location attribute (easy - although not present there - with
archer-jankratochvil-vla) leading to more and more hacks.

There is a similar expression vs. location issue in the entryval patchset:
+       /* DW_AT_GNU_call_site_target is a DWARF expression, not a DWARF
+          location.  */
+       if (VALUE_LVAL (val) == lval_memory)
+         return value_address (val);
+       else
+         return value_as_address (val);
just fortunately as the result is always address it should get IMO converted
right in both cases.

BTW I also tried to fix the crash - to report an error instead - but that is
not much possible unless the printing gets converted to `struct value'
compliant one.  Currently it can address at least the virtual method table
content while still referencing the original value so there is no way to check
the boundaries.


It is a miracle GDB worked but if everyone was happy with the former state why
not to use it.  When archer-jankratochvil-vla gets merged in some form
(reworked from scratch or at least significantly) - which needs to happen some
day anyway - I will happily reapply this reverted patch.


No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Sorry,
Jan


gdb/
2011-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Revert:
	2011-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
        * dwarf2expr.c (ctx_no_read_reg): New function.
        * dwarf2expr.h (ctx_no_read_reg): New declaration.
        * dwarf2read.c (read_2_signed_bytes, read_4_signed_bytes): Remove.
        (decode_locdesc_read_mem, decode_locdesc_ctx_funcs): New.
        (decode_locdesc): Replace by a caller of dwarf_expr_eval.

gdb/testsuite/
2011-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-simple-locdesc.exp (p &s.shl): KFAIL it.
	Revert the part of:
	2011-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
	* gdb.dwarf2/dw2-stack-boundary.exp (check partial symtab errors):
	Change the expected string.

--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -1280,14 +1280,6 @@ abort_expression:
   gdb_assert (ctx->recursion_depth >= 0);
 }
 
-/* Stub dwarf_expr_context_funcs.read_reg implementation.  */
-
-CORE_ADDR
-ctx_no_read_reg (void *baton, int regnum)
-{
-  error (_("Registers access is invalid in this context"));
-}
-
 /* Stub dwarf_expr_context_funcs.get_frame_base implementation.  */
 
 void
--- a/gdb/dwarf2expr.h
+++ b/gdb/dwarf2expr.h
@@ -255,7 +255,6 @@ void dwarf_expr_require_composition (const gdb_byte *, const gdb_byte *,
 
 /* Stub dwarf_expr_context_funcs implementations.  */
 
-CORE_ADDR ctx_no_read_reg (void *baton, int regnum);
 void ctx_no_get_frame_base (void *baton, const gdb_byte **start,
 			    size_t *length);
 CORE_ADDR ctx_no_get_frame_cfa (void *baton);
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10015,12 +10015,24 @@ read_2_bytes (bfd *abfd, gdb_byte *buf)
   return bfd_get_16 (abfd, buf);
 }
 
+static int
+read_2_signed_bytes (bfd *abfd, gdb_byte *buf)
+{
+  return bfd_get_signed_16 (abfd, buf);
+}
+
 static unsigned int
 read_4_bytes (bfd *abfd, gdb_byte *buf)
 {
   return bfd_get_32 (abfd, buf);
 }
 
+static int
+read_4_signed_bytes (bfd *abfd, gdb_byte *buf)
+{
+  return bfd_get_signed_32 (abfd, buf);
+}
+
 static ULONGEST
 read_8_bytes (bfd *abfd, gdb_byte *buf)
 {
@@ -14084,37 +14096,6 @@ read_signatured_type (struct objfile *objfile,
   dwarf2_per_objfile->read_in_chain = &type_sig->per_cu;
 }
 
-/* Workaround as dwarf_expr_context_funcs.read_mem implementation before
-   a proper runtime DWARF expressions evaluator gets implemented.
-   Otherwise gnuv3_baseclass_offset would error by:
-   Expected a negative vbase offset (old compiler?)  */
-
-static void
-decode_locdesc_read_mem (void *baton, gdb_byte *buf, CORE_ADDR addr,
-			 size_t length)
-{
-  struct dwarf_expr_context *ctx = baton;
-  struct gdbarch *gdbarch = ctx->gdbarch;
-  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
-
-  memset (buf, 0, length);
-
-  if (TYPE_LENGTH (ptr_type) == length)
-    store_typed_address (buf, ptr_type, addr);
-}
-
-static const struct dwarf_expr_context_funcs decode_locdesc_ctx_funcs =
-{
-  ctx_no_read_reg,
-  decode_locdesc_read_mem,
-  ctx_no_get_frame_base,
-  ctx_no_get_frame_cfa,
-  ctx_no_get_frame_pc,
-  ctx_no_get_tls_address,
-  ctx_no_dwarf_call,
-  ctx_no_get_base_type
-};
-
 /* Decode simple location descriptions.
    Given a pointer to a dwarf block that defines a location, compute
    the location and return the value.
@@ -14132,59 +14113,235 @@ static const struct dwarf_expr_context_funcs decode_locdesc_ctx_funcs =
    object is optimized out.  The return value is 0 for that case.
    FIXME drow/2003-11-16: No callers check for this case any more; soon all
    callers will only want a very basic result and this can become a
-   complaint.  */
+   complaint.
+
+   Note that stack[0] is unused except as a default error return.  */
 
 static CORE_ADDR
 decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->objfile;
-  struct dwarf_expr_context *ctx;
-  struct cleanup *old_chain;
-  volatile struct gdb_exception ex;
+  int i;
+  int size = blk->size;
+  gdb_byte *data = blk->data;
+  CORE_ADDR stack[64];
+  int stacki;
+  unsigned int bytes_read, unsnd;
+  gdb_byte op;
 
-  ctx = new_dwarf_expr_context ();
-  old_chain = make_cleanup_free_dwarf_expr_context (ctx);
-  make_cleanup_value_free_to_mark (value_mark ());
+  i = 0;
+  stacki = 0;
+  stack[stacki] = 0;
+  stack[++stacki] = 0;
 
-  ctx->gdbarch = get_objfile_arch (objfile);
-  ctx->addr_size = cu->header.addr_size;
-  ctx->offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
-  ctx->baton = ctx;
-  ctx->funcs = &decode_locdesc_ctx_funcs;
+  while (i < size)
+    {
+      op = data[i++];
+      switch (op)
+	{
+	case DW_OP_lit0:
+	case DW_OP_lit1:
+	case DW_OP_lit2:
+	case DW_OP_lit3:
+	case DW_OP_lit4:
+	case DW_OP_lit5:
+	case DW_OP_lit6:
+	case DW_OP_lit7:
+	case DW_OP_lit8:
+	case DW_OP_lit9:
+	case DW_OP_lit10:
+	case DW_OP_lit11:
+	case DW_OP_lit12:
+	case DW_OP_lit13:
+	case DW_OP_lit14:
+	case DW_OP_lit15:
+	case DW_OP_lit16:
+	case DW_OP_lit17:
+	case DW_OP_lit18:
+	case DW_OP_lit19:
+	case DW_OP_lit20:
+	case DW_OP_lit21:
+	case DW_OP_lit22:
+	case DW_OP_lit23:
+	case DW_OP_lit24:
+	case DW_OP_lit25:
+	case DW_OP_lit26:
+	case DW_OP_lit27:
+	case DW_OP_lit28:
+	case DW_OP_lit29:
+	case DW_OP_lit30:
+	case DW_OP_lit31:
+	  stack[++stacki] = op - DW_OP_lit0;
+	  break;
 
-  /* DW_AT_data_member_location expects the structure address to be pushed on
-     the stack.  Simulate the offset by address 0.  */
-  dwarf_expr_push_address (ctx, 0, 0);
+	case DW_OP_reg0:
+	case DW_OP_reg1:
+	case DW_OP_reg2:
+	case DW_OP_reg3:
+	case DW_OP_reg4:
+	case DW_OP_reg5:
+	case DW_OP_reg6:
+	case DW_OP_reg7:
+	case DW_OP_reg8:
+	case DW_OP_reg9:
+	case DW_OP_reg10:
+	case DW_OP_reg11:
+	case DW_OP_reg12:
+	case DW_OP_reg13:
+	case DW_OP_reg14:
+	case DW_OP_reg15:
+	case DW_OP_reg16:
+	case DW_OP_reg17:
+	case DW_OP_reg18:
+	case DW_OP_reg19:
+	case DW_OP_reg20:
+	case DW_OP_reg21:
+	case DW_OP_reg22:
+	case DW_OP_reg23:
+	case DW_OP_reg24:
+	case DW_OP_reg25:
+	case DW_OP_reg26:
+	case DW_OP_reg27:
+	case DW_OP_reg28:
+	case DW_OP_reg29:
+	case DW_OP_reg30:
+	case DW_OP_reg31:
+	  stack[++stacki] = op - DW_OP_reg0;
+	  if (i < size)
+	    dwarf2_complex_location_expr_complaint ();
+	  break;
 
-  TRY_CATCH (ex, RETURN_MASK_ERROR)
-    {
-      dwarf_expr_eval (ctx, blk->data, blk->size);
-    }
-  if (ex.reason < 0)
-    {
-      if (ex.message)
-	complaint (&symfile_complaints, "%s", ex.message);
-    }
-  else if (ctx->num_pieces == 0)
-    switch (ctx->location)
-      {
-      /* The returned number will be bogus, just do not complain for locations
-	 in global registers - it is here only a partial symbol address.  */
-      case DWARF_VALUE_REGISTER:
+	case DW_OP_regx:
+	  unsnd = read_unsigned_leb128 (NULL, (data + i), &bytes_read);
+	  i += bytes_read;
+	  stack[++stacki] = unsnd;
+	  if (i < size)
+	    dwarf2_complex_location_expr_complaint ();
+	  break;
 
-      case DWARF_VALUE_MEMORY:
-      case DWARF_VALUE_STACK:
-	{
-	  CORE_ADDR address = dwarf_expr_fetch_address (ctx, 0);
+	case DW_OP_addr:
+	  stack[++stacki] = read_address (objfile->obfd, &data[i],
+					  cu, &bytes_read);
+	  i += bytes_read;
+	  break;
+
+	case DW_OP_const1u:
+	  stack[++stacki] = read_1_byte (objfile->obfd, &data[i]);
+	  i += 1;
+	  break;
+
+	case DW_OP_const1s:
+	  stack[++stacki] = read_1_signed_byte (objfile->obfd, &data[i]);
+	  i += 1;
+	  break;
+
+	case DW_OP_const2u:
+	  stack[++stacki] = read_2_bytes (objfile->obfd, &data[i]);
+	  i += 2;
+	  break;
+
+	case DW_OP_const2s:
+	  stack[++stacki] = read_2_signed_bytes (objfile->obfd, &data[i]);
+	  i += 2;
+	  break;
 
-	  do_cleanups (old_chain);
-	  return address;
+	case DW_OP_const4u:
+	  stack[++stacki] = read_4_bytes (objfile->obfd, &data[i]);
+	  i += 4;
+	  break;
+
+	case DW_OP_const4s:
+	  stack[++stacki] = read_4_signed_bytes (objfile->obfd, &data[i]);
+	  i += 4;
+	  break;
+
+	case DW_OP_constu:
+	  stack[++stacki] = read_unsigned_leb128 (NULL, (data + i),
+						  &bytes_read);
+	  i += bytes_read;
+	  break;
+
+	case DW_OP_consts:
+	  stack[++stacki] = read_signed_leb128 (NULL, (data + i), &bytes_read);
+	  i += bytes_read;
+	  break;
+
+	case DW_OP_dup:
+	  stack[stacki + 1] = stack[stacki];
+	  stacki++;
+	  break;
+
+	case DW_OP_plus:
+	  stack[stacki - 1] += stack[stacki];
+	  stacki--;
+	  break;
+
+	case DW_OP_plus_uconst:
+	  stack[stacki] += read_unsigned_leb128 (NULL, (data + i),
+						 &bytes_read);
+	  i += bytes_read;
+	  break;
+
+	case DW_OP_minus:
+	  stack[stacki - 1] -= stack[stacki];
+	  stacki--;
+	  break;
+
+	case DW_OP_deref:
+	  /* If we're not the last op, then we definitely can't encode
+	     this using GDB's address_class enum.  This is valid for partial
+	     global symbols, although the variable's address will be bogus
+	     in the psymtab.  */
+	  if (i < size)
+	    dwarf2_complex_location_expr_complaint ();
+	  break;
+
+        case DW_OP_GNU_push_tls_address:
+	  /* The top of the stack has the offset from the beginning
+	     of the thread control block at which the variable is located.  */
+	  /* Nothing should follow this operator, so the top of stack would
+	     be returned.  */
+	  /* This is valid for partial global symbols, but the variable's
+	     address will be bogus in the psymtab.  */
+	  if (i < size)
+	    dwarf2_complex_location_expr_complaint ();
+          break;
+
+	case DW_OP_GNU_uninit:
+	  break;
+
+	default:
+	  {
+	    const char *name = dwarf_stack_op_name (op);
+
+	    if (name)
+	      complaint (&symfile_complaints, _("unsupported stack op: '%s'"),
+			 name);
+	    else
+	      complaint (&symfile_complaints, _("unsupported stack op: '%02x'"),
+			 op);
+	  }
+
+	  return (stack[stacki]);
 	}
-      }
 
-  do_cleanups (old_chain);
-  dwarf2_complex_location_expr_complaint ();
-  return 0;
+      /* Enforce maximum stack depth of SIZE-1 to avoid writing
+         outside of the allocated space.  Also enforce minimum>0.  */
+      if (stacki >= ARRAY_SIZE (stack) - 1)
+	{
+	  complaint (&symfile_complaints,
+		     _("location description stack overflow"));
+	  return 0;
+	}
+
+      if (stacki <= 0)
+	{
+	  complaint (&symfile_complaints,
+		     _("location description stack underflow"));
+	  return 0;
+	}
+    }
+  return (stack[stacki]);
 }
 
 /* memory allocation interface */
--- a/gdb/testsuite/gdb.dwarf2/dw2-simple-locdesc.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-simple-locdesc.exp
@@ -32,7 +32,15 @@ clean_restart $executable
 
 # Re: [patch 2/2] Fix decode_locdesc for gcc-4.7.x optimized DWARF
 # http://sourceware.org/ml/gdb-patches/2011-07/msg00766.html
-gdb_test "p &s.shl" { = \(int \*\) 0x1000000}
+set test "p &s.shl"
+gdb_test_multiple $test $test {
+    -re " = \\(int \\*\\) 0x1000000\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " = \\(int \\*\\) 0x14\r\n$gdb_prompt $" {
+	kfail "gdb/9999" $test
+    }
+}
 
 # Re: RFC: fix DW_AT_data_member_location buglet
 # http://sourceware.org/ml/gdb-patches/2011-05/msg00291.html
--- a/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
@@ -41,7 +41,7 @@ if [is_remote host] {
     }
 }
 gdb_test_no_output "set complaints 100"
-gdb_test "file $binfile" {Reading symbols from .*\.\.\.Asked for position 0 of stack, stack only has 0 elements on it\..*\.\.\.done\.} "check partial symtab errors"
+gdb_test "file $binfile" {Reading symbols from .*\.\.\.location description stack underflow\.\.\.location description stack overflow\.\.\.done\.} "check partial symtab errors"
 
 gdb_test "p underflow" {Asked for position 0 of stack, stack only has 0 elements on it\.}
 gdb_test "p overflow" " = 2"