[PATCH 15/43] Make DWARF evaluator return a single struct value

Zoran Zaric via Gdb-patches
2021-03-01T14:47:11Z

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

The patch is addressing the issue of class users writing and reading
the internal data of the dwarf_expr_context class.

At this point, all conditions are met for the DWARF evaluator to return
an evaluation result in a form of a single struct value object.

gdb/ChangeLog:

* dwarf2/expr.c (pieced_value_funcs): Chenge to static
function.
(allocate_piece_closure): Change to static function.
(dwarf_expr_context::fetch_result): New function.
* dwarf2/expr.h (struct piece_closure): Remove declaration.
(struct dwarf_expr_context): fetch_result new declaration.
fetch, fetch_address and fetch_in_stack_memory members move
to private.
(allocate_piece_closure): Remove.
* dwarf2/frame.c (execute_stack_op): Change to use
fetch_result.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to use
fetch_result.
(dwarf2_locexpr_baton_eval): Change to use fetch_result.
* dwarf2/loc.h (invalid_synthetic_pointer): Expose function.
---
gdb/dwarf2/expr.c | 163 ++++++++++++++++++++++++++++++++++++-
gdb/dwarf2/expr.h | 25 +++---
gdb/dwarf2/frame.c | 19 ++---
gdb/dwarf2/loc.c | 195 ++++++---------------------------------------
gdb/dwarf2/loc.h | 5 ++
5 files changed, 207 insertions(+), 200 deletions(-)

diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 486521b5e04..b2248681899 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -125,9 +125,10 @@ struct piece_closure
struct frame_id frame_id;
};

-/* See expr.h. */
+/* Allocate a closure for a value formed from separately-described
+ PIECES. */

-struct piece_closure *
+static struct piece_closure *
allocate_piece_closure (dwarf2_per_cu_data *per_cu,
dwarf2_per_objfile *per_objfile,
std::vector<dwarf_expr_piece> &&pieces,
@@ -628,7 +629,7 @@ free_pieced_value_closure (struct value *v)
}

