[PATCH 02/30] Move frame context info to dwarf_expr_context

[Zoran Zaric <Zoran.Zaric@amd.com>]

From: Zoran Zaric <zoran.zaric@amd.com>

Following 15 patches in this patch series is cleaning up the design of
the DWARF expression evaluator (dwarf_expr_context) to make future
extensions of that evaluator easier and cleaner to implement.

There are three subclasses of the dwarf_expr_context class
(dwarf_expr_executor, dwarf_evaluate_loc_desc and
evaluate_for_locexpr_baton). Here is a short description of each class:

- dwarf_expr_executor is evaluating a DWARF expression in a context
  of a Call Frame Information. The overridden methods of this subclass
  report an error if a specific DWARF operation, represented by that
  method, is not allowed in a CFI context. The source code of this
  subclass lacks the support for composite as well as implicit pointer
  location description.

- dwarf_evaluate_loc_desc can evaluate any expression with no
  restrictions. All of the methods that this subclass overrides are
  actually doing what they are intended to do. This subclass contains
  a full support for all location description types.

- evaluate_for_locexpr_baton subclass is a specialization of the
  dwarf_evaluate_loc_desc subclass and it’s function is to add
  support for passed in buffers. This seems to be a way to go around
  the fact that DWARF standard lacks a bit offset support for memory
  location descriptions as well as using any location description for
  the push object address functionality.

It all comes down to this question: what is a function of a DWARF
expression evaluator?

Is it to evaluate the expression in a given context or to check the
correctness of that expression in that context?

Currently, the only reason why there is a dwarf_expr_executor subclass
is to report an invalid DWARF expression in a context of a CFI, but is
that what the evaluator is supposed to do considering that the evaluator
is not tied to a given DWARF version?

There are more and more vendor and GNU extensions that are not part of
the DWARF standard, so is it that impossible to expect that some of the
extensions could actually lift the previously imposed restrictions of
the CFI context? Not to mention that every new DWARF version is lifting
some restrictions anyway.

The thing that makes more sense for an evaluator to do, is to take the
context of an evaluation and checks the requirements of every operation
evaluated against that context. With this approach, the evaluator would
report an error only if parts of the context, necessary for the
evaluation, are missing.

If this approach is taken, then the unification of the
dwarf_evaluate_loc_desc, dwarf_expr_executor and dwarf_expr_context
is the next logical step. This makes a design of the DWARF expression
evaluator cleaner and allows more flexibility when supporting future
vendor and GNU extensions.

Additional benefit here is that now all evaluators have access to all
location description types, which means that a vendor extended CFI
rules could support composite location description as well. This also
means that a new evaluator interface can be changed to return a single
struct value (that describes the result of the evaluation) instead of
a caller poking around the dwarf_expr_context internal data for answers
(like it is done currently).

This patch starts the merging proccess by moving the frame context
information and support from dwarf_expr_executor and
dwarf_evaluate_loc_desc to dwarf_expr_context evaluator. The idea
is to report an error when a given operation requires a frame
information to be resolved, if that information is not present.

gdb/ChangeLog:

	* dwarf2/expr.c (ensure_have_frame): New function.
	(read_addr_from_reg): Add from frame.c.
	(dwarf_expr_context::dwarf_expr_context): Add frame info to
	dwarf_expr_context.
	(dwarf_expr_context::read_addr_from_reg): Remove.
	(dwarf_expr_context::get_reg_value): Move from
	dwarf_evaluate_loc_desc.
	(dwarf_expr_context::get_frame_base): Move from
	dwarf_evaluate_loc_desc.
	(dwarf_expr_context::execute_stack_op): Call frame context info
	check. Remove use of read_addr_from_reg method.
	* dwarf2/expr.h (struct dwarf_expr_context): Add frame info
	member, read_addr_from_reg, get_reg_value and get_frame_base
	declaration.
	(read_addr_from_reg): Move to expr.c.
	* dwarf2/frame.c (read_addr_from_reg): Move to
	dwarf_expr_context.
	(dwarf_expr_executor::read_addr_from_reg): Remove.
	(dwarf_expr_executor::get_frame_base): Remove.
	(dwarf_expr_executor::get_reg_value): Remove.
	(execute_stack_op): Use read_addr_from_reg function instead of
	read_addr_from_reg method.
	* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_frame_base): Move
	to dwarf_expr_context.
	(dwarf_evaluate_loc_desc::get_reg_value): Move to
	dwarf_expr_context.
	(dwarf_evaluate_loc_desc::read_addr_from_reg): Remove.
	(dwarf2_locexpr_baton_eval):Use read_addr_from_reg function
	instead of read_addr_from_reg method.
