Re: [RFA v2 16/17] Convert dwarf_expr_context_funcs to methods

[Tom Tromey <tom@tromey.com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Unfortunately, this caused regressions:
Pedro>  https://sourceware.org/ml/gdb-testers/2016-q4/msg01382.html

Sorry about this.  Something must be wrong with regression testing here,
because I see these failures when I run the tests individually, but I
never saw them when comparing against the baseline.

I'll set up to use the try builder tomorrow and then rely on that
instead.  Meanwhile here's the patch I'll be testing.

Tom

commit 0b88198c29a9773741b8450f79e4fe1ee535efac
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Oct 30 20:45:08 2016 -0600

    Fix dwarf_expr_context method regressions
    
    This fixes some regressions found in the patch to convert
    dwarf_expr_context to use methods.  Specifically:
    
    * get_base_type could erroneously throw; this was rewritten to move
      the size checks into the only spot needing them.
    * Previously the "symbol needs frame" implementation reused th
      "cfa" function for the get_frame_pc slot; this reimplements
      it under the correct name.
    * Not enough members were saved and restored in one implementation
      of push_dwarf_reg_entry_value; this patch fixes this oversight
      and also takes the opportunity to remove an extraneous structure
      definition.
    
    2016-10-30  Tom Tromey  <tom@tromey.com>
    
    	* dwarf2loc.c (dwarf_evaluate_loc_desc::get_base_type): Rename
    	from impl_get_base_type.  Rewrite.
    	(struct dwarf_expr_baton): Remove.
    	(dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value): Save and
    	restore more fields.
    	(symbol_needs_eval_context::get_frame_pc): New method.
    	* dwarf2expr.h (dwarf_expr_context::get_base_type): Now public,
    	virtual.
    	(dwarf_expr_context::impl_get_base_type): Remove.
    	* dwarf2expr.c (dwarf_expr_context::get_base_type): Remove.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c38c65c..ac5b0ce 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2016-10-30  Tom Tromey  <tom@tromey.com>
+
+	* dwarf2loc.c (dwarf_evaluate_loc_desc::get_base_type): Rename
+	from impl_get_base_type.  Rewrite.
+	(struct dwarf_expr_baton): Remove.
+	(dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value): Save and
+	restore more fields.
+	(symbol_needs_eval_context::get_frame_pc): New method.
+	* dwarf2expr.h (dwarf_expr_context::get_base_type): Now public,
+	virtual.
+	(dwarf_expr_context::impl_get_base_type): Remove.
+	* dwarf2expr.c (dwarf_expr_context::get_base_type): Remove.
+
 2016-10-29  Pedro Alves  <palves@redhat.com>
 
 	* NEWS: Clarify C++ requirement.
diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index a01d6d8..398ca0e 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -406,22 +406,6 @@ base_types_equal_p (struct type *t1, struct type *t2)
   return TYPE_LENGTH (t1) == TYPE_LENGTH (t2);
 }
 
-/* A convenience function to call get_base_type and return the result.
-   DIE is the DIE whose type we need.  SIZE is non-zero if this
-   function should verify that the resulting type has the correct
-   size.  */
-
-struct type *
-dwarf_expr_context::get_base_type (cu_offset die, int size)
-{
-  struct type *result = this->impl_get_base_type (die);
-  if (result == NULL)
-    error (_("Could not find type for DW_OP_GNU_const_type"));
-  if (size != 0 && TYPE_LENGTH (result) != size)
-    error (_("DW_OP_GNU_const_type has different sizes for type and data"));
-  return result;
-}
-
 /* If <BUF..BUF_END] contains DW_FORM_block* with single DW_OP_reg* return the
    DWARF register number.  Otherwise return -1.  */
 
diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h
index 3d08120..883c54c 100644
--- a/gdb/dwarf2expr.h
+++ b/gdb/dwarf2expr.h
@@ -179,10 +179,9 @@ struct dwarf_expr_context
 
   /* Return the base type given by the indicated DIE.  This can throw
      an exception if the DIE is invalid or does not represent a base
-     type.  If can also be NULL in the special case where the
-     callbacks are not performing evaluation, and thus it is
-     meaningful to substitute a stub type of the correct size.  */
-  virtual struct type *impl_get_base_type (cu_offset die)
+     type.  SIZE is non-zero if this function should verify that the
+     resulting type has the correct size.  */
+  virtual struct type *get_base_type (cu_offset die, int size)
   {
     /* Anything will do.  */
     return builtin_type (this->gdbarch)->builtin_int;
@@ -210,7 +209,6 @@ private:
   void push (struct value *value, int in_stack_memory);
   int stack_empty_p () const;
   void add_piece (ULONGEST size, ULONGEST offset);
-  struct type *get_base_type (cu_offset die, int size);
   void execute_stack_op (const gdb_byte *op_ptr, const gdb_byte *op_end);
   void pop ();
 };
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 6f25314..93aec1f 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -515,10 +515,14 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
     per_cu_dwarf_call (this, die_offset, per_cu);
   }
 
-  /* Callback function for dwarf2_evaluate_loc_desc.  */
-  struct type *impl_get_base_type (cu_offset die_offset) OVERRIDE
+  struct type *get_base_type (cu_offset die_offset, int size) OVERRIDE
   {
-    return dwarf2_get_die_type (die_offset, per_cu);
+    struct type *result = dwarf2_get_die_type (die_offset, per_cu);
+    if (result == NULL)
+      error (_("Could not find type for DW_OP_GNU_const_type"));
+    if (size != 0 && TYPE_LENGTH (result) != size)
+      error (_("DW_OP_GNU_const_type has different sizes for type and data"));
+    return result;
   }
 
   /* Callback function for dwarf2_evaluate_loc_desc.
@@ -553,7 +557,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
   {
     struct frame_info *caller_frame;
     struct dwarf2_per_cu_data *caller_per_cu;
-    struct dwarf_expr_baton baton_local;
     struct call_site_parameter *parameter;
     const gdb_byte *data_src;
     size_t size;
@@ -570,17 +573,20 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
       throw_error (NO_ENTRY_VALUE_ERROR,
 		   _("Cannot resolve DW_AT_GNU_call_site_data_value"));
 
-    baton_local.frame = caller_frame;
-    baton_local.per_cu = caller_per_cu;
-    baton_local.obj_address = 0;
+    scoped_restore save_frame = make_scoped_restore (&this->frame,
+						     caller_frame);
+    scoped_restore save_per_cu = make_scoped_restore (&this->per_cu,
+						      caller_per_cu);
+    scoped_restore save_obj_addr = make_scoped_restore (&this->obj_address,
+							(CORE_ADDR) 0);
 
     scoped_restore save_arch = make_scoped_restore (&this->gdbarch);
     this->gdbarch
-      = get_objfile_arch (dwarf2_per_cu_objfile (baton_local.per_cu));
+      = get_objfile_arch (dwarf2_per_cu_objfile (per_cu));
     scoped_restore save_addr_size = make_scoped_restore (&this->addr_size);
-    this->addr_size = dwarf2_per_cu_addr_size (baton_local.per_cu);
+    this->addr_size = dwarf2_per_cu_addr_size (per_cu);
     scoped_restore save_offset = make_scoped_restore (&this->offset);
-    this->offset = dwarf2_per_cu_text_offset (baton_local.per_cu);
+    this->offset = dwarf2_per_cu_text_offset (per_cu);
 
     this->eval (data_src, size);
   }
@@ -2707,6 +2713,12 @@ class symbol_needs_eval_context : public dwarf_expr_context
     return 1;
   }
 
+  CORE_ADDR get_frame_pc () OVERRIDE
+  {
+    needs = SYMBOL_NEEDS_FRAME;
+    return 1;
+  }
+
   /* Thread-local accesses require registers, but not a frame.  */
   CORE_ADDR get_tls_address (CORE_ADDR offset) OVERRIDE
   {