/* Functions for accessing a variable described by DW_OP_piece. */
-const struct lval_funcs pieced_value_funcs = {
+static const struct lval_funcs pieced_value_funcs = {
read_pieced_value,
write_pieced_value,
indirect_pieced_value,
@@ -886,6 +887,162 @@ dwarf_expr_context::push_dwarf_reg_entry_value
this->eval (data_src, size);
}

+/* See expr.h. */
+
+struct value *
+dwarf_expr_context::fetch_result (struct type *type,
+ struct type *subobj_type,
+ LONGEST subobj_offset)
+{
+ struct value *retval = nullptr;
+
+ if (type == nullptr)
+ type = address_type ();
+
+ if (subobj_type == nullptr)
+ subobj_type = type;
+
+ if (this->pieces.size () > 0)
+ {
+ struct piece_closure *c;
+ ULONGEST bit_size = 0;
+
+ for (dwarf_expr_piece &piece : this->pieces)
+ bit_size += piece.size;
+ /* Complain if the expression is larger than the size of the
+ outer type. */
+ if (bit_size > 8 * TYPE_LENGTH (type))
+ invalid_synthetic_pointer ();
+
+ c = allocate_piece_closure (this->per_cu, this->per_objfile,
+ std::move (this->pieces), this->frame);
+ retval = allocate_computed_value (subobj_type,
+ &pieced_value_funcs, c);
+ set_value_offset (retval, subobj_offset);
+ }
+ else
+ {
+ switch (this->location)
+ {
+ case DWARF_VALUE_REGISTER:
+ {
+ int dwarf_regnum
+ = longest_to_int (value_as_long (this->fetch (0)));
+ int gdb_regnum = dwarf_reg_to_regnum_or_error (this->gdbarch,
+ dwarf_regnum);
+
+ if (subobj_offset != 0)
+ error (_("cannot use offset on synthetic pointer to register"));
+
+ gdb_assert (this->frame != NULL);
+
+ retval = value_from_register (subobj_type, gdb_regnum,
+ this->frame);
+ if (value_optimized_out (retval))
+ {
+ struct value *tmp;
+
+ /* This means the register has undefined value / was
+ not saved. As we're computing the location of some
+ variable etc. in the program, not a value for
+ inspecting a register ($pc, $sp, etc.), return a
+ generic optimized out value instead, so that we show
+ <optimized out> instead of <not saved>. */
+ tmp = allocate_value (subobj_type);
+ value_contents_copy (tmp, 0, retval, 0,
+ TYPE_LENGTH (subobj_type));
+ retval = tmp;
+ }
+ }
+ break;
+
+ case DWARF_VALUE_MEMORY:
+ {
+ struct type *ptr_type;
+ CORE_ADDR address = this->fetch_address (0);
+ bool in_stack_memory = this->fetch_in_stack_memory (0);
+
+ /* DW_OP_deref_size (and possibly other operations too) may
+ create a pointer instead of an address. Ideally, the
+ pointer to address conversion would be performed as part
+ of those operations, but the type of the object to
+ which the address refers is not known at the time of
+ the operation. Therefore, we do the conversion here
+ since the type is readily available. */
+
+ switch (subobj_type->code ())
+ {
+ case TYPE_CODE_FUNC:
+ case TYPE_CODE_METHOD:
+ ptr_type = builtin_type (this->gdbarch)->builtin_func_ptr;
+ break;
+ default:
+ ptr_type = builtin_type (this->gdbarch)->builtin_data_ptr;
+ break;
+ }
+ address = value_as_address (value_from_pointer (ptr_type, address));
+
+ retval = value_at_lazy (subobj_type,
+ address + subobj_offset);
+ if (in_stack_memory)
+ set_value_stack (retval, 1);
+ }
+ break;
+
+ case DWARF_VALUE_STACK:
+ {
+ struct value *value = this->fetch (0);
+ size_t n = TYPE_LENGTH (value_type (value));
+ size_t len = TYPE_LENGTH (subobj_type);
+ size_t max = TYPE_LENGTH (type);
+
+ if (subobj_offset + len > max)
+ invalid_synthetic_pointer ();
+
+ retval = allocate_value (subobj_type);
+
+ /* The given offset is relative to the actual object. */
+ if (gdbarch_byte_order (this->gdbarch) == BFD_ENDIAN_BIG)
+ subobj_offset += n - max;
+
+ memcpy (value_contents_raw (retval),
+ value_contents_all (value) + subobj_offset, len);
+ }
+ break;
+
+ case DWARF_VALUE_LITERAL:
+ {
+ bfd_byte *contents;
+ size_t n = TYPE_LENGTH (subobj_type);
+
+ if (subobj_offset + n > this->len)
+ invalid_synthetic_pointer ();
+
+ retval = allocate_value (subobj_type);
+ contents = value_contents_raw (retval);
+ memcpy (contents, this->data + subobj_offset, n);
+ }
+ break;
+
+ case DWARF_VALUE_OPTIMIZED_OUT:
+ retval = allocate_optimized_out_value (subobj_type);
+ break;
+
+ /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced
+ operation by execute_stack_op. */
+ case DWARF_VALUE_IMPLICIT_POINTER:
+ /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context --
+ it can only be encountered when making a piece. */
+ default:
+ internal_error (__FILE__, __LINE__, _("invalid location type"));
+ }
+ }
+
+ set_value_initialized (retval, this->initialized);
+
+ return retval;
+}
+
/* Require that TYPE be an integral type; throw an exception if not. */

static void
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index 3ac19e5ba0b..a0ac21f2ed1 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -26,7 +26,6 @@
#include "gdbtypes.h"

struct dwarf2_per_objfile;
-struct piece_closure;

/* The location of a value. */
enum dwarf_value_location
@@ -125,9 +124,13 @@ struct dwarf_expr_context

void push_address (CORE_ADDR value, bool in_stack_memory);
void eval (const gdb_byte *addr, size_t len);
- struct value *fetch (int n);
- CORE_ADDR fetch_address (int n);
- bool fetch_in_stack_memory (int n);
+
+ /* Fetch the result of the expression evaluation in a form of
+ a struct value, where TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET
+ describe the source level representation of that result. */
+ struct value *fetch_result (struct type *type = nullptr,
+ struct type *subobj_type = nullptr,
+ LONGEST subobj_offset = 0);

/* The stack of values. */
std::vector<dwarf_stack_value> stack;
@@ -207,6 +210,9 @@ 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 ();
+ struct value *fetch (int n);
+ CORE_ADDR fetch_address (int n);
+ bool fetch_in_stack_memory (int n);

/* Return the location expression for the frame base attribute, in
START and LENGTH. The result must be live until the current
@@ -301,15 +307,4 @@ extern const gdb_byte *safe_read_sleb128 (const gdb_byte *buf,
extern const gdb_byte *safe_skip_leb128 (const gdb_byte *buf,
const gdb_byte *buf_end);

-extern const struct lval_funcs pieced_value_funcs;
-
-/* Allocate a closure for a value formed from separately-described
- PIECES. */
-
-struct piece_closure *allocate_piece_closure
- (dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile,
- std::vector<dwarf_expr_piece> &&pieces,
- struct frame_info *frame);
-
#endif /* dwarf2expr.h */
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 8faadeb3469..21daecec61c 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -228,8 +228,6 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
struct frame_info *this_frame, CORE_ADDR initial,
int initial_in_stack_memory, dwarf2_per_objfile *per_objfile)
{
- CORE_ADDR result;
-
dwarf_expr_context ctx (per_objfile);
scoped_value_mark free_values;

@@ -241,18 +239,13 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
ctx.push_address (initial, initial_in_stack_memory);
ctx.eval (exp, len);

- if (ctx.location == DWARF_VALUE_MEMORY)
- result = ctx.fetch_address (0);
- else if (ctx.location == DWARF_VALUE_REGISTER)
- result = read_addr_from_reg (this_frame, value_as_long (ctx.fetch (0)));
+ CORE_ADDR result;
+ struct value *result_val = ctx.fetch_result ();
+
+ if (VALUE_LVAL (result_val) == lval_memory)
+ result = value_address (result_val);
else
- {
- /* This is actually invalid DWARF, but if we ever do run across
- it somehow, we might as well support it. So, instead, report
- it as unimplemented. */
- error (_("\
-Not implemented: computing unwound register using explicit value operator"));
- }
+ result = value_as_address (result_val);

return result;
}
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 92ff3111a5a..e2d6b32717b 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -91,7 +91,7 @@ enum debug_loc_kind