---
 gdb/dwarf2/expr.c  | 76 +++++++++++++++++++++++++++++++++++++++++++---
 gdb/dwarf2/expr.h  | 31 ++++++++++---------
 gdb/dwarf2/frame.c | 36 ++--------------------
 gdb/dwarf2/loc.c   | 52 +------------------------------
 4 files changed, 92 insertions(+), 103 deletions(-)

diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index eaeb76f9c6..91d0b99640 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "block.h"
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "value.h"
@@ -56,6 +57,27 @@ dwarf_gdbarch_types_init (struct gdbarch *gdbarch)
   return types;
 }
 
+/* Ensure that a FRAME is defined, throw an exception otherwise.  */
+
+static void
+ensure_have_frame (struct frame_info *frame, const char *op_name)
+{
+  if (frame == nullptr)
+    throw_error (GENERIC_ERROR,
+		 _("%s evaluation requires a frame."), op_name);
+}
+
+/* See expr.h.  */
+
+CORE_ADDR
+read_addr_from_reg (struct frame_info *frame, int reg)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  int regnum = dwarf_reg_to_regnum_or_error (gdbarch, reg);
+
+  return address_from_register (regnum, frame);
+}
+
 /* Return the type used for DWARF operations where the type is
    unspecified in the DWARF spec.  Only certain sizes are
    supported.  */
@@ -99,7 +121,8 @@ dwarf_expr_context::dwarf_expr_context (dwarf2_per_objfile *per_objfile)
   len (0),
   data (NULL),
   initialized (0),
-  per_objfile (per_objfile)
+  per_objfile (per_objfile),
+  frame (nullptr)
 {
 }
 
@@ -142,6 +165,47 @@ dwarf_expr_context::fetch (int n)
   return stack[stack.size () - (1 + n)].value;
 }
 
+/* See expr.h.  */
+
+struct value *
+dwarf_expr_context::get_reg_value (struct type *type, int reg)
+{
+  ensure_have_frame (frame, "DW_OP_regval_type");
+
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  int regnum = dwarf_reg_to_regnum_or_error (gdbarch, reg);
+
+  return value_from_register (type, regnum, frame);
+}
+
+/* See expr.h.  */
+
+void
+dwarf_expr_context::get_frame_base (const gdb_byte **start,
+				    size_t * length)
+{
+  ensure_have_frame (frame, "DW_OP_fbreg");
+
+  const struct block *bl = get_frame_block (frame, NULL);
+
+  if (bl == NULL)
+    error (_("frame address is not available."));
+
+  /* Use block_linkage_function, which returns a real (not inlined)
+     function, instead of get_frame_function, which may return an
+     inlined function.  */
+  struct symbol *framefunc = block_linkage_function (bl);
+
+  /* If we found a frame-relative symbol then it was certainly within
+     some function associated with a frame. If we can't find the frame,
+     something has gone wrong.  */
+  gdb_assert (framefunc != NULL);
+
+  func_get_frame_base_dwarf_block (framefunc,
+				   get_frame_address_in_block (frame),
+				   start, length);
+}
+
 /* Require that TYPE be an integral type; throw an exception if not.  */
 
 static void
@@ -830,7 +894,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
 	case DW_OP_breg31:
 	  {
 	    op_ptr = safe_read_sleb128 (op_ptr, op_end, &offset);
-	    result = this->read_addr_from_reg (op - DW_OP_breg0);
+	    ensure_have_frame (this->frame, "DW_OP_breg");
+
+	    result = read_addr_from_reg (this->frame, op - DW_OP_breg0);
 	    result += offset;
 	    result_val = value_from_ulongest (address_type, result);
 	  }