/* See loc.h. */

-static void
+void
invalid_synthetic_pointer (void)
{
error (_("access outside bounds of object "
@@ -1457,6 +1457,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
try
{
ctx.eval (data, size);
+ retval = ctx.fetch_result (type, subobj_type,
+ subobj_byte_offset);
}
catch (const gdb_exception_error &ex)
{
@@ -1479,155 +1481,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
throw;
}

- if (ctx.pieces.size () > 0)
- {
- struct piece_closure *c;
- ULONGEST bit_size = 0;
-
- for (dwarf_expr_piece &piece : ctx.pieces)
- bit_size += piece.size;
- /* Complain if the expression is larger than the size of the
- outer type. */
- if (bit_size > 8 * TYPE_LENGTH (type))
- invalid_synthetic_pointer ();
-
- c = allocate_piece_closure (per_cu, per_objfile, std::move (ctx.pieces),
- frame);
- /* We must clean up the value chain after creating the piece
- closure but before allocating the result. */
- free_values.free_to_mark ();
- retval = allocate_computed_value (subobj_type,
- &pieced_value_funcs, c);
- set_value_offset (retval, subobj_byte_offset);
- }
- else
- {
- switch (ctx.location)
- {
- case DWARF_VALUE_REGISTER:
- {
- struct gdbarch *arch = get_frame_arch (frame);
- int dwarf_regnum
- = longest_to_int (value_as_long (ctx.fetch (0)));
- int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, dwarf_regnum);
-
- if (subobj_byte_offset != 0)
- error (_("cannot use offset on synthetic pointer to register"));
- free_values.free_to_mark ();
- retval = value_from_register (subobj_type, gdb_regnum, frame);
- if (value_optimized_out (retval))
- {
- struct value *tmp;
-
- /* This means the register has undefined value / was
- not saved. As we're computing the location of some
- variable etc. in the program, not a value for
- inspecting a register ($pc, $sp, etc.), return a
- generic optimized out value instead, so that we show
- <optimized out> instead of <not saved>. */
- tmp = allocate_value (subobj_type);
- value_contents_copy (tmp, 0, retval, 0,
- TYPE_LENGTH (subobj_type));
- retval = tmp;
- }
- }
- break;
-
- case DWARF_VALUE_MEMORY:
- {
- struct type *ptr_type;
- CORE_ADDR address = ctx.fetch_address (0);
- bool in_stack_memory = ctx.fetch_in_stack_memory (0);
-
- /* DW_OP_deref_size (and possibly other operations too) may
- create a pointer instead of an address. Ideally, the
- pointer to address conversion would be performed as part
- of those operations, but the type of the object to
- which the address refers is not known at the time of
- the operation. Therefore, we do the conversion here
- since the type is readily available. */
-
- switch (subobj_type->code ())
- {
- case TYPE_CODE_FUNC:
- case TYPE_CODE_METHOD:
- ptr_type = builtin_type (ctx.gdbarch)->builtin_func_ptr;
- break;
- default:
- ptr_type = builtin_type (ctx.gdbarch)->builtin_data_ptr;
- break;
- }
- address = value_as_address (value_from_pointer (ptr_type, address));
-
- free_values.free_to_mark ();
- retval = value_at_lazy (subobj_type,
- address + subobj_byte_offset);
- if (in_stack_memory)
- set_value_stack (retval, 1);
- }
- break;
-
- case DWARF_VALUE_STACK:
- {
- struct value *value = ctx.fetch (0);
- size_t n = TYPE_LENGTH (value_type (value));
- size_t len = TYPE_LENGTH (subobj_type);
- size_t max = TYPE_LENGTH (type);
- gdbarch *objfile_gdbarch = per_objfile->objfile->arch ();
-
- if (subobj_byte_offset + len > max)
- invalid_synthetic_pointer ();
-
- /* Preserve VALUE because we are going to free values back
- to the mark, but we still need the value contents
- below. */
- value_ref_ptr value_holder = value_ref_ptr::new_reference (value);
- free_values.free_to_mark ();
-
- retval = allocate_value (subobj_type);
-
- /* The given offset is relative to the actual object. */
- if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
- subobj_byte_offset += n - max;
-
- memcpy (value_contents_raw (retval),
- value_contents_all (value) + subobj_byte_offset, len);
- }
- break;
-
- case DWARF_VALUE_LITERAL:
- {
- bfd_byte *contents;
- size_t n = TYPE_LENGTH (subobj_type);
-
- if (subobj_byte_offset + n > ctx.len)
- invalid_synthetic_pointer ();
-
- free_values.free_to_mark ();
- retval = allocate_value (subobj_type);
- contents = value_contents_raw (retval);
- memcpy (contents, ctx.data + subobj_byte_offset, n);
- }
- break;
-
- case DWARF_VALUE_OPTIMIZED_OUT:
- free_values.free_to_mark ();
- retval = allocate_optimized_out_value (subobj_type);
- break;
-
- /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced
- operation by execute_stack_op. */
- case DWARF_VALUE_IMPLICIT_POINTER:
- /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context --
- it can only be encountered when making a piece. */
- default:
- internal_error (__FILE__, __LINE__, _("invalid location type"));
- }
- }
-
- set_value_initialized (retval, ctx.initialized);
+ /* We need to clean up all the values that are not needed any more.
+ The problem with a value_ref_ptr class is that it disconnects the
+ RETVAL from the value garbage collection, so we need to make
+ a copy of that value on the stack to keep everything consistent.
+ The value_ref_ptr will clean up after itself at the end of this block. */
+ value_ref_ptr value_holder = value_ref_ptr::new_reference (retval);
+ free_values.free_to_mark ();

- return retval;
+ return value_copy(retval);
}

/* The exported interface to dwarf2_evaluate_loc_desc_full; it always
@@ -1668,6 +1530,9 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
dwarf2_per_objfile *per_objfile = dlbaton->per_objfile;
dwarf_expr_context ctx (per_objfile);

+ struct value *result;
+ scoped_value_mark free_values;
+
ctx.frame = frame;
ctx.per_cu = dlbaton->per_cu;
if (addr_stack != nullptr)
@@ -1686,6 +1551,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
try
{
ctx.eval (dlbaton->data, dlbaton->size);
+ result = ctx.fetch_result ();
}
catch (const gdb_exception_error &ex)
{
@@ -1703,29 +1569,20 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
throw;
}

- switch (ctx.location)
+ if (value_optimized_out (result))
+ return 0;
+
+ if (VALUE_LVAL (result) == lval_memory)
+ *valp = value_address (result);
+ else
{
- case DWARF_VALUE_STACK:
- *is_reference = false;
- /* FALLTHROUGH */
-
- case DWARF_VALUE_REGISTER:
- case DWARF_VALUE_MEMORY:
- *valp = ctx.fetch_address (0);
- if (ctx.location == DWARF_VALUE_REGISTER)
- *valp = read_addr_from_reg (frame, *valp);
- return 1;
- case DWARF_VALUE_LITERAL:
- *valp = extract_signed_integer (ctx.data, ctx.len,
- gdbarch_byte_order (ctx.gdbarch));
- return 1;
- /* Unsupported dwarf values. */
- case DWARF_VALUE_OPTIMIZED_OUT:
- case DWARF_VALUE_IMPLICIT_POINTER:
- break;
+ if (VALUE_LVAL (result) == not_lval)
+ *is_reference = false;
+
+ *valp = value_as_address (result);
}

- return 0;
+ return 1;
}

/* See dwarf2loc.h. */
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 33388ef88ee..02c686e5592 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -278,6 +278,11 @@ extern int dwarf_reg_to_regnum (struct gdbarch *arch, int dwarf_reg);
extern int dwarf_reg_to_regnum_or_error (struct gdbarch *arch,
ULONGEST dwarf_reg);

+/* Helper function which throws an error if a synthetic pointer is
+ invalid. */
+
+extern void invalid_synthetic_pointer ();
+
/* Fetch the value pointed to by a synthetic pointer. */

extern struct value *indirect_synthetic_pointer
--
2.17.1