@@ -839,7 +905,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
 	  {
 	    op_ptr = safe_read_uleb128 (op_ptr, op_end, &reg);
 	    op_ptr = safe_read_sleb128 (op_ptr, op_end, &offset);
-	    result = this->read_addr_from_reg (reg);
+	    ensure_have_frame (this->frame, "DW_OP_bregx");
+
+	    result = read_addr_from_reg (this->frame, reg);
 	    result += offset;
 	    result_val = value_from_ulongest (address_type, result);
 	  }
@@ -866,7 +934,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
 	    if (this->location == DWARF_VALUE_MEMORY)
 	      result = fetch_address (0);
 	    else if (this->location == DWARF_VALUE_REGISTER)
-	      result = this->read_addr_from_reg (value_as_long (fetch (0)));
+	      result = read_addr_from_reg (this->frame, value_as_long (fetch (0)));
 	    else
 	      error (_("Not implemented: computing frame "
 		       "base using explicit value operator"));
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index fd9c2bb624..140d3c36db 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -186,24 +186,12 @@ struct dwarf_expr_context
   /* We evaluate the expression in the context of this objfile.  */
   dwarf2_per_objfile *per_objfile;
 
-  /* Return the value of register number REGNUM (a DWARF register number),
-     read as an address.  */
-  virtual CORE_ADDR read_addr_from_reg (int regnum) = 0;
-
-  /* Return a value of type TYPE, stored in register number REGNUM
-     of the frame associated to the given BATON.
-
-     REGNUM is a DWARF register number.  */
-  virtual struct value *get_reg_value (struct type *type, int regnum) = 0;
+  /* Frame information used for the evaluation.  */
+  struct frame_info *frame;
 
   /* Read LENGTH bytes at ADDR into BUF.  */
   virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
 
-  /* Return the location expression for the frame base attribute, in
-     START and LENGTH.  The result must be live until the current
-     expression evaluation is complete.  */
-  virtual void get_frame_base (const gdb_byte **start, size_t *length) = 0;
-
   /* Return the CFA for the frame.  */
   virtual CORE_ADDR get_frame_cfa () = 0;
 
@@ -259,8 +247,23 @@ struct dwarf_expr_context
   void add_piece (ULONGEST size, ULONGEST offset);
   void execute_stack_op (const gdb_byte *op_ptr, const gdb_byte *op_end);
   void pop ();
+
+  /* Return a value of type TYPE, stored in register number REGNUM
+     of the frame associated to the given BATON.
+
+     REGNUM is a DWARF register number.  */
+  struct value *get_reg_value (struct type *type, int regnum);
+
+  /* Return the location expression for the frame base attribute, in
+     START and LENGTH.  The result must be live until the current
+     expression evaluation is complete.  */
+  void get_frame_base (const gdb_byte **start, size_t *length);
 };
 
+/* Return the value of register number REG (a DWARF register number),
+   read as an address in a given FRAME.  */
+CORE_ADDR read_addr_from_reg (struct frame_info *, int);
+
 void dwarf_expr_require_composition (const gdb_byte *, const gdb_byte *,
 				     const char *);
 
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 7df3ccfe46..ce1f9e2763 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -191,18 +191,6 @@ dwarf2_frame_state::dwarf2_frame_state (CORE_ADDR pc_, struct dwarf2_cie *cie)
     retaddr_column (cie->return_address_register)
 {
 }
-\f
-
-/* Helper functions for execute_stack_op.  */
-
-static CORE_ADDR
-read_addr_from_reg (struct frame_info *this_frame, int reg)
-{
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  int regnum = dwarf_reg_to_regnum_or_error (gdbarch, reg);
-
-  return address_from_register (regnum, this_frame);
-}
 
 /* Execute the required actions for both the DW_CFA_restore and
 DW_CFA_restore_extended instructions.  */
@@ -243,31 +231,11 @@ class dwarf_expr_executor : public dwarf_expr_context
     : dwarf_expr_context (per_objfile)
   {}
 
-  struct frame_info *this_frame;
-
-  CORE_ADDR read_addr_from_reg (int reg) override
-  {
-    return ::read_addr_from_reg (this_frame, reg);
-  }
-
-  struct value *get_reg_value (struct type *type, int reg) override
-  {
-    struct gdbarch *gdbarch = get_frame_arch (this_frame);
-    int regnum = dwarf_reg_to_regnum_or_error (gdbarch, reg);
-
-    return value_from_register (type, regnum, this_frame);
-  }
-
   void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
   {
     read_memory (addr, buf, len);
   }
 
-  void get_frame_base (const gdb_byte **start, size_t *length) override
-  {
-    invalid ("DW_OP_fbreg");
-  }
-
   void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
 				   union call_site_parameter_u kind_u,
 				   int deref_size) override
@@ -323,7 +291,7 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
   dwarf_expr_executor ctx (per_objfile);
   scoped_value_mark free_values;
 
-  ctx.this_frame = this_frame;
+  ctx.frame = this_frame;
   ctx.gdbarch = get_frame_arch (this_frame);
   ctx.addr_size = addr_size;
   ctx.ref_addr_size = -1;
@@ -334,7 +302,7 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
   if (ctx.location == DWARF_VALUE_MEMORY)
     result = ctx.fetch_address (0);
   else if (ctx.location == DWARF_VALUE_REGISTER)
-    result = ctx.read_addr_from_reg (value_as_long (ctx.fetch (0)));
+    result = read_addr_from_reg (this_frame, value_as_long (ctx.fetch (0)));
   else
     {
       /* This is actually invalid DWARF, but if we ever do run across
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index d5da8772be..8913051e4e 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -654,7 +654,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
     : dwarf_expr_context (per_objfile)
   {}
 
-  struct frame_info *frame;
   struct dwarf2_per_cu_data *per_cu;
   CORE_ADDR obj_address;
 
@@ -780,61 +779,12 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
     this->eval (data_src, size);
   }
 
-  /* Using the frame specified in BATON, find the location expression
-     describing the frame base.  Return a pointer to it in START and
-     its length in LENGTH.  */
-  void get_frame_base (const gdb_byte **start, size_t * length) override
-  {
-    /* FIXME: cagney/2003-03-26: This code should be using
-       get_frame_base_address(), and then implement a dwarf2 specific
-       this_base method.  */
-    struct symbol *framefunc;
-    const struct block *bl = get_frame_block (frame, NULL);
-
-    if (bl == NULL)
-      error (_("frame address is not available."));
-
-    /* Use block_linkage_function, which returns a real (not inlined)
-       function, instead of get_frame_function, which may return an
-       inlined function.  */
-    framefunc = block_linkage_function (bl);
-
-    /* If we found a frame-relative symbol then it was certainly within
-       some function associated with a frame. If we can't find the frame,
-       something has gone wrong.  */
-    gdb_assert (framefunc != NULL);
-
-    func_get_frame_base_dwarf_block (framefunc,
-				     get_frame_address_in_block (frame),
-				     start, length);
-  }
-
   /* Read memory at ADDR (length LEN) into BUF.  */
 
   void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
   {
     read_memory (addr, buf, len);
   }
-
-  /* Using the frame specified in BATON, return the value of register
-     REGNUM, treated as a pointer.  */
-  CORE_ADDR read_addr_from_reg (int dwarf_regnum) override
-  {
-    struct gdbarch *gdbarch = get_frame_arch (frame);
-    int regnum = dwarf_reg_to_regnum_or_error (gdbarch, dwarf_regnum);
-
-    return address_from_register (regnum, frame);
-  }
-
-  /* Implement "get_reg_value" callback.  */
-
-  struct value *get_reg_value (struct type *type, int dwarf_regnum) override
-  {
-    struct gdbarch *gdbarch = get_frame_arch (frame);
-    int regnum = dwarf_reg_to_regnum_or_error (gdbarch, dwarf_regnum);
-
-    return value_from_register (type, regnum, frame);
-  }
 };
 
 /* See dwarf2loc.h.  */
@@ -2555,7 +2505,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
     case DWARF_VALUE_MEMORY:
       *valp = ctx.fetch_address (0);
       if (ctx.location == DWARF_VALUE_REGISTER)
-	*valp = ctx.read_addr_from_reg (*valp);
+	*valp = read_addr_from_reg (frame, *valp);
       return 1;
     case DWARF_VALUE_LITERAL:
       *valp = extract_signed_integer (ctx.data, ctx.len,
-- 
2.17.1