* [PATCH v3 00/17] DWARF expression evaluator design cleanup
@ 2021-05-28 15:46 Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 01/17] Replace the symbol needs evaluator with a parser Zoran Zaric
` (17 more replies)
0 siblings, 18 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
The idea of this patch series is to cleanup the design of the DWARF
expression evaluator (dwarf_expr_context class) and allow a future
extensions of that evaluator needed for the upcoming vendor specific
DWARF extensions.
Main motivation behind this series of patches is AMD’s effort to
improve DWARF support for heavily optimized code, but more
specifically, optimized code for SIMD and SIMT architectures.
For the interested parties, the AMD’s DWARF standard extensions
documentation can be found at:
https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html
That being said, the patch series can also be viewed as a standalone
series that introduces a welcome cleanup of the DWARF expression
evaluator module.
Zoran Zaric (17):
Replace the symbol needs evaluator with a parser
Cleanup of the dwarf_expr_context constructor
Move frame context info to dwarf_expr_context
Remove get_frame_cfa from dwarf_expr_context
Move compilation unit info to dwarf_expr_context
Move dwarf_call to dwarf_expr_context
Move get_object_address to dwarf_expr_context
Move read_mem to dwarf_expr_context
Move push_dwarf_reg_entry_value to expr.c
Inline get_reg_value method of dwarf_expr_context
Remove empty frame and full evaluators
Merge evaluate_for_locexpr_baton evaluator
Move piece_closure and its support to expr.c
Make value_copy also copy the stack data member
Make DWARF evaluator return a single struct value
Simplify dwarf_expr_context class interface
Add as_lval argument to expression evaluator
gdb/dwarf2/expr.c | 1190 ++++++++++--
gdb/dwarf2/expr.h | 152 +-
gdb/dwarf2/frame.c | 117 +-
gdb/dwarf2/loc.c | 1672 +++++------------
gdb/dwarf2/loc.h | 30 +-
gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c | 25 +
.../gdb.dwarf2/symbol_needs_eval_fail.exp | 112 ++
.../gdb.dwarf2/symbol_needs_eval_timeout.exp | 131 ++
.../amd64-py-framefilter-invalidarg.S | 1 -
gdb/testsuite/lib/dwarf.exp | 4 +
gdb/value.c | 2 +
11 files changed, 1927 insertions(+), 1509 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 01/17] Replace the symbol needs evaluator with a parser
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-06-07 21:48 ` Simon Marchi
2021-05-28 15:46 ` [PATCH v3 02/17] Cleanup of the dwarf_expr_context constructor Zoran Zaric
` (16 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
This patch addresses a design problem with the symbol_needs_eval_context
class. It exposes the problem by introducing two new testsuite test
cases.
To explain the issue, I first need to explain the dwarf_expr_context
class that the symbol_needs_eval_context class derives from.
The intention behind the dwarf_expr_context class is to commonize the
DWARF expression evaluation mechanism for different evaluation
contexts. Currently in gdb, the evaluation context can contain some or
all of the following information: architecture, object file, frame and
compilation unit.
Depending on the information needed to evaluate a given expression,
there are currently three distinct DWARF expression evaluators:
- Frame: designed to evaluate an expression in the context of a call
frame information (dwarf_expr_executor class). This evaluator doesn’t
need a compilation unit information.
- Location description: designed to evaluate an expression in the
context of a source level information (dwarf_evaluate_loc_desc
class). This evaluator expects all information needed for the
evaluation of the given expression to be present.
- Symbol needs: designed to answer a question about the parts of the
context information required to evaluate a DWARF expression behind a
given symbol (symbol_needs_eval_context class). This evaluator
doesn’t need a frame information.
The functional difference between the symbol needs evaluator and the
others is that this evaluator is not meant to interact with the actual
target. Instead, it is supposed to check which parts of the context
information are needed for the given DWARF expression to be evaluated by
the location description evaluator.
The idea is to take advantage of the existing dwarf_expr_context
evaluation mechanism and to fake all required interactions with the
actual target, by returning back dummy values. The evaluation result is
returned as one of three possible values, based on operations found in a
given expression:
- SYMBOL_NEEDS_NONE,
- SYMBOL_NEEDS_REGISTERS and
- SYMBOL_NEEDS_FRAME.
The problem here is that faking results of target interactions can yield
an incorrect evaluation result.
For example, if we have a conditional DWARF expression, where the
condition depends on a value read from an actual target, and the true
branch of the condition requires a frame information to be evaluated,
while the false branch doesn’t, fake target reads could conclude that a
frame information is not needed, where in fact it is. This wrong
information would then cause the expression to be actually evaluated (by
the location description evaluator) with a missing frame information.
This would then crash the debugger.
The gdb.dwarf2/symbol_needs_eval_fail.exp test introduces this
scenario, with the following DWARF expression:
DW_OP_addr $some_variable
DW_OP_deref
# conditional jump to DW_OP_bregx
DW_OP_bra 4
DW_OP_lit0
# jump to DW_OP_stack_value
DW_OP_skip 3
DW_OP_bregx $dwarf_regnum 0
DW_OP_stack_value
This expression describes a case where some variable dictates the
location of another variable. Depending on a value of some_variable, the
variable whose location is described by this expression is either read
from a register or it is defined as a constant value 0. In both cases,
the value will be returned as an implicit location description on the
DWARF stack.
Currently, when the symbol needs evaluator fakes a memory read from the
address behind the some_variable variable, the constant value 0 is used
as the value of the variable A, and the check returns the
SYMBOL_NEEDS_NONE result.
This is clearly a wrong result and it causes the debugger to crash.
The scenario might sound strange to some people, but it comes from a
SIMD/SIMT architecture where $some_variable is an execution mask. In
any case, it is a valid DWARF expression, and GDB shouldn't crash while
evaluating it. Also, a similar example could be made based on a
condition of the frame base value, where if that value is concluded to
be 0, the variable location could be defaulted to a TLS based memory
address.
The gdb.dwarf2/symbol_needs_eval_timeout.exp test introduces a second
scenario. This scenario is a bit more abstract due to the DWARF
assembler lacking the CFI support, but it exposes a different
manifestation of the same problem. Like in the previous scenario, the
DWARF expression used in the test is valid:
DW_OP_lit1
DW_OP_addr $some_variable
DW_OP_deref
# jump to DW_OP_fbreg
DW_OP_skip 4
DW_OP_drop
DW_OP_fbreg 0
DW_OP_dup
DW_OP_lit0
DW_OP_eq
# conditional jump to DW_OP_drop
DW_OP_bra -9
DW_OP_stack_value
Similarly to the previous scenario, the location of a variable A is an
implicit location description with a constant value that depends on a
value held by a global variable. The difference from the previous case
is that DWARF expression contains a loop instead of just one branch. The
end condition of that loop depends on the expectation that a frame base
value is never zero. Currently, the act of faking the target reads will
cause the symbol needs evaluator to get stuck in an infinite loop.
Somebody could argue that we could change the fake reads to return
something else, but that would only hide the real problem.
The general impression seems to be that the desired design is to have
one class that deals with parsing of the DWARF expression, while there
are virtual methods that deal with specifics of some operations.
Using an evaluator mechanism here doesn’t seem to be correct, because
the act of evaluation relies on accessing the data from the actual
target with the possibility of skipping the evaluation of some parts of
the expression.
To better explain the proposed solution for the issue, I first need to
explain a couple more details behind the current design:
There are multiple places in gdb that handle DWARF expression parsing
for different purposes. Some are in charge of converting the expression
to some other internal representation (decode_location_expression,
disassemble_dwarf_expression and dwarf2_compile_expr_to_ax), some are
analysing the expression for specific information
(compute_stack_depth_worker) and some are in charge of evaluating the
expression in a given context (dwarf_expr_context::execute_stack_op
and decode_locdesc).
The problem is that all those functions have a similar (large) switch
statement that handles each DWARF expression operation. The result of
this is a code duplication and harder maintenance.
As a step into the right direction to solve this problem (at least for
the purpose of a DWARF expression evaluation) the expression parsing was
commonized inside of an evaluator base class (dwarf_expr_context). This
makes sense for all derived classes, except for the symbol needs
evaluator (symbol_needs_eval_context) class.
As described previously the problem with this evaluator is that if the
evaluator is not allowed to access the actual target, it is not really
evaluating.
Instead, the desired function of a symbol needs evaluator seems to fall
more into expression analysis category. This means that a more natural
fit for this evaluator is to be a symbol needs analysis, similar to the
existing compute_stack_depth_worker analysis.
Another problem is that using a heavyweight mechanism of an evaluator
to do an expression analysis seems to be an unneeded overhead. It also
requires a more complicated design of the parent class to support fake
target reads.
The reality is that the whole symbol_needs_eval_context class can be
replaced with a lightweight recursive analysis function, that will give
more correct result without compromising the design of the
dwarf_expr_context class. The analysis treats the expression byte
stream as a DWARF operation graph, where each graph node can be
visited only once and each operation can decide if the frame context
is needed for their evaluation.
The downside of this approach is adding of one more similar switch
statement, but at least this way the new symbol needs analysis will be
a lightweight mechnism and it will provide a correct result for any
given DWARF expression.
A more desired long term design would be to have one class that deals
with parsing of the DWARF expression, while there would be a virtual
methods that deal with specifics of some DWARF operations. Then that
class would be used as a base for all DWARF expression parsing mentioned
at the beginning.
This however, requires a far bigger changes that are out of the scope
of this patch series.
The new analysis requires the DWARF location description for the
argc argument of the main function to change in the assembly file
gdb.python/amd64-py-framefilter-invalidarg.S. Originally, expression
ended with a 0 value byte, which was never reached by the symbol needs
evaluator, because it was detecting a stack underflow when evaluating
the operation before. The new approach does not simulate a DWARF
stack anymore, so the 0 value byte needs to be removed because it
makes the DWARF expression invalid.
gdb/ChangeLog:
* dwarf2/loc.c (class symbol_needs_eval_context): Remove.
(dwarf2_get_symbol_read_needs): New function.
(dwarf2_loc_desc_get_symbol_read_needs): Remove.
(locexpr_get_symbol_read_needs): Use
dwarf2_get_symbol_read_needs.
gdb/testsuite/ChangeLog:
* gdb.python/amd64-py-framefilter-invalidarg.S : Update argc
DWARF location expression.
* lib/dwarf.exp (_location): Handle DW_OP_fbreg.
* gdb.dwarf2/symbol_needs_eval.c: New file.
* gdb.dwarf2/symbol_needs_eval_fail.exp: New file.
* gdb.dwarf2/symbol_needs_eval_timeout.exp: New file.
---
gdb/dwarf2/loc.c | 515 ++++++++++++++----
gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c | 25 +
.../gdb.dwarf2/symbol_needs_eval_fail.exp | 112 ++++
.../gdb.dwarf2/symbol_needs_eval_timeout.exp | 131 +++++
.../amd64-py-framefilter-invalidarg.S | 1 -
gdb/testsuite/lib/dwarf.exp | 4 +
6 files changed, 672 insertions(+), 116 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index e816f926696..2fa68ff0426 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2736,151 +2736,430 @@ dwarf2_compile_property_to_c (string_file *stream,
data, data + size, per_cu, per_objfile);
}
-\f
-/* Helper functions and baton for dwarf2_loc_desc_get_symbol_read_needs. */
+/* Compute the correct symbol_needs_kind value for the location
+ expression in EXPR. */
-class symbol_needs_eval_context : public dwarf_expr_context
+static enum symbol_needs_kind
+dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
+ dwarf2_per_cu_data *per_cu,
+ dwarf2_per_objfile *per_objfile,
+ bfd_endian byte_order,
+ int addr_size,
+ int ref_addr_size,
+ int depth = 0)
{
-public:
- symbol_needs_eval_context (dwarf2_per_objfile *per_objfile)
- : dwarf_expr_context (per_objfile)
- {}
+ const gdb_byte *expr_end = expr.data () + expr.size ();
+ enum symbol_needs_kind symbol_needs = SYMBOL_NEEDS_NONE;
+ const int max_depth = 256;
+ std::vector <const gdb_byte *> next_op;
+ std::set <const gdb_byte *> visited_op;
- enum symbol_needs_kind needs;
- struct dwarf2_per_cu_data *per_cu;
+ if (depth > max_depth)
+ error (_("DWARF-2 expression error: Loop detected (%d)."), depth);
- /* Reads from registers do require a frame. */
- CORE_ADDR read_addr_from_reg (int regnum) override
- {
- needs = SYMBOL_NEEDS_FRAME;
- return 1;
- }
+ depth++;
- /* "get_reg_value" callback: Reads from registers do require a
- frame. */
+ next_op.push_back (expr.data ());
- struct value *get_reg_value (struct type *type, int regnum) override
- {
- needs = SYMBOL_NEEDS_FRAME;
- return value_zero (type, not_lval);
- }
+ while (!next_op.empty ())
+ {
+ const gdb_byte *op_ptr = next_op.back ();
- /* Reads from memory do not require a frame. */
- void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
- {
- memset (buf, 0, len);
- }
+ if (op_ptr >= expr_end
+ || visited_op.find(op_ptr) != visited_op.end())
+ {
+ next_op.pop_back ();
+ continue;
+ }
- /* Frame-relative accesses do require a frame. */
- void get_frame_base (const gdb_byte **start, size_t *length) override
- {
- static gdb_byte lit0 = DW_OP_lit0;
+ enum dwarf_location_atom op
+ = (enum dwarf_location_atom) *op_ptr++;
+ uint64_t uoffset;
+ int64_t offset;
- *start = &lit0;
- *length = 1;
+ /* The DWARF expression might have a bug causing an infinite
+ loop. In that case, quitting is the only way out. */
+ QUIT;
- needs = SYMBOL_NEEDS_FRAME;
- }
+ 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:
+ case DW_OP_stack_value:
+ case DW_OP_dup:
+ case DW_OP_drop:
+ case DW_OP_swap:
+ case DW_OP_over:
+ case DW_OP_rot:
+ case DW_OP_deref:
+ case DW_OP_abs:
+ case DW_OP_neg:
+ case DW_OP_not:
+ case DW_OP_and:
+ case DW_OP_div:
+ case DW_OP_minus:
+ case DW_OP_mod:
+ case DW_OP_mul:
+ case DW_OP_or:
+ case DW_OP_plus:
+ case DW_OP_shl:
+ case DW_OP_shr:
+ case DW_OP_shra:
+ case DW_OP_xor:
+ case DW_OP_le:
+ case DW_OP_ge:
+ case DW_OP_eq:
+ case DW_OP_lt:
+ case DW_OP_gt:
+ case DW_OP_ne:
+ case DW_OP_GNU_push_tls_address:
+ case DW_OP_nop:
+ case DW_OP_GNU_uninit:
+ case DW_OP_push_object_address:
+ break;
- /* CFA accesses require a frame. */
- CORE_ADDR get_frame_cfa () override
- {
- needs = SYMBOL_NEEDS_FRAME;
- return 1;
- }
+ case DW_OP_form_tls_address:
+ if (symbol_needs <= SYMBOL_NEEDS_REGISTERS)
+ symbol_needs = SYMBOL_NEEDS_REGISTERS;
+ break;
- CORE_ADDR get_frame_pc () override
- {
- needs = SYMBOL_NEEDS_FRAME;
- return 1;
- }
+ case DW_OP_convert:
+ case DW_OP_GNU_convert:
+ case DW_OP_reinterpret:
+ case DW_OP_GNU_reinterpret:
+ case DW_OP_addrx:
+ case DW_OP_GNU_addr_index:
+ case DW_OP_GNU_const_index:
+ case DW_OP_constu:
+ case DW_OP_plus_uconst:
+ case DW_OP_piece:
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ break;
- /* Thread-local accesses require registers, but not a frame. */
- CORE_ADDR get_tls_address (CORE_ADDR offset) override
- {
- if (needs <= SYMBOL_NEEDS_REGISTERS)
- needs = SYMBOL_NEEDS_REGISTERS;
- return 1;
- }
+ case DW_OP_consts:
+ op_ptr = safe_read_sleb128 (op_ptr, expr_end, &offset);
+ break;
- /* Helper interface of per_cu_dwarf_call for
- dwarf2_loc_desc_get_symbol_read_needs. */
+ case DW_OP_bit_piece:
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ break;
- void dwarf_call (cu_offset die_offset) override
- {
- per_cu_dwarf_call (this, die_offset, per_cu, per_objfile);
- }
+ case DW_OP_deref_type:
+ case DW_OP_GNU_deref_type:
+ op_ptr++;
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ break;
- /* Helper interface of sect_variable_value for
- dwarf2_loc_desc_get_symbol_read_needs. */
+ case DW_OP_addr:
+ op_ptr += addr_size;
+ break;
- struct value *dwarf_variable_value (sect_offset sect_off) override
- {
- return sect_variable_value (this, sect_off, per_cu, per_objfile);
- }
+ case DW_OP_const1u:
+ case DW_OP_const1s:
+ op_ptr += 1;
+ break;
- /* DW_OP_entry_value accesses require a caller, therefore a
- frame. */
+ case DW_OP_const2u:
+ case DW_OP_const2s:
+ op_ptr += 2;
+ break;
- void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
- union call_site_parameter_u kind_u,
- int deref_size) override
- {
- needs = SYMBOL_NEEDS_FRAME;
+ case DW_OP_const4s:
+ case DW_OP_const4u:
+ op_ptr += 4;
+ break;
- /* The expression may require some stub values on DWARF stack. */
- push_address (0, 0);
- }
+ case DW_OP_const8s:
+ case DW_OP_const8u:
+ op_ptr += 8;
+ break;
- /* DW_OP_addrx and DW_OP_GNU_addr_index doesn't require a frame. */
+ 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:
+ case DW_OP_regx:
+ case DW_OP_breg0:
+ case DW_OP_breg1:
+ case DW_OP_breg2:
+ case DW_OP_breg3:
+ case DW_OP_breg4:
+ case DW_OP_breg5:
+ case DW_OP_breg6:
+ case DW_OP_breg7:
+ case DW_OP_breg8:
+ case DW_OP_breg9:
+ case DW_OP_breg10:
+ case DW_OP_breg11:
+ case DW_OP_breg12:
+ case DW_OP_breg13:
+ case DW_OP_breg14:
+ case DW_OP_breg15:
+ case DW_OP_breg16:
+ case DW_OP_breg17:
+ case DW_OP_breg18:
+ case DW_OP_breg19:
+ case DW_OP_breg20:
+ case DW_OP_breg21:
+ case DW_OP_breg22:
+ case DW_OP_breg23:
+ case DW_OP_breg24:
+ case DW_OP_breg25:
+ case DW_OP_breg26:
+ case DW_OP_breg27:
+ case DW_OP_breg28:
+ case DW_OP_breg29:
+ case DW_OP_breg30:
+ case DW_OP_breg31:
+ case DW_OP_bregx:
+ case DW_OP_fbreg:
+ case DW_OP_call_frame_cfa:
+ case DW_OP_entry_value:
+ case DW_OP_GNU_entry_value:
+ case DW_OP_GNU_parameter_ref:
+ case DW_OP_regval_type:
+ case DW_OP_GNU_regval_type:
+ symbol_needs = SYMBOL_NEEDS_FRAME;
+ break;
- CORE_ADDR get_addr_index (unsigned int index) override
- {
- /* Nothing to do. */
- return 1;
- }
+ case DW_OP_implicit_value:
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr += uoffset;
+ break;
- /* DW_OP_push_object_address has a frame already passed through. */
+ case DW_OP_implicit_pointer:
+ case DW_OP_GNU_implicit_pointer:
+ op_ptr += ref_addr_size;
+ op_ptr = safe_read_sleb128 (op_ptr, expr_end, &offset);
+ break;
- CORE_ADDR get_object_address () override
- {
- /* Nothing to do. */
- return 1;
- }
-};
+ case DW_OP_deref_size:
+ case DW_OP_pick:
+ op_ptr++;
+ break;
-/* Compute the correct symbol_needs_kind value for the location
- expression at DATA (length SIZE). */
+ case DW_OP_skip:
+ offset = extract_signed_integer (op_ptr, 2, byte_order);
+ op_ptr += 2;
+ op_ptr += offset;
+ break;
-static enum symbol_needs_kind
-dwarf2_loc_desc_get_symbol_read_needs (const gdb_byte *data, size_t size,
- dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile)
-{
- scoped_value_mark free_values;
+ case DW_OP_bra:
+ visited_op.insert (next_op.back ());
+ next_op.pop_back ();
- symbol_needs_eval_context ctx (per_objfile);
+ offset = extract_signed_integer (op_ptr, 2, byte_order);
+ op_ptr += 2;
+ next_op.push_back (op_ptr + offset);
+ next_op.push_back (op_ptr);
+ continue;
- ctx.needs = SYMBOL_NEEDS_NONE;
- ctx.per_cu = per_cu;
- ctx.gdbarch = per_objfile->objfile->arch ();
- ctx.addr_size = per_cu->addr_size ();
- ctx.ref_addr_size = per_cu->ref_addr_size ();
+ case DW_OP_call2:
+ {
+ cu_offset cu_off
+ = (cu_offset) extract_unsigned_integer (op_ptr, 2, byte_order);
+ op_ptr += 2;
+
+ auto get_frame_pc = [&symbol_needs] ()
+ {
+ symbol_needs = SYMBOL_NEEDS_FRAME;
+ return 0;
+ };
+
+ struct dwarf2_locexpr_baton baton
+ = dwarf2_fetch_die_loc_cu_off (cu_off, per_cu,
+ per_objfile,
+ get_frame_pc);
- ctx.eval (data, size);
+ /* If SYMBOL_NEEDS_FRAME is returned from the previous call,
+ we dont have to check the baton content. */
+ if (symbol_needs != SYMBOL_NEEDS_FRAME)
+ {
+ gdbarch *arch = baton.per_objfile->objfile->arch ();
+ gdb::array_view<const gdb_byte> sub_expr (baton.data,
+ baton.size);
+ symbol_needs
+ = dwarf2_get_symbol_read_needs (sub_expr,
+ baton.per_cu,
+ baton.per_objfile,
+ gdbarch_byte_order (arch),
+ baton.per_cu->addr_size (),
+ baton.per_cu->ref_addr_size (),
+ depth);
+ }
+ break;
+ }
+
+ case DW_OP_call4:
+ {
+ cu_offset cu_off
+ = (cu_offset) extract_unsigned_integer (op_ptr, 4, byte_order);
+ op_ptr += 4;
+
+ auto get_frame_pc = [&symbol_needs] ()
+ {
+ symbol_needs = SYMBOL_NEEDS_FRAME;
+ return 0;
+ };
- bool in_reg = ctx.location == DWARF_VALUE_REGISTER;
+ struct dwarf2_locexpr_baton baton
+ = dwarf2_fetch_die_loc_cu_off (cu_off, per_cu,
+ per_objfile,
+ get_frame_pc);
- /* If the location has several pieces, and any of them are in
- registers, then we will need a frame to fetch them from. */
- for (dwarf_expr_piece &p : ctx.pieces)
- if (p.location == DWARF_VALUE_REGISTER)
- in_reg = true;
+ /* If SYMBOL_NEEDS_FRAME is returned from the previous call,
+ we dont have to check the baton content. */
+ if (symbol_needs != SYMBOL_NEEDS_FRAME)
+ {
+ gdbarch *arch = baton.per_objfile->objfile->arch ();
+ gdb::array_view<const gdb_byte> sub_expr (baton.data,
+ baton.size);
+ symbol_needs
+ = dwarf2_get_symbol_read_needs (sub_expr,
+ baton.per_cu,
+ baton.per_objfile,
+ gdbarch_byte_order (arch),
+ baton.per_cu->addr_size (),
+ baton.per_cu->ref_addr_size (),
+ depth);
+ }
+ break;
+ }
+
+ case DW_OP_GNU_variable_value:
+ {
+ sect_offset sect_off
+ = (sect_offset) extract_unsigned_integer (op_ptr,
+ ref_addr_size,
+ byte_order);
+ op_ptr += ref_addr_size;
+
+ struct type *die_type
+ = dwarf2_fetch_die_type_sect_off (sect_off, per_cu,
+ per_objfile);
+
+ if (die_type == NULL)
+ error (_("Bad DW_OP_GNU_variable_value DIE."));
+
+ /* Note: Things still work when the following test is
+ removed. This test and error is here to conform to the
+ proposed specification. */
+ if (die_type->code () != TYPE_CODE_INT
+ && die_type->code () != TYPE_CODE_PTR)
+ error (_("Type of DW_OP_GNU_variable_value DIE must be "
+ "an integer or pointer."));
+
+ auto get_frame_pc = [&symbol_needs] ()
+ {
+ symbol_needs = SYMBOL_NEEDS_FRAME;
+ return 0;
+ };
- if (in_reg)
- ctx.needs = SYMBOL_NEEDS_FRAME;
+ struct dwarf2_locexpr_baton baton
+ = dwarf2_fetch_die_loc_sect_off (sect_off, per_cu,
+ per_objfile,
+ get_frame_pc, true);
- return ctx.needs;
+ /* If SYMBOL_NEEDS_FRAME is returned from the previous call,
+ we dont have to check the baton content. */
+ if (symbol_needs != SYMBOL_NEEDS_FRAME)
+ {
+ gdbarch *arch = baton.per_objfile->objfile->arch ();
+ gdb::array_view<const gdb_byte> sub_expr (baton.data,
+ baton.size);
+ symbol_needs
+ = dwarf2_get_symbol_read_needs (sub_expr,
+ baton.per_cu,
+ baton.per_objfile,
+ gdbarch_byte_order (arch),
+ baton.per_cu->addr_size (),
+ baton.per_cu->ref_addr_size (),
+ depth);
+ }
+ break;
+ }
+
+ case DW_OP_const_type:
+ case DW_OP_GNU_const_type:
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ offset = *op_ptr++;
+ op_ptr += offset;
+ break;
+
+ default:
+ error (_("Unhandled DWARF expression opcode 0x%x"), op);
+ }
+
+ /* If it is known that a frame information is
+ needed we can stop parsing the expression. */
+ if (symbol_needs == SYMBOL_NEEDS_FRAME)
+ break;
+
+ visited_op.insert (next_op.back ());
+ next_op.pop_back ();
+ next_op.push_back (op_ptr);
+ }
+
+ return symbol_needs;
}
/* A helper function that throws an unimplemented error mentioning a
@@ -3722,9 +4001,15 @@ locexpr_get_symbol_read_needs (struct symbol *symbol)
struct dwarf2_locexpr_baton *dlbaton
= (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
- return dwarf2_loc_desc_get_symbol_read_needs (dlbaton->data, dlbaton->size,
- dlbaton->per_cu,
- dlbaton->per_objfile);
+ gdbarch *arch = dlbaton->per_objfile->objfile->arch ();
+ gdb::array_view<const gdb_byte> expr (dlbaton->data, dlbaton->size);
+
+ return dwarf2_get_symbol_read_needs (expr,
+ dlbaton->per_cu,
+ dlbaton->per_objfile,
+ gdbarch_byte_order (arch),
+ dlbaton->per_cu->addr_size (),
+ dlbaton->per_cu->ref_addr_size ());
}
/* Return true if DATA points to the end of a piece. END is one past
diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
new file mode 100644
index 00000000000..9740944a73c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017-2020 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int exec_mask = 1;
+
+int
+main (void)
+{
+ asm volatile ("main_label: .globl main_label");
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
new file mode 100644
index 00000000000..033e0426347
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
@@ -0,0 +1,112 @@
+# Copyright 2017-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test the symbol needs check mechanism if it assumes that faking
+# reads from a target is a safe thing to do.
+#
+# In particular, the test uses a relative branch DWARF operation to
+# hide a register read. If the target reads are indeed faked, the
+# result returned will be wrong.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Choose suitable integer registers for the test.
+
+set dwarf_regnum 0
+
+if { [is_aarch64_target] } {
+ set regname x0
+} elseif { [is_aarch32_target]
+ || [istarget "s390*-*-*" ]
+ || [istarget "powerpc*-*-*"]
+ || [istarget "rs6000*-*-aix*"] } {
+ set regname r0
+} elseif { [is_x86_like_target] } {
+ set regname eax
+} elseif { [is_amd64_regs_target] } {
+ set regname rax
+} else {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
+
+# Make some DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global dwarf_regnum regname
+
+ set exec_mask_var [gdb_target_symbol exec_mask]
+
+ cu {} {
+ DW_TAG_compile_unit {
+ {DW_AT_name symbol_needs_eval.c}
+ {DW_AT_comp_dir /tmp}
+ } {
+ declare_labels int_type_label
+
+ # define int type
+ int_type_label: DW_TAG_base_type {
+ {DW_AT_name "int"}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_byte_size 4 DW_FORM_sdata}
+ }
+
+ # define artificial variable a
+ DW_TAG_variable {
+ {DW_AT_name a}
+ {DW_AT_type :$int_type_label}
+ {DW_AT_location {
+ DW_OP_addr $exec_mask_var
+ DW_OP_deref
+
+ # conditional jump to DW_OP_bregx
+ DW_OP_bra 4
+ DW_OP_lit0
+
+ # jump to DW_OP_stack_value
+ DW_OP_skip 3
+ DW_OP_bregx $dwarf_regnum 0
+ DW_OP_stack_value
+ } SPECIAL_expr}
+ {external 1 flag}
+ }
+ }
+ }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+}
+
+# The variable's location expression requires a frame,
+# so an error should be reported.
+gdb_test "print/d a" "No frame selected." "variable a can't be printed"
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_test_no_output "set var \$$regname = 2" "init reg to 2"
+
+gdb_test "print/d a" " = 2" "a == 2"
diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
new file mode 100644
index 00000000000..4189b4486b3
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
@@ -0,0 +1,131 @@
+# Copyright 2017-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test the symbol needs check mechanism if it assumes that faking
+# reads from a target is a safe thing to do.
+#
+# In particular, the test uses a relative branch DWARF operation to
+# potentially cause an infinite loop, if the target reads are indeed
+# faked.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Choose suitable integer registers for the test.
+
+set dwarf_regnum 0
+
+if { [is_aarch64_target] } {
+ set regname x0
+} elseif { [is_aarch32_target]
+ || [istarget "s390*-*-*" ]
+ || [istarget "powerpc*-*-*"]
+ || [istarget "rs6000*-*-aix*"] } {
+ set regname r0
+} elseif { [is_x86_like_target] } {
+ set regname eax
+} elseif { [is_amd64_regs_target] } {
+ set regname rax
+} else {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
+
+# Make some DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global dwarf_regnum regname
+
+ set exec_mask_var [gdb_target_symbol exec_mask]
+
+ cu {} {
+ DW_TAG_compile_unit {
+ {DW_AT_name symbol_needs_eval.c}
+ {DW_AT_comp_dir /tmp}
+ } {
+ declare_labels int_type_label
+
+ # define int type
+ int_type_label: DW_TAG_base_type {
+ {DW_AT_name "int"}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_byte_size 4 DW_FORM_sdata}
+ }
+
+ # add info for variable exec_mask
+ DW_TAG_variable {
+ {DW_AT_name exec_mask}
+ {DW_AT_type :$int_type_label}
+ {DW_AT_location {
+ DW_OP_addr $exec_mask_var
+ } SPECIAL_expr}
+ {external 1 flag}
+ }
+
+ # add info for subprogram main
+ DW_TAG_subprogram {
+ {MACRO_AT_func { main }}
+ {DW_AT_frame_base {
+ DW_OP_regx $dwarf_regnum
+ } SPECIAL_expr}
+ } {
+ # define artificial variable a
+ DW_TAG_variable {
+ {DW_AT_name a}
+ {DW_AT_type :$int_type_label}
+ {DW_AT_location {
+ DW_OP_lit1
+ DW_OP_addr $exec_mask_var
+ DW_OP_deref
+
+ # jump to DW_OP_fbreg
+ DW_OP_skip 4
+ DW_OP_drop
+ DW_OP_fbreg 0
+ DW_OP_dup
+ DW_OP_lit0
+ DW_OP_eq
+
+ # conditional jump to DW_OP_drop
+ DW_OP_bra -9
+ DW_OP_stack_value
+ } SPECIAL_expr}
+ {external 1 flag}
+ }
+ }
+ }
+ }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_test_no_output "set var \$$regname = 2" "init reg to 2"
+gdb_test_no_output "set var exec_mask = 0" "init exec_mask to 0"
+
+gdb_test "print/d a" " = 2" "a == 2"
diff --git a/gdb/testsuite/gdb.python/amd64-py-framefilter-invalidarg.S b/gdb/testsuite/gdb.python/amd64-py-framefilter-invalidarg.S
index 0adc69fdad6..c8cb9e5aca8 100644
--- a/gdb/testsuite/gdb.python/amd64-py-framefilter-invalidarg.S
+++ b/gdb/testsuite/gdb.python/amd64-py-framefilter-invalidarg.S
@@ -102,7 +102,6 @@ die4e:
.uleb128 1f - 2f # DW_AT_location
2:
.byte 0x13 # DW_OP_drop
- .quad 0
1:
#endif
die5c:
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index e4dc284f4ee..de722d24684 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1216,6 +1216,10 @@ namespace eval Dwarf {
_op .sleb128 $argvec(offset)
}
+ DW_OP_fbreg {
+ _op .sleb128 [lindex $line 1]
+ }
+
default {
if {[llength $line] > 1} {
error "Unimplemented: operands in location for $opcode"
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 02/17] Cleanup of the dwarf_expr_context constructor
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 01/17] Replace the symbol needs evaluator with a parser Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 03/17] Move frame context info to dwarf_expr_context Zoran Zaric
` (15 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
Move the initial values for dwarf_expr_context class data members
to the class declaration in expr.h.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context):
Remove initial data members values.
* dwarf2/expr.h (dwarf_expr_context): Add initial values
to the class data members.
---
gdb/dwarf2/expr.c | 11 +----------
gdb/dwarf2/expr.h | 16 ++++++++--------
2 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 107b9cdbd5d..aa166b22d9c 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -90,16 +90,7 @@ dwarf_expr_context::address_type () const
/* Create a new context for the expression evaluator. */
dwarf_expr_context::dwarf_expr_context (dwarf2_per_objfile *per_objfile)
-: gdbarch (NULL),
- addr_size (0),
- ref_addr_size (0),
- recursion_depth (0),
- max_recursion_depth (0x100),
- location (DWARF_VALUE_MEMORY),
- len (0),
- data (NULL),
- initialized (0),
- per_objfile (per_objfile)
+: per_objfile (per_objfile)
{
}
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index b28a0775602..fc31be31a4d 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -132,32 +132,32 @@ struct dwarf_expr_context
std::vector<dwarf_stack_value> stack;
/* Target architecture to use for address operations. */
- struct gdbarch *gdbarch;
+ struct gdbarch *gdbarch = nullptr;
/* Target address size in bytes. */
- int addr_size;
+ int addr_size = 0;
/* DW_FORM_ref_addr size in bytes. If -1 DWARF is executed from a frame
context and operations depending on DW_FORM_ref_addr are not allowed. */
- int ref_addr_size;
+ int ref_addr_size = 0;
/* The current depth of dwarf expression recursion, via DW_OP_call*,
DW_OP_fbreg, DW_OP_push_object_address, etc., and the maximum
depth we'll tolerate before raising an error. */
- int recursion_depth, max_recursion_depth;
+ int recursion_depth = 0, max_recursion_depth = 0x100;
/* Location of the value. */
- enum dwarf_value_location location;
+ enum dwarf_value_location location = DWARF_VALUE_MEMORY;
/* For DWARF_VALUE_LITERAL, the current literal value's length and
data. For DWARF_VALUE_IMPLICIT_POINTER, LEN is the offset of the
target DIE of sect_offset kind. */
- ULONGEST len;
- const gdb_byte *data;
+ ULONGEST len = 0;
+ const gdb_byte *data = nullptr;
/* Initialization status of variable: Non-zero if variable has been
initialized; zero otherwise. */
- int initialized;
+ int initialized = 0;
/* A vector of pieces.
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 03/17] Move frame context info to dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 01/17] Replace the symbol needs evaluator with a parser Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 02/17] Cleanup of the dwarf_expr_context constructor Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-06-09 0:45 ` Simon Marchi
2021-05-28 15:46 ` [PATCH v3 04/17] Remove get_frame_cfa from dwarf_expr_context Zoran Zaric
` (14 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
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 process 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 | 78 ++++++++++++++++++++++++++++++++++++++++++++--
gdb/dwarf2/expr.h | 31 +++++++++---------
gdb/dwarf2/frame.c | 36 ++-------------------
gdb/dwarf2/loc.c | 55 +-------------------------------
4 files changed, 95 insertions(+), 105 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index aa166b22d9c..2f5a4a696c7 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,31 @@ dwarf_gdbarch_types_init (struct gdbarch *gdbarch)
return types;
}
+/* Ensure that a FRAME is defined, throw an exception otherwise.
+
+ Throwing NOT_AVAILABLE_ERROR error so that a client can chose
+ to react differently if the evaluation ended because there
+ was a missing context information. */
+
+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. */
@@ -133,6 +159,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 (this->frame, "DW_OP_regval_type");
+
+ 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);
+}
+
+/* See expr.h. */
+
+void
+dwarf_expr_context::get_frame_base (const gdb_byte **start,
+ size_t * length)
+{
+ ensure_have_frame (this->frame, "DW_OP_fbreg");
+
+ const struct block *bl = get_frame_block (this->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 (this->frame),
+ start, length);
+}
+
/* Require that TYPE be an integral type; throw an exception if not. */
static void
@@ -821,7 +888,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);
}
@@ -830,7 +899,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
{
op_ptr = safe_read_uleb128 (op_ptr, op_end, ®);
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);
}
@@ -857,7 +928,8 @@ 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 fc31be31a4d..0e67bddff13 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 = nullptr;
/* 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
+ in a current context.
+
+ 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 *frame, int reg);
+
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 f8612c96f27..97a9555b9b0 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -192,18 +192,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. */
@@ -244,31 +232,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
@@ -324,7 +292,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;
@@ -335,7 +303,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 2fa68ff0426..44f5dc80209 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,64 +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
- {
- if (frame == nullptr)
- error (_("frame address is not available."));
-
- /* 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. */
@@ -2558,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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 04/17] Remove get_frame_cfa from dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (2 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 03/17] Move frame context info to dwarf_expr_context Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context Zoran Zaric
` (13 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
Following the idea of merging the evaluators, the get_frame_cfa method
can be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc
classes to their base class dwarf_expr_context. Once this is done,
it becomes apparent that the method is only called once and it can be
inlined.
It is also necessary to check if the frame context information was
provided before the DW_OP_call_frame_cfa operation is executed.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_frame_cfa): Remove
method.
(dwarf_expr_context::execute_stack_op): Call frame context info
check for DW_OP_call_frame_cfa. Remove use of get_frame_cfa.
* dwarf2/expr.h (dwarf_expr_context::get_frame_cfa): Remove
method.
* dwarf2/frame.c (dwarf_expr_context::get_frame_cfa): Remove
method.
* dwarf2/loc.c (dwarf_expr_context::get_frame_cfa): Remove
method.
---
gdb/dwarf2/expr.c | 5 ++++-
gdb/dwarf2/expr.h | 3 ---
gdb/dwarf2/frame.c | 5 -----
gdb/dwarf2/loc.c | 8 --------
4 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 2f5a4a696c7..ced78034fdf 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -29,6 +29,7 @@
#include "dwarf2/expr.h"
#include "dwarf2/loc.h"
#include "dwarf2/read.h"
+#include "frame.h"
#include "gdbsupport/underlying.h"
#include "gdbarch.h"
@@ -1221,7 +1222,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
break;
case DW_OP_call_frame_cfa:
- result = this->get_frame_cfa ();
+ ensure_have_frame (this->frame, "DW_OP_call_frame_cfa");
+
+ result = dwarf2_frame_cfa (this->frame);
result_val = value_from_ulongest (address_type, result);
in_stack_memory = true;
break;
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index 0e67bddff13..ba17dc40165 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -192,9 +192,6 @@ struct dwarf_expr_context
/* Read LENGTH bytes at ADDR into BUF. */
virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
- /* Return the CFA for the frame. */
- virtual CORE_ADDR get_frame_cfa () = 0;
-
/* Return the PC for the frame. */
virtual CORE_ADDR get_frame_pc ()
{
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 97a9555b9b0..c543848cda6 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -249,11 +249,6 @@ class dwarf_expr_executor : public dwarf_expr_context
invalid ("DW_OP_push_object_address");
}
- CORE_ADDR get_frame_cfa () override
- {
- invalid ("DW_OP_call_frame_cfa");
- }
-
CORE_ADDR get_tls_address (CORE_ADDR offset) override
{
invalid ("DW_OP_form_tls_address");
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 44f5dc80209..7b9f06bd5a9 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -657,14 +657,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
struct dwarf2_per_cu_data *per_cu;
CORE_ADDR obj_address;
- /* Helper function for dwarf2_evaluate_loc_desc. Computes the CFA for
- the frame in BATON. */
-
- CORE_ADDR get_frame_cfa () override
- {
- return dwarf2_frame_cfa (frame);
- }
-
/* Helper function for dwarf2_evaluate_loc_desc. Computes the PC for
the frame in BATON. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (3 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 04/17] Remove get_frame_cfa from dwarf_expr_context Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-06-09 0:50 ` Simon Marchi
2021-05-28 15:46 ` [PATCH v3 06/17] Move dwarf_call " Zoran Zaric
` (12 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
This patch moves the compilation unit 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 compilation unit information to be resolved,
which is not available.
With this change, it also makes sense to always acquire ref_addr_size
information from the compilation unit context, considering that all
DWARF operations that refer to that information require a compilation
unit context to be present during their evaluation.
gdb/ChangeLog:
* dwarf2/expr.c (ensure_have_per_cu): New function.
(dwarf_expr_context::dwarf_expr_context): Add compilation unit
context information.
(dwarf_expr_context::get_base_type): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::get_addr_index): Remove method.
(dwarf_expr_context::dwarf_variable_value): Remove method.
(dwarf_expr_context::execute_stack_op): Call compilation unit
context info check. Inline get_addr_index and
dwarf_variable_value methods.
* dwarf2/expr.h (struct dwarf_expr_context): Add compilation
context info.
(dwarf_expr_context::get_addr_index): Remove method.
(dwarf_expr_context::dwarf_variable_value): Remove method.
(dwarf_expr_context::ref_addr_size): Remove member.
* dwarf2/frame.c (dwarf_expr_executor::get_addr_index): Remove
method.
(dwarf_expr_executor::dwarf_variable_value): Remove method.
* dwarf2/loc.c (sect_variable_value): Expose function.
(dwarf_evaluate_loc_desc::get_addr_index): Remove method.
(dwarf_evaluate_loc_desc::dwarf_variable_value): Remove method.
(class dwarf_evaluate_loc_desc): Move compilation unit context
information to dwarf_expr_context class.
* dwarf2/loc.h (sect_variable_value): Expose function.
---
gdb/dwarf2/expr.c | 76 ++++++++++++++++++++++++++++++++++++----------
gdb/dwarf2/expr.h | 29 +++++-------------
gdb/dwarf2/frame.c | 11 -------
gdb/dwarf2/loc.c | 37 ++--------------------
gdb/dwarf2/loc.h | 8 +++++
5 files changed, 79 insertions(+), 82 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index ced78034fdf..d70f92f27bc 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -72,6 +72,20 @@ ensure_have_frame (struct frame_info *frame, const char *op_name)
_("%s evaluation requires a frame."), op_name);
}
+/* Ensure that a PER_CU is defined and throw an exception otherwise.
+
+ Throwing NOT_AVAILABLE_ERROR error so that a client can chose
+ to react differently if the evaluation ended because there
+ was a missing context information. */
+
+static void
+ensure_have_per_cu (struct dwarf2_per_cu_data *per_cu, const char* op_name)
+{
+ if (per_cu == nullptr)
+ throw_error (GENERIC_ERROR,
+ _("%s evaluation requires a compilation unit."), op_name);
+}
+
/* See expr.h. */
CORE_ADDR
@@ -201,6 +215,23 @@ dwarf_expr_context::get_frame_base (const gdb_byte **start,
start, length);
}
+/* See expr.h. */
+
+struct type *
+dwarf_expr_context::get_base_type (cu_offset die_cu_off)
+{
+ if (per_cu == nullptr)
+ return builtin_type (this->gdbarch)->builtin_int;
+
+ struct type *result = dwarf2_get_die_type (die_cu_off, this->per_cu,
+ this->per_objfile);
+
+ if (result == nullptr)
+ error (_("Could not find type for operation"));
+
+ return result;
+}
+
/* Require that TYPE be an integral type; throw an exception if not. */
static void
@@ -697,14 +728,20 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_addrx:
case DW_OP_GNU_addr_index:
+ ensure_have_per_cu (this->per_cu, "DW_OP_addrx");
+
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
- result = this->get_addr_index (uoffset);
+ result = dwarf2_read_addr_index (this->per_cu, this->per_objfile,
+ uoffset);
result += this->per_objfile->objfile->text_section_offset ();
result_val = value_from_ulongest (address_type, result);
break;
case DW_OP_GNU_const_index:
+ ensure_have_per_cu (this->per_cu, "DW_OP_GNU_const_index");
+
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
- result = this->get_addr_index (uoffset);
+ result = dwarf2_read_addr_index (this->per_cu, this->per_objfile,
+ uoffset);
result_val = value_from_ulongest (address_type, result);
break;
@@ -834,15 +871,14 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_GNU_implicit_pointer:
{
int64_t len;
+ ensure_have_per_cu (this->per_cu, "DW_OP_implicit_pointer");
- if (this->ref_addr_size == -1)
- error (_("DWARF-2 expression error: DW_OP_implicit_pointer "
- "is not allowed in frame context"));
+ int ref_addr_size = this->per_cu->ref_addr_size ();
/* The referred-to DIE of sect_offset kind. */
- this->len = extract_unsigned_integer (op_ptr, this->ref_addr_size,
- byte_order);
- op_ptr += this->ref_addr_size;
+ this->len = extract_unsigned_integer (op_ptr, ref_addr_size,
+ byte_order);
+ op_ptr += ref_addr_size;
/* The byte offset into the data. */
op_ptr = safe_read_sleb128 (op_ptr, op_end, &len);
@@ -1008,7 +1044,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
{
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
cu_offset type_die_cu_off = (cu_offset) uoffset;
- type = get_base_type (type_die_cu_off, 0);
+ type = get_base_type (type_die_cu_off);
}
else
type = address_type;
@@ -1331,13 +1367,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_GNU_variable_value:
{
+ ensure_have_per_cu (this->per_cu, "DW_OP_GNU_variable_value");
+ int ref_addr_size = this->per_cu->ref_addr_size ();
+
sect_offset sect_off
= (sect_offset) extract_unsigned_integer (op_ptr,
- this->ref_addr_size,
+ ref_addr_size,
byte_order);
- op_ptr += this->ref_addr_size;
- result_val = value_cast (address_type,
- this->dwarf_variable_value (sect_off));
+ op_ptr += ref_addr_size;
+ result_val = sect_variable_value (sect_off, this->per_cu,
+ this->per_objfile);
+ result_val = value_cast (address_type, result_val);
}
break;
@@ -1407,7 +1447,11 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
data = op_ptr;
op_ptr += n;
- type = get_base_type (type_die_cu_off, n);
+ type = get_base_type (type_die_cu_off);
+
+ if (TYPE_LENGTH (type) != n)
+ error (_("DW_OP_const_type has different sizes for type and data"));
+
result_val = value_from_contents (type, data);
}
break;
@@ -1421,7 +1465,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
cu_offset type_die_cu_off = (cu_offset) uoffset;
- type = get_base_type (type_die_cu_off, 0);
+ type = get_base_type (type_die_cu_off);
result_val = this->get_reg_value (type, reg);
}
break;
@@ -1439,7 +1483,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
if (to_underlying (type_die_cu_off) == 0)
type = address_type;
else
- type = get_base_type (type_die_cu_off, 0);
+ type = get_base_type (type_die_cu_off);
result_val = fetch (0);
pop ();
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index ba17dc40165..f14070de522 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -137,10 +137,6 @@ struct dwarf_expr_context
/* Target address size in bytes. */
int addr_size = 0;
- /* DW_FORM_ref_addr size in bytes. If -1 DWARF is executed from a frame
- context and operations depending on DW_FORM_ref_addr are not allowed. */
- int ref_addr_size = 0;
-
/* The current depth of dwarf expression recursion, via DW_OP_call*,
DW_OP_fbreg, DW_OP_push_object_address, etc., and the maximum
depth we'll tolerate before raising an error. */
@@ -189,6 +185,9 @@ struct dwarf_expr_context
/* Frame information used for the evaluation. */
struct frame_info *frame = nullptr;
+ /* Compilation unit used for the evaluation. */
+ struct dwarf2_per_cu_data *per_cu = nullptr;
+
/* Read LENGTH bytes at ADDR into BUF. */
virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
@@ -208,19 +207,6 @@ struct dwarf_expr_context
subroutine. */
virtual void dwarf_call (cu_offset die_cu_off) = 0;
- /* Execute "variable value" operation on the DIE at SECT_OFF. */
- virtual struct value *dwarf_variable_value (sect_offset sect_off) = 0;
-
- /* Return the base type given by the indicated DIE at DIE_CU_OFF.
- This can throw an exception if the DIE is invalid or does not
- represent a base 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_cu_off, int size)
- {
- /* Anything will do. */
- return builtin_type (this->gdbarch)->builtin_int;
- }
-
/* Push on DWARF stack an entry evaluated for DW_TAG_call_site's
parameter matching KIND and KIND_U at the caller of specified BATON.
If DEREF_SIZE is not -1 then use DW_AT_call_data_value instead of
@@ -229,10 +215,6 @@ struct dwarf_expr_context
union call_site_parameter_u kind_u,
int deref_size) = 0;
- /* Return the address indexed by DW_OP_addrx or DW_OP_GNU_addr_index.
- This can throw an exception if the index is out of range. */
- virtual CORE_ADDR get_addr_index (unsigned int index) = 0;
-
/* Return the `object address' for DW_OP_push_object_address. */
virtual CORE_ADDR get_object_address () = 0;
@@ -255,6 +237,11 @@ struct dwarf_expr_context
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 base type given by the indicated DIE at DIE_CU_OFF.
+ This can throw an exception if the DIE is invalid or does not
+ represent a base type. */
+ struct type *get_base_type (cu_offset die_cu_off);
};
/* Return the value of register number REG (a DWARF register number),
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index c543848cda6..492c1ec9d7c 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -259,16 +259,6 @@ class dwarf_expr_executor : public dwarf_expr_context
invalid ("DW_OP_call*");
}
- struct value *dwarf_variable_value (sect_offset sect_off) override
- {
- invalid ("DW_OP_GNU_variable_value");
- }
-
- CORE_ADDR get_addr_index (unsigned int index) override
- {
- invalid ("DW_OP_addrx or DW_OP_GNU_addr_index");
- }
-
private:
void invalid (const char *op) ATTRIBUTE_NORETURN
@@ -290,7 +280,6 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
ctx.frame = this_frame;
ctx.gdbarch = get_frame_arch (this_frame);
ctx.addr_size = addr_size;
- ctx.ref_addr_size = -1;
ctx.push_address (initial, initial_in_stack_memory);
ctx.eval (exp, len);
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 7b9f06bd5a9..f5bd5c57c61 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -620,12 +620,10 @@ per_cu_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset,
ctx->eval (block.data, block.size);
}
-/* Given context CTX, section offset SECT_OFF, and compilation unit
- data PER_CU, execute the "variable value" operation on the DIE
- found at SECT_OFF. */
+/* See loc.h. */
-static struct value *
-sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off,
+struct value *
+sect_variable_value (sect_offset sect_off,
dwarf2_per_cu_data *per_cu,
dwarf2_per_objfile *per_objfile)
{
@@ -654,7 +652,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
: dwarf_expr_context (per_objfile)
{}
- struct dwarf2_per_cu_data *per_cu;
CORE_ADDR obj_address;
/* Helper function for dwarf2_evaluate_loc_desc. Computes the PC for
@@ -680,32 +677,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
per_cu_dwarf_call (this, die_offset, per_cu, per_objfile);
}
- /* Helper interface of sect_variable_value for
- dwarf2_evaluate_loc_desc. */
-
- struct value *dwarf_variable_value (sect_offset sect_off) override
- {
- return sect_variable_value (this, sect_off, per_cu, per_objfile);
- }
-
- struct type *get_base_type (cu_offset die_offset, int size) override
- {
- struct type *result = dwarf2_get_die_type (die_offset, per_cu, per_objfile);
- if (result == NULL)
- error (_("Could not find type for DW_OP_const_type"));
- if (size != 0 && TYPE_LENGTH (result) != size)
- error (_("DW_OP_const_type has different sizes for type and data"));
- return result;
- }
-
- /* Callback function for dwarf2_evaluate_loc_desc.
- Fetch the address indexed by DW_OP_addrx or DW_OP_GNU_addr_index. */
-
- CORE_ADDR get_addr_index (unsigned int index) override
- {
- return dwarf2_read_addr_index (per_cu, per_objfile, index);
- }
-
/* Callback function for get_object_address. Return the address of the VLA
object. */
@@ -2195,7 +2166,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
ctx.gdbarch = per_objfile->objfile->arch ();
ctx.addr_size = per_cu->addr_size ();
- ctx.ref_addr_size = per_cu->ref_addr_size ();
try
{
@@ -2462,7 +2432,6 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
ctx.gdbarch = per_objfile->objfile->arch ();
ctx.addr_size = dlbaton->per_cu->addr_size ();
- ctx.ref_addr_size = dlbaton->per_cu->ref_addr_size ();
if (push_initial_value)
ctx.push_address (ctx.obj_address, false);
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 2943baf91d2..a729b370e2e 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -53,6 +53,14 @@ extern void func_get_frame_base_dwarf_block (struct symbol *framefunc,
const gdb_byte **start,
size_t *length);
+/* Given section offset SECT_OFF, and compilation unit data
+ PER_CU, execute the "variable value" operation on the DIE
+ found at SECT_OFF. */
+
+struct value *sect_variable_value (sect_offset sect_off,
+ dwarf2_per_cu_data *per_cu,
+ dwarf2_per_objfile *per_objfile);
+
/* Evaluate a location description, starting at DATA and with length
SIZE, to find the current location of variable of TYPE in the context
of FRAME. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 06/17] Move dwarf_call to dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (4 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 07/17] Move get_object_address " Zoran Zaric
` (11 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
Following the idea of merging the evaluators, the dwarf_call and
get_frame_pc method can be moved from dwarf_expr_executor and
dwarf_evaluate_loc_desc classes to their base class dwarf_expr_context.
Once this is done, the get_frame_pc can be replace with lambda
function.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_call): Move from
dwarf_evaluate_loc_desc.
(dwarf_expr_context::get_frame_pc): Replace with lambda.
* dwarf2/expr.h (dwarf_expr_context::get_frame_pc): Remove
method.
* dwarf2/frame.c (dwarf_expr_executor::dwarf_call): Remove
method.
(dwarf_expr_executor::get_frame_pc): Remove method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_frame_pc): Remove
method.
(dwarf_evaluate_loc_desc::dwarf_call): Move to
dwarf_expr_context.
(per_cu_dwarf_call): Inline function.
---
gdb/dwarf2/expr.c | 28 +++++++++++++++++++++++++++-
gdb/dwarf2/expr.h | 22 ++++++----------------
gdb/dwarf2/frame.c | 10 ----------
gdb/dwarf2/loc.c | 43 -------------------------------------------
4 files changed, 33 insertions(+), 70 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index d70f92f27bc..689224b63a3 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -232,6 +232,31 @@ dwarf_expr_context::get_base_type (cu_offset die_cu_off)
return result;
}
+/* See expr.h. */
+
+void
+dwarf_expr_context::dwarf_call (cu_offset die_cu_off)
+{
+ ensure_have_per_cu (this->per_cu, "DW_OP_call");
+
+ struct frame_info *frame = this->frame;
+
+ auto get_pc_from_frame = [frame] ()
+ {
+ ensure_have_frame (frame, "DW_OP_call");
+ return get_frame_address_in_block (frame);
+ };
+
+ struct dwarf2_locexpr_baton block
+ = dwarf2_fetch_die_loc_cu_off (die_cu_off, this->per_cu, this->per_objfile,
+ get_pc_from_frame);
+
+ /* DW_OP_call_ref is currently not supported. */
+ gdb_assert (block.per_cu == this->per_cu);
+
+ this->eval (block.data, block.size);
+}
+
/* Require that TYPE be an integral type; throw an exception if not. */
static void
@@ -1277,7 +1302,8 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
returned. */
result = value_as_long (fetch (0));
pop ();
- result = this->get_tls_address (result);
+ result = target_translate_tls_address (this->per_objfile->objfile,
+ result);
result_val = value_from_ulongest (address_type, result);
break;
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index f14070de522..6b51f2eaae5 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -191,22 +191,6 @@ struct dwarf_expr_context
/* Read LENGTH bytes at ADDR into BUF. */
virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
- /* Return the PC for the frame. */
- virtual CORE_ADDR get_frame_pc ()
- {
- error (_("%s is invalid in this context"), "DW_OP_implicit_pointer");
- }
-
- /* Return the thread-local storage address for
- DW_OP_GNU_push_tls_address or DW_OP_form_tls_address. */
- virtual CORE_ADDR get_tls_address (CORE_ADDR offset) = 0;
-
- /* Execute DW_AT_location expression for the DWARF expression
- subroutine in the DIE at DIE_CU_OFF in the CU. Do not touch
- STACK while it being passed to and returned from the called DWARF
- subroutine. */
- virtual void dwarf_call (cu_offset die_cu_off) = 0;
-
/* Push on DWARF stack an entry evaluated for DW_TAG_call_site's
parameter matching KIND and KIND_U at the caller of specified BATON.
If DEREF_SIZE is not -1 then use DW_AT_call_data_value instead of
@@ -242,6 +226,12 @@ struct dwarf_expr_context
This can throw an exception if the DIE is invalid or does not
represent a base type. */
struct type *get_base_type (cu_offset die_cu_off);
+
+ /* Execute DW_AT_location expression for the DWARF expression
+ subroutine in the DIE at DIE_CU_OFF in the CU. Do not touch
+ STACK while it being passed to and returned from the called DWARF
+ subroutine. */
+ void dwarf_call (cu_offset die_cu_off);
};
/* Return the value of register number REG (a DWARF register number),
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 492c1ec9d7c..20da0f45e84 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -249,16 +249,6 @@ class dwarf_expr_executor : public dwarf_expr_context
invalid ("DW_OP_push_object_address");
}
- CORE_ADDR get_tls_address (CORE_ADDR offset) override
- {
- invalid ("DW_OP_form_tls_address");
- }
-
- void dwarf_call (cu_offset die_offset) override
- {
- invalid ("DW_OP_call*");
- }
-
private:
void invalid (const char *op) ATTRIBUTE_NORETURN
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index f5bd5c57c61..d88239880af 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -600,26 +600,6 @@ func_get_frame_base_dwarf_block (struct symbol *framefunc, CORE_ADDR pc,
framefunc->natural_name ());
}
-static void
-per_cu_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset,
- dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile)
-{
- struct dwarf2_locexpr_baton block;
-
- auto get_frame_pc_from_ctx = [ctx] ()
- {
- return ctx->get_frame_pc ();
- };
-
- block = dwarf2_fetch_die_loc_cu_off (die_offset, per_cu, per_objfile,
- get_frame_pc_from_ctx);
-
- /* DW_OP_call_ref is currently not supported. */
- gdb_assert (block.per_cu == per_cu);
-
- ctx->eval (block.data, block.size);
-}
-
/* See loc.h. */
struct value *
@@ -654,29 +634,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
CORE_ADDR obj_address;
- /* Helper function for dwarf2_evaluate_loc_desc. Computes the PC for
- the frame in BATON. */
-
- CORE_ADDR get_frame_pc () override
- {
- return get_frame_address_in_block (frame);
- }
-
- /* Using the objfile specified in BATON, find the address for the
- current thread's thread-local storage with offset OFFSET. */
- CORE_ADDR get_tls_address (CORE_ADDR offset) override
- {
- return target_translate_tls_address (per_objfile->objfile, offset);
- }
-
- /* Helper interface of per_cu_dwarf_call for
- dwarf2_evaluate_loc_desc. */
-
- void dwarf_call (cu_offset die_offset) override
- {
- per_cu_dwarf_call (this, die_offset, per_cu, per_objfile);
- }
-
/* Callback function for get_object_address. Return the address of the VLA
object. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 07/17] Move get_object_address to dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (5 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 06/17] Move dwarf_call " Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 08/17] Move read_mem " Zoran Zaric
` (10 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
Following the idea of merging the evaluators, the get_object_address
and can be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc
classes to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_object_address): Move
from dwarf_evaluate_loc_desc.
(class dwarf_expr_context): Add object address member to
dwarf_expr_context.
* dwarf2/expr.h (dwarf_expr_context::get_frame_pc): Remove
method.
* dwarf2/frame.c (dwarf_expr_executor::get_object_address):
Remove method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::get_object_address):
move to dwarf_expr_context.
(class dwarf_evaluate_loc_desc): Move object address member to
dwarf_expr_context.
---
gdb/dwarf2/expr.h | 10 +++++++++-
gdb/dwarf2/frame.c | 5 -----
gdb/dwarf2/loc.c | 12 ------------
3 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index 6b51f2eaae5..ad1918eb18c 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -188,6 +188,9 @@ struct dwarf_expr_context
/* Compilation unit used for the evaluation. */
struct dwarf2_per_cu_data *per_cu = nullptr;
+ /* Object address used for the evaluation. */
+ CORE_ADDR obj_address = 0;
+
/* Read LENGTH bytes at ADDR into BUF. */
virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
@@ -200,7 +203,12 @@ struct dwarf_expr_context
int deref_size) = 0;
/* Return the `object address' for DW_OP_push_object_address. */
- virtual CORE_ADDR get_object_address () = 0;
+ virtual CORE_ADDR get_object_address ()
+ {
+ if (obj_address == 0)
+ error (_("Location address is not set."));
+ return obj_address;
+ }
private:
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 20da0f45e84..52b09809ccb 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -244,11 +244,6 @@ class dwarf_expr_executor : public dwarf_expr_context
invalid ("DW_OP_entry_value");
}
- CORE_ADDR get_object_address () override
- {
- invalid ("DW_OP_push_object_address");
- }
-
private:
void invalid (const char *op) ATTRIBUTE_NORETURN
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index d88239880af..07b7ececff5 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -632,18 +632,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
: dwarf_expr_context (per_objfile)
{}
- CORE_ADDR obj_address;
-
- /* Callback function for get_object_address. Return the address of the VLA
- object. */
-
- CORE_ADDR get_object_address () override
- {
- if (obj_address == 0)
- error (_("Location address is not set."));
- return obj_address;
- }
-
/* Execute DWARF block of call_site_parameter which matches KIND and
KIND_U. Choose DEREF_SIZE value of that parameter. Search
caller of this objects's frame.
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 08/17] Move read_mem to dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (6 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 07/17] Move get_object_address " Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 09/17] Move push_dwarf_reg_entry_value to expr.c Zoran Zaric
` (9 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
Following the idea of merging the evaluators, the read_mem method can
be moved from dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Move from
dwarf_evaluate_loc_desc.
* dwarf2/frame.c (dwarf_expr_executor::read_mem): Remove
method.
* dwarf2/loc.c (dwarf_evaluate_loc_desc::read_mem): Move to
dwarf_expr_context.
---
gdb/dwarf2/expr.c | 9 +++++++++
gdb/dwarf2/expr.h | 2 +-
gdb/dwarf2/frame.c | 5 -----
gdb/dwarf2/loc.c | 7 -------
4 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 689224b63a3..b2d58ff4afe 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -257,6 +257,15 @@ dwarf_expr_context::dwarf_call (cu_offset die_cu_off)
this->eval (block.data, block.size);
}
+/* See expr.h. */
+
+void
+dwarf_expr_context::read_mem (gdb_byte *buf, CORE_ADDR addr,
+ size_t length)
+{
+ read_memory (addr, buf, length);
+}
+
/* 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 ad1918eb18c..7e2aed07ed7 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -192,7 +192,7 @@ struct dwarf_expr_context
CORE_ADDR obj_address = 0;
/* Read LENGTH bytes at ADDR into BUF. */
- virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length) = 0;
+ virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length);
/* Push on DWARF stack an entry evaluated for DW_TAG_call_site's
parameter matching KIND and KIND_U at the caller of specified BATON.
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 52b09809ccb..99330092c12 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -232,11 +232,6 @@ class dwarf_expr_executor : public dwarf_expr_context
: dwarf_expr_context (per_objfile)
{}
- void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
- {
- read_memory (addr, buf, len);
- }
-
void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
union call_site_parameter_u kind_u,
int deref_size) override
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 07b7ececff5..34dd3a55fdd 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -686,13 +686,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
this->eval (data_src, size);
}
-
- /* 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);
- }
};
/* See dwarf2loc.h. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 09/17] Move push_dwarf_reg_entry_value to expr.c
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (7 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 08/17] Move read_mem " Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 10/17] Inline get_reg_value method of dwarf_expr_context Zoran Zaric
` (8 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
Following the idea of merging the evaluators, the
push_dwarf_reg_entry_value method can be moved from
dwarf_expr_executor and dwarf_evaluate_loc_desc classes
to their base class dwarf_expr_context.
gdb/ChangeLog:
* dwarf2/expr.c
(dwarf_expr_context::push_dwarf_reg_entry_value): Move from
dwarf_evaluate_loc_desc.
* dwarf2/frame.c
(dwarf_expr_executor::push_dwarf_reg_entry_value): Remove
method.
* dwarf2/loc.c (dwarf_expr_reg_to_entry_parameter): Expose
function.
(dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value): Move to
dwarf_expr_context.
* dwarf2/loc.h (dwarf_expr_reg_to_entry_parameter): Expose
function.
---
gdb/dwarf2/expr.c | 50 +++++++++++++++++++++++++++++++++
gdb/dwarf2/expr.h | 16 +++++------
gdb/dwarf2/frame.c | 7 -----
gdb/dwarf2/loc.c | 70 ++--------------------------------------------
gdb/dwarf2/loc.h | 12 ++++++++
5 files changed, 72 insertions(+), 83 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index b2d58ff4afe..303b2b5ba14 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -266,6 +266,56 @@ dwarf_expr_context::read_mem (gdb_byte *buf, CORE_ADDR addr,
read_memory (addr, buf, length);
}
+/* See expr.h. */
+
+void
+dwarf_expr_context::push_dwarf_reg_entry_value
+ (enum call_site_parameter_kind kind, union call_site_parameter_u kind_u,
+ int deref_size)
+{
+ ensure_have_per_cu (this->per_cu, "DW_OP_entry_value");
+ ensure_have_frame (this->frame, "DW_OP_entry_value");
+
+ dwarf2_per_cu_data *caller_per_cu;
+ dwarf2_per_objfile *caller_per_objfile;
+ struct frame_info *caller_frame = get_prev_frame (this->frame);
+ struct call_site_parameter *parameter
+ = dwarf_expr_reg_to_entry_parameter (this->frame, kind, kind_u,
+ &caller_per_cu,
+ &caller_per_objfile);
+ const gdb_byte *data_src
+ = deref_size == -1 ? parameter->value : parameter->data_value;
+ size_t size
+ = deref_size == -1 ? parameter->value_size : parameter->data_value_size;
+
+ /* DEREF_SIZE size is not verified here. */
+ if (data_src == nullptr)
+ throw_error (NO_ENTRY_VALUE_ERROR,
+ _("Cannot resolve DW_AT_call_data_value"));
+
+ /* We are about to evaluate an expression in the context of the caller
+ of the current frame. This evaluation context may be different from
+ the current (callee's) context), so temporarily set the caller's context.
+
+ It is possible for the caller to be from a different objfile from the
+ callee if the call is made through a function pointer. */
+ 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_per_objfile = make_scoped_restore (&this->per_objfile,
+ caller_per_objfile);
+
+ scoped_restore save_arch = make_scoped_restore (&this->gdbarch);
+ this->gdbarch = this->per_objfile->objfile->arch ();
+ scoped_restore save_addr_size = make_scoped_restore (&this->addr_size);
+ this->addr_size = this->per_cu->addr_size ();
+
+ this->eval (data_src, size);
+}
+
/* 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 7e2aed07ed7..72cf14abcfc 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -194,14 +194,6 @@ struct dwarf_expr_context
/* Read LENGTH bytes at ADDR into BUF. */
virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length);
- /* Push on DWARF stack an entry evaluated for DW_TAG_call_site's
- parameter matching KIND and KIND_U at the caller of specified BATON.
- If DEREF_SIZE is not -1 then use DW_AT_call_data_value instead of
- DW_AT_call_value. */
- virtual void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
- union call_site_parameter_u kind_u,
- int deref_size) = 0;
-
/* Return the `object address' for DW_OP_push_object_address. */
virtual CORE_ADDR get_object_address ()
{
@@ -240,6 +232,14 @@ struct dwarf_expr_context
STACK while it being passed to and returned from the called DWARF
subroutine. */
void dwarf_call (cu_offset die_cu_off);
+
+ /* Push on DWARF stack an entry evaluated for DW_TAG_call_site's
+ parameter matching KIND and KIND_U at the caller of specified BATON.
+ If DEREF_SIZE is not -1 then use DW_AT_call_data_value instead of
+ DW_AT_call_value. */
+ void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
+ union call_site_parameter_u kind_u,
+ int deref_size);
};
/* Return the value of register number REG (a DWARF register number),
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 99330092c12..75f6c4d18e8 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -232,13 +232,6 @@ class dwarf_expr_executor : public dwarf_expr_context
: dwarf_expr_context (per_objfile)
{}
- void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
- union call_site_parameter_u kind_u,
- int deref_size) override
- {
- invalid ("DW_OP_entry_value");
- }
-
private:
void invalid (const char *op) ATTRIBUTE_NORETURN
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 34dd3a55fdd..63f060417be 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -52,13 +52,6 @@ static struct value *dwarf2_evaluate_loc_desc_full
size_t size, dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
struct type *subobj_type, LONGEST subobj_byte_offset);
-static struct call_site_parameter *dwarf_expr_reg_to_entry_parameter
- (struct frame_info *frame,
- enum call_site_parameter_kind kind,
- union call_site_parameter_u kind_u,
- dwarf2_per_cu_data **per_cu_return,
- dwarf2_per_objfile **per_objfile_return);
-
static struct value *indirect_synthetic_pointer
(sect_offset die, LONGEST byte_offset,
dwarf2_per_cu_data *per_cu,
@@ -631,61 +624,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
dwarf_evaluate_loc_desc (dwarf2_per_objfile *per_objfile)
: dwarf_expr_context (per_objfile)
{}
-
- /* Execute DWARF block of call_site_parameter which matches KIND and
- KIND_U. Choose DEREF_SIZE value of that parameter. Search
- caller of this objects's frame.
-
- The caller can be from a different CU - per_cu_dwarf_call
- implementation can be more simple as it does not support cross-CU
- DWARF executions. */
-
- void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
- union call_site_parameter_u kind_u,
- int deref_size) override
- {
- struct frame_info *caller_frame;
- dwarf2_per_cu_data *caller_per_cu;
- dwarf2_per_objfile *caller_per_objfile;
- struct call_site_parameter *parameter;
- const gdb_byte *data_src;
- size_t size;
-
- caller_frame = get_prev_frame (frame);
-
- parameter = dwarf_expr_reg_to_entry_parameter (frame, kind, kind_u,
- &caller_per_cu,
- &caller_per_objfile);
- data_src = deref_size == -1 ? parameter->value : parameter->data_value;
- size = deref_size == -1 ? parameter->value_size : parameter->data_value_size;
-
- /* DEREF_SIZE size is not verified here. */
- if (data_src == NULL)
- throw_error (NO_ENTRY_VALUE_ERROR,
- _("Cannot resolve DW_AT_call_data_value"));
-
- /* We are about to evaluate an expression in the context of the caller
- of the current frame. This evaluation context may be different from
- the current (callee's) context), so temporarily set the caller's context.
-
- It is possible for the caller to be from a different objfile from the
- callee if the call is made through a function pointer. */
- 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_per_objfile = make_scoped_restore (&this->per_objfile,
- caller_per_objfile);
-
- scoped_restore save_arch = make_scoped_restore (&this->gdbarch);
- this->gdbarch = this->per_objfile->objfile->arch ();
- scoped_restore save_addr_size = make_scoped_restore (&this->addr_size);
- this->addr_size = this->per_cu->addr_size ();
-
- this->eval (data_src, size);
- }
};
/* See dwarf2loc.h. */
@@ -1166,13 +1104,9 @@ call_site_parameter_matches (struct call_site_parameter *parameter,
return 0;
}
-/* Fetch call_site_parameter from caller matching KIND and KIND_U.
- FRAME is for callee.
-
- Function always returns non-NULL, it throws NO_ENTRY_VALUE_ERROR
- otherwise. */
+/* See loc.h. */
-static struct call_site_parameter *
+struct call_site_parameter *
dwarf_expr_reg_to_entry_parameter (struct frame_info *frame,
enum call_site_parameter_kind kind,
union call_site_parameter_u kind_u,
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index a729b370e2e..e79bce6eb75 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -61,6 +61,18 @@ struct value *sect_variable_value (sect_offset sect_off,
dwarf2_per_cu_data *per_cu,
dwarf2_per_objfile *per_objfile);
+/* Fetch call_site_parameter from caller matching KIND and KIND_U.
+ FRAME is for callee.
+
+ Function always returns non-NULL, it throws NO_ENTRY_VALUE_ERROR
+ otherwise. */
+
+struct call_site_parameter *dwarf_expr_reg_to_entry_parameter
+ (struct frame_info *frame, enum call_site_parameter_kind kind,
+ union call_site_parameter_u kind_u, dwarf2_per_cu_data **per_cu_return,
+ dwarf2_per_objfile **per_objfile_return);
+
+
/* Evaluate a location description, starting at DATA and with length
SIZE, to find the current location of variable of TYPE in the context
of FRAME. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 10/17] Inline get_reg_value method of dwarf_expr_context
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (8 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 09/17] Move push_dwarf_reg_entry_value to expr.c Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 11/17] Remove empty frame and full evaluators Zoran Zaric
` (7 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
The get_reg_value method is a small function that is only called once,
so it can be inlined to simplify the dwarf_expr_context class.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::get_reg_value): Remove
method.
(dwarf_expr_context::execute_stack_op): Inline get_reg_value
method.
* dwarf2/expr.h (dwarf_expr_context::get_reg_value): Remove
method.
---
gdb/dwarf2/expr.c | 24 +++++++-----------------
gdb/dwarf2/expr.h | 6 ------
2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 303b2b5ba14..1a068741894 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -176,19 +176,6 @@ dwarf_expr_context::fetch (int n)
/* See expr.h. */
-struct value *
-dwarf_expr_context::get_reg_value (struct type *type, int reg)
-{
- ensure_have_frame (this->frame, "DW_OP_regval_type");
-
- 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);
-}
-
-/* See expr.h. */
-
void
dwarf_expr_context::get_frame_base (const gdb_byte **start,
size_t * length)
@@ -1544,14 +1531,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_regval_type:
case DW_OP_GNU_regval_type:
{
- struct type *type;
-
op_ptr = safe_read_uleb128 (op_ptr, op_end, ®);
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
cu_offset type_die_cu_off = (cu_offset) uoffset;
- type = get_base_type (type_die_cu_off);
- result_val = this->get_reg_value (type, reg);
+ ensure_have_frame (this->frame, "DW_OP_regval_type");
+
+ struct type *type = get_base_type (type_die_cu_off);
+ int regnum
+ = dwarf_reg_to_regnum_or_error (get_frame_arch (this->frame),
+ reg);
+ result_val = value_from_register (type, regnum, this->frame);
}
break;
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index 72cf14abcfc..aa80d1f31cb 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -211,12 +211,6 @@ struct dwarf_expr_context
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
- in a current context.
-
- 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. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 11/17] Remove empty frame and full evaluators
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (9 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 10/17] Inline get_reg_value method of dwarf_expr_context Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 12/17] Merge evaluate_for_locexpr_baton evaluator Zoran Zaric
` (6 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
There are no virtual methods that require different specialization in
dwarf_expr_context class. This means that derived classes
dwarf_expr_executor and dwarf_evaluate_loc_desc are not needed any
more.
As a result of this, the evaluate_for_locexpr_baton class base class
is now the dwarf_expr_context class.
There might be a need for a better class hierarchy when we know more
about the direction of the future DWARF versions and gdb extensions,
but that is out of the scope of this patch series.
gdb/ChangeLog:
* dwarf2/frame.c (class dwarf_expr_executor): Remove class.
(execute_stack_op): Instantiate dwarf_expr_context instead of
dwarf_evaluate_loc_desc class.
* dwarf2/loc.c (class dwarf_evaluate_loc_desc): Remove class.
(dwarf2_evaluate_loc_desc_full): Instantiate dwarf_expr_context
instead of dwarf_evaluate_loc_desc class.
(struct evaluate_for_locexpr_baton): Derive from
dwarf_expr_context.
---
gdb/dwarf2/frame.c | 18 +-----------------
gdb/dwarf2/loc.c | 16 ++++------------
2 files changed, 5 insertions(+), 29 deletions(-)
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 75f6c4d18e8..78dbb489506 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -224,22 +224,6 @@ register %s (#%d) at %s"),
}
}
-class dwarf_expr_executor : public dwarf_expr_context
-{
-public:
-
- dwarf_expr_executor (dwarf2_per_objfile *per_objfile)
- : dwarf_expr_context (per_objfile)
- {}
-
- private:
-
- void invalid (const char *op) ATTRIBUTE_NORETURN
- {
- error (_("%s is invalid in this context"), op);
- }
-};
-
static CORE_ADDR
execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
struct frame_info *this_frame, CORE_ADDR initial,
@@ -247,7 +231,7 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
{
CORE_ADDR result;
- dwarf_expr_executor ctx (per_objfile);
+ dwarf_expr_context ctx (per_objfile);
scoped_value_mark free_values;
ctx.frame = this_frame;
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 63f060417be..6d639c2e4cd 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -618,14 +618,6 @@ sect_variable_value (sect_offset sect_off,
type, true);
}
-class dwarf_evaluate_loc_desc : public dwarf_expr_context
-{
-public:
- dwarf_evaluate_loc_desc (dwarf2_per_objfile *per_objfile)
- : dwarf_expr_context (per_objfile)
- {}
-};
-
/* See dwarf2loc.h. */
unsigned int entry_values_debug = 0;
@@ -2029,7 +2021,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
if (size == 0)
return allocate_optimized_out_value (subobj_type);
- dwarf_evaluate_loc_desc ctx (per_objfile);
+ dwarf_expr_context ctx (per_objfile);
ctx.frame = frame;
ctx.per_cu = per_cu;
ctx.obj_address = 0;
@@ -2228,16 +2220,16 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
per_objfile, NULL, 0);
}
-/* A specialization of dwarf_evaluate_loc_desc that is used by
+/* A specialization of dwarf_expr_context that is used by
dwarf2_locexpr_baton_eval. This subclass exists to handle the case
where a caller of dwarf2_locexpr_baton_eval passes in some data,
but with the address being 0. In this situation, we arrange for
memory reads to come from the passed-in buffer. */
-struct evaluate_for_locexpr_baton : public dwarf_evaluate_loc_desc
+struct evaluate_for_locexpr_baton : public dwarf_expr_context
{
evaluate_for_locexpr_baton (dwarf2_per_objfile *per_objfile)
- : dwarf_evaluate_loc_desc (per_objfile)
+ : dwarf_expr_context (per_objfile)
{}
/* The data that was passed in. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 12/17] Merge evaluate_for_locexpr_baton evaluator
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (10 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 11/17] Remove empty frame and full evaluators Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 13/17] Move piece_closure and its support to expr.c Zoran Zaric
` (5 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
The evaluate_for_locexpr_baton is the last derived class from the
dwarf_expr_context class. It's purpose is to support the passed in
buffer functionality.
Although, it is not really necessary to merge this class with it's
base class, doing that simplifies new expression evaluator design.
Considering that this functionality is going around the DWARF standard,
it is also reasonable to expect that with a new evaluator design and
extending the push object address functionality to accept any location
description, there will be no need to support passed in buffers.
Alternatively, it would also makes sense to abstract the interaction
between the evaluator and a given resource in the near future. The
passed in buffer would then be a specialization of that abstraction.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::read_mem): Merge with
evaluate_for_locexpr_baton implementation.
* dwarf2/loc.c (class evaluate_for_locexpr_baton): Remove
class.
(evaluate_for_locexpr_baton::read_mem): Move to
dwarf_expr_context.
(dwarf2_locexpr_baton_eval): Instantiate dwarf_expr_context
instead of evaluate_for_locexpr_baton class.
---
gdb/dwarf2/expr.c | 20 ++++++++++++++++++--
gdb/dwarf2/expr.h | 18 ++++++++----------
gdb/dwarf2/loc.c | 45 ++-------------------------------------------
3 files changed, 28 insertions(+), 55 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 1a068741894..3a3f3ef08b6 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -250,6 +250,19 @@ void
dwarf_expr_context::read_mem (gdb_byte *buf, CORE_ADDR addr,
size_t length)
{
+ if (length == 0)
+ return;
+
+ /* Prefer the passed-in memory, if it exists. */
+ CORE_ADDR offset = addr - this->obj_address;
+
+ if (offset < this->data_view.size ()
+ && offset + length <= this->data_view.size ())
+ {
+ memcpy (buf, this->data_view.data (), length);
+ return;
+ }
+
read_memory (addr, buf, length);
}
@@ -1581,8 +1594,11 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_push_object_address:
/* Return the address of the object we are currently observing. */
- result = this->get_object_address ();
- result_val = value_from_ulongest (address_type, result);
+ if (this->data_view.data () == nullptr
+ && this->obj_address == 0)
+ error (_("Location address is not set."));
+
+ result_val = value_from_ulongest (address_type, this->obj_address);
break;
default:
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index aa80d1f31cb..35852183fc5 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -191,16 +191,8 @@ struct dwarf_expr_context
/* Object address used for the evaluation. */
CORE_ADDR obj_address = 0;
- /* Read LENGTH bytes at ADDR into BUF. */
- virtual void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length);
-
- /* Return the `object address' for DW_OP_push_object_address. */
- virtual CORE_ADDR get_object_address ()
- {
- if (obj_address == 0)
- error (_("Location address is not set."));
- return obj_address;
- }
+ /* The data that was passed in. */
+ gdb::array_view<const gdb_byte> data_view;
private:
@@ -234,6 +226,12 @@ struct dwarf_expr_context
void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
union call_site_parameter_u kind_u,
int deref_size);
+
+ /* Read LENGTH bytes at ADDR into BUF. This method also handles the
+ case where a caller of the evaluator passes in some data,
+ but with the address being 0. In this situation, we arrange for
+ memory reads to come from the passed-in buffer. */
+ void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length);
};
/* Return the value of register number REG (a DWARF register number),
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 6d639c2e4cd..ef25f3c5e86 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2220,45 +2220,6 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
per_objfile, NULL, 0);
}
-/* A specialization of dwarf_expr_context that is used by
- dwarf2_locexpr_baton_eval. This subclass exists to handle the case
- where a caller of dwarf2_locexpr_baton_eval passes in some data,
- but with the address being 0. In this situation, we arrange for
- memory reads to come from the passed-in buffer. */
-
-struct evaluate_for_locexpr_baton : public dwarf_expr_context
-{
- evaluate_for_locexpr_baton (dwarf2_per_objfile *per_objfile)
- : dwarf_expr_context (per_objfile)
- {}
-
- /* The data that was passed in. */
- gdb::array_view<const gdb_byte> data_view;
-
- CORE_ADDR get_object_address () override
- {
- if (data_view.data () == nullptr && obj_address == 0)
- error (_("Location address is not set."));
- return obj_address;
- }
-
- void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
- {
- if (len == 0)
- return;
-
- /* Prefer the passed-in memory, if it exists. */
- CORE_ADDR offset = addr - obj_address;
- if (offset < data_view.size () && offset + len <= data_view.size ())
- {
- memcpy (buf, data_view.data (), len);
- return;
- }
-
- read_memory (addr, buf, len);
- }
-};
-
/* Evaluates a dwarf expression and stores the result in VAL,
expecting that the dwarf expression only produces a single
CORE_ADDR. FRAME is the frame in which the expression is
@@ -2282,13 +2243,11 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
return 0;
dwarf2_per_objfile *per_objfile = dlbaton->per_objfile;
- evaluate_for_locexpr_baton ctx (per_objfile);
+ dwarf_expr_context ctx (per_objfile);
ctx.frame = frame;
ctx.per_cu = dlbaton->per_cu;
- if (addr_stack == nullptr)
- ctx.obj_address = 0;
- else
+ if (addr_stack != nullptr)
{
ctx.obj_address = addr_stack->addr;
ctx.data_view = addr_stack->valaddr;
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 13/17] Move piece_closure and its support to expr.c
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (11 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 12/17] Merge evaluate_for_locexpr_baton evaluator Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 14/17] Make value_copy also copy the stack data member Zoran Zaric
` (4 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
Following 5 patches series is trying to clean up the interface of the
DWARF expression evaluator class (dwarf_expr_context).
After merging all expression evaluators into one class, the next
logical step is to make a clean user interface for that class. To do
that, we first need to address the issue of class users writing and
reading the internal data of the class directly.
Fixing the case of writing is simple, it makes sense for an evaluator
instance to be per architecture basis. Currently, the best separation
seems to be per object file, so having that data (dwarf2_per_objfile)
as a constructor argument makes sense. It also makes sense to get the
address size from that object file, but unfortunately that interface
does not exist at the moment.
Luckily, address size information is already available to the users
through other means. As a result, the address size also needs to be a
class constructor argument, at least until a better interface for
acquiring that information from an object file is implemented.
The rest of the user written data comes down to a context of an
evaluated expression (compilation unit context, frame context and
passed in buffer context) and a source type information that a result
of evaluating expression is representing. So, it makes sense for all of
these to be arguments of an evaluation method.
To address the problem of reading the dwarf_expr_context class
internal data, we first need to understand why it is implemented that
way?
This is actualy a question of which existing class can be used to
represent both values and a location descriptions and why it is not
used currently?
The answer is in a struct value class/structure, but the problem is
that before the evaluators were merged, only one evaluator had an
infrastructure to resolve composite and implicit pointer location
descriptions.
After the merge, we are now able to use the struct value to represent
any result of the expression evaluation. It also makes sense to move
all infrastructure for those location descriptions to the expr.c file
considering that that is the only place using that infrastructure.
What we are left with in the end is a clean public interface of the
dwarf_expr_context class containing:
- constructor,
- destructor,
- push_address method and
- eval_exp method.
The idea with this particular patch is to move piece_closure structure
and the interface that handles it (lval_funcs) to expr.c file.
While implicit pointer location descriptions are still not useful in
the CFI context (of the AMD’s DWARF standard extensions), the composite
location descriptions are certainly necessary to describe a results of
specific compiler optimizations.
Considering that a piece_closure structure is used to represent both,
there was no benefit in splitting them.
gdb/ChangeLog:
* dwarf2/expr.c (struct piece_closure): Add from loc.c.
(allocate_piece_closure): Add from loc.c.
(bits_to_bytes): Add from loc.c.
(rw_pieced_value): Add from loc.c.
(read_pieced_value): Add from loc.c.
(write_pieced_value): Add from loc.c.
(check_pieced_synthetic_pointer): Add from loc.c.
(indirect_pieced_value): Add from loc.c.
(coerce_pieced_ref): Add from loc.c.
(copy_pieced_value_closure): Add from loc.c.
(free_pieced_value_closure): Add from loc.c.
(sect_variable_value): Add from loc.c.
* dwarf2/loc.c (sect_variable_value): Move to expr.c.
(struct piece_closure): Move to expr.c.
(allocate_piece_closure): Move to expr.c.
(bits_to_bytes): Move to expr.c.
(rw_pieced_value): Move to expr.c.
(read_pieced_value): Move to expr.c.
(write_pieced_value): Move to expr.c.
(check_pieced_synthetic_pointer): Move to expr.c.
(indirect_pieced_value): Move to expr.c.
(coerce_pieced_ref): Move to expr.c.
(copy_pieced_value_closure): Move to expr.c.
(free_pieced_value_closure): Move to expr.c.
---
gdb/dwarf2/expr.c | 563 ++++++++++++++++++++++++++++++++++++++++++++
gdb/dwarf2/expr.h | 10 +
gdb/dwarf2/loc.c | 581 +---------------------------------------------
gdb/dwarf2/loc.h | 15 +-
4 files changed, 582 insertions(+), 587 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 3a3f3ef08b6..022a97f1a07 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -86,6 +86,15 @@ ensure_have_per_cu (struct dwarf2_per_cu_data *per_cu, const char* op_name)
_("%s evaluation requires a compilation unit."), op_name);
}
+/* Return the number of bytes overlapping a contiguous chunk of N_BITS
+ bits whose first bit is located at bit offset START. */
+
+static size_t
+bits_to_bytes (ULONGEST start, ULONGEST n_bits)
+{
+ return (start % HOST_CHAR_BIT + n_bits + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT;
+}
+
/* See expr.h. */
CORE_ADDR
@@ -97,6 +106,560 @@ read_addr_from_reg (struct frame_info *frame, int reg)
return address_from_register (regnum, frame);
}
+struct piece_closure
+{
+ /* Reference count. */
+ int refc = 0;
+
+ /* The objfile from which this closure's expression came. */
+ dwarf2_per_objfile *per_objfile = nullptr;
+
+ /* The CU from which this closure's expression came. */
+ struct dwarf2_per_cu_data *per_cu = nullptr;
+
+ /* The pieces describing this variable. */
+ std::vector<dwarf_expr_piece> pieces;
+
+ /* Frame ID of frame to which a register value is relative, used
+ only by DWARF_VALUE_REGISTER. */
+ struct frame_id frame_id;
+};
+
+/* See expr.h. */
+
+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)
+{
+ struct piece_closure *c = new piece_closure;
+
+ c->refc = 1;
+ /* We must capture this here due to sharing of DWARF state. */
+ c->per_objfile = per_objfile;
+ c->per_cu = per_cu;
+ c->pieces = std::move (pieces);
+ if (frame == nullptr)
+ c->frame_id = null_frame_id;
+ else
+ c->frame_id = get_frame_id (frame);
+
+ for (dwarf_expr_piece &piece : c->pieces)
+ if (piece.location == DWARF_VALUE_STACK)
+ value_incref (piece.v.value);
+
+ return c;
+}
+
+/* Read or write a pieced value V. If FROM != NULL, operate in "write
+ mode": copy FROM into the pieces comprising V. If FROM == NULL,
+ operate in "read mode": fetch the contents of the (lazy) value V by
+ composing it from its pieces. */
+
+static void
+rw_pieced_value (struct value *v, struct value *from)
+{
+ int i;
+ LONGEST offset = 0, max_offset;
+ gdb_byte *v_contents;
+ const gdb_byte *from_contents;
+ struct piece_closure *c
+ = (struct piece_closure *) value_computed_closure (v);
+ gdb::byte_vector buffer;
+ bool bits_big_endian = type_byte_order (value_type (v)) == BFD_ENDIAN_BIG;
+
+ if (from != nullptr)
+ {
+ from_contents = value_contents (from);
+ v_contents = nullptr;
+ }
+ else
+ {
+ if (value_type (v) != value_enclosing_type (v))
+ internal_error (__FILE__, __LINE__,
+ _("Should not be able to create a lazy value with "
+ "an enclosing type"));
+ v_contents = value_contents_raw (v);
+ from_contents = nullptr;
+ }
+
+ ULONGEST bits_to_skip = 8 * value_offset (v);
+ if (value_bitsize (v))
+ {
+ bits_to_skip += (8 * value_offset (value_parent (v))
+ + value_bitpos (v));
+ if (from != nullptr
+ && (type_byte_order (value_type (from))
+ == BFD_ENDIAN_BIG))
+ {
+ /* Use the least significant bits of FROM. */
+ max_offset = 8 * TYPE_LENGTH (value_type (from));
+ offset = max_offset - value_bitsize (v);
+ }
+ else
+ max_offset = value_bitsize (v);
+ }
+ else
+ max_offset = 8 * TYPE_LENGTH (value_type (v));
+
+ /* Advance to the first non-skipped piece. */
+ for (i = 0; i < c->pieces.size () && bits_to_skip >= c->pieces[i].size; i++)
+ bits_to_skip -= c->pieces[i].size;
+
+ for (; i < c->pieces.size () && offset < max_offset; i++)
+ {
+ struct dwarf_expr_piece *p = &c->pieces[i];
+ size_t this_size_bits, this_size;
+
+ this_size_bits = p->size - bits_to_skip;
+ if (this_size_bits > max_offset - offset)
+ this_size_bits = max_offset - offset;
+
+ switch (p->location)
+ {
+ case DWARF_VALUE_REGISTER:
+ {
+ struct frame_info *frame = frame_find_by_id (c->frame_id);
+ struct gdbarch *arch = get_frame_arch (frame);
+ int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
+ ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
+ int optim, unavail;
+
+ if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
+ && p->offset + p->size < reg_bits)
+ {
+ /* Big-endian, and we want less than full size. */
+ bits_to_skip += reg_bits - (p->offset + p->size);
+ }
+ else
+ bits_to_skip += p->offset;
+
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
+ buffer.resize (this_size);
+
+ if (from == nullptr)
+ {
+ /* Read mode. */
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ buffer, &optim, &unavail))
+ {
+ if (optim)
+ mark_value_bits_optimized_out (v, offset,
+ this_size_bits);
+ if (unavail)
+ mark_value_bits_unavailable (v, offset,
+ this_size_bits);
+ break;
+ }
+
+ copy_bitwise (v_contents, offset,
+ buffer.data (), bits_to_skip % 8,
+ this_size_bits, bits_big_endian);
+ }
+ else
+ {
+ /* Write mode. */
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
+ {
+ /* Data is copied non-byte-aligned into the register.
+ Need some bits from original register value. */
+ get_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ buffer, &optim, &unavail);
+ if (optim)
+ throw_error (OPTIMIZED_OUT_ERROR,
+ _("Can't do read-modify-write to "
+ "update bitfield; containing word "
+ "has been optimized out"));
+ if (unavail)
+ throw_error (NOT_AVAILABLE_ERROR,
+ _("Can't do read-modify-write to "
+ "update bitfield; containing word "
+ "is unavailable"));
+ }
+
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ from_contents, offset,
+ this_size_bits, bits_big_endian);
+ put_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ buffer);
+ }
+ }
+ break;
+
+ case DWARF_VALUE_MEMORY:
+ {
+ bits_to_skip += p->offset;
+
+ CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
+
+ if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
+ && offset % 8 == 0)
+ {
+ /* Everything is byte-aligned; no buffer needed. */
+ if (from != nullptr)
+ write_memory_with_notification (start_addr,
+ (from_contents
+ + offset / 8),
+ this_size_bits / 8);
+ else
+ read_value_memory (v, offset,
+ p->v.mem.in_stack_memory,
+ p->v.mem.addr + bits_to_skip / 8,
+ v_contents + offset / 8,
+ this_size_bits / 8);
+ break;
+ }
+
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
+ buffer.resize (this_size);
+
+ if (from == nullptr)
+ {
+ /* Read mode. */
+ read_value_memory (v, offset,
+ p->v.mem.in_stack_memory,
+ p->v.mem.addr + bits_to_skip / 8,
+ buffer.data (), this_size);
+ copy_bitwise (v_contents, offset,
+ buffer.data (), bits_to_skip % 8,
+ this_size_bits, bits_big_endian);
+ }
+ else
+ {
+ /* Write mode. */
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
+ {
+ if (this_size <= 8)
+ {
+ /* Perform a single read for small sizes. */
+ read_memory (start_addr, buffer.data (),
+ this_size);
+ }
+ else
+ {
+ /* Only the first and last bytes can possibly have
+ any bits reused. */
+ read_memory (start_addr, buffer.data (), 1);
+ read_memory (start_addr + this_size - 1,
+ &buffer[this_size - 1], 1);
+ }
+ }
+
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ from_contents, offset,
+ this_size_bits, bits_big_endian);
+ write_memory_with_notification (start_addr,
+ buffer.data (),
+ this_size);
+ }
+ }
+ break;
+
+ case DWARF_VALUE_STACK:
+ {
+ if (from != nullptr)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
+ gdbarch *objfile_gdbarch = c->per_objfile->objfile->arch ();
+ ULONGEST stack_value_size_bits
+ = 8 * TYPE_LENGTH (value_type (p->v.value));
+
+ /* Use zeroes if piece reaches beyond stack value. */
+ if (p->offset + p->size > stack_value_size_bits)
+ break;
+
+ /* Piece is anchored at least significant bit end. */
+ if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
+ bits_to_skip += stack_value_size_bits - p->offset - p->size;
+ else
+ bits_to_skip += p->offset;
+
+ copy_bitwise (v_contents, offset,
+ value_contents_all (p->v.value),
+ bits_to_skip,
+ this_size_bits, bits_big_endian);
+ }
+ break;
+
+ case DWARF_VALUE_LITERAL:
+ {
+ if (from != nullptr)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
+ ULONGEST literal_size_bits = 8 * p->v.literal.length;
+ size_t n = this_size_bits;
+
+ /* Cut off at the end of the implicit value. */
+ bits_to_skip += p->offset;
+ if (bits_to_skip >= literal_size_bits)
+ break;
+ if (n > literal_size_bits - bits_to_skip)
+ n = literal_size_bits - bits_to_skip;
+
+ copy_bitwise (v_contents, offset,
+ p->v.literal.data, bits_to_skip,
+ n, bits_big_endian);
+ }
+ break;
+
+ case DWARF_VALUE_IMPLICIT_POINTER:
+ if (from != nullptr)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
+ /* These bits show up as zeros -- but do not cause the value to
+ be considered optimized-out. */
+ break;
+
+ case DWARF_VALUE_OPTIMIZED_OUT:
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+
+ default:
+ internal_error (__FILE__, __LINE__, _("invalid location type"));
+ }
+
+ offset += this_size_bits;
+ bits_to_skip = 0;
+ }
+}
+
+static void
+read_pieced_value (struct value *v)
+{
+ rw_pieced_value (v, nullptr);
+}
+
+static void
+write_pieced_value (struct value *to, struct value *from)
+{
+ rw_pieced_value (to, from);
+}
+
+/* An implementation of an lval_funcs method to see whether a value is
+ a synthetic pointer. */
+
+static int
+check_pieced_synthetic_pointer (const struct value *value, LONGEST bit_offset,
+ int bit_length)
+{
+ struct piece_closure *c
+ = (struct piece_closure *) value_computed_closure (value);
+ int i;
+
+ bit_offset += 8 * value_offset (value);
+ if (value_bitsize (value))
+ bit_offset += value_bitpos (value);
+
+ for (i = 0; i < c->pieces.size () && bit_length > 0; i++)
+ {
+ struct dwarf_expr_piece *p = &c->pieces[i];
+ size_t this_size_bits = p->size;
+
+ if (bit_offset > 0)
+ {
+ if (bit_offset >= this_size_bits)
+ {
+ bit_offset -= this_size_bits;
+ continue;
+ }
+
+ bit_length -= this_size_bits - bit_offset;
+ bit_offset = 0;
+ }
+ else
+ bit_length -= this_size_bits;
+
+ if (p->location != DWARF_VALUE_IMPLICIT_POINTER)
+ return 0;
+ }
+
+ return 1;
+}
+
+/* An implementation of an lval_funcs method to indirect through a
+ pointer. This handles the synthetic pointer case when needed. */
+
+static struct value *
+indirect_pieced_value (struct value *value)
+{
+ struct piece_closure *c
+ = (struct piece_closure *) value_computed_closure (value);
+ int i;
+ struct dwarf_expr_piece *piece = NULL;
+
+ struct type *type = check_typedef (value_type (value));
+ if (type->code () != TYPE_CODE_PTR)
+ return NULL;
+
+ int bit_length = 8 * TYPE_LENGTH (type);
+ LONGEST bit_offset = 8 * value_offset (value);
+ if (value_bitsize (value))
+ bit_offset += value_bitpos (value);
+
+ for (i = 0; i < c->pieces.size () && bit_length > 0; i++)
+ {
+ struct dwarf_expr_piece *p = &c->pieces[i];
+ size_t this_size_bits = p->size;
+
+ if (bit_offset > 0)
+ {
+ if (bit_offset >= this_size_bits)
+ {
+ bit_offset -= this_size_bits;
+ continue;
+ }
+
+ bit_length -= this_size_bits - bit_offset;
+ bit_offset = 0;
+ }
+ else
+ bit_length -= this_size_bits;
+
+ if (p->location != DWARF_VALUE_IMPLICIT_POINTER)
+ return NULL;
+
+ if (bit_length != 0)
+ error (_("Invalid use of DW_OP_implicit_pointer"));
+
+ piece = p;
+ break;
+ }
+
+ gdb_assert (piece != NULL && c->per_cu != nullptr);
+ struct frame_info *frame = get_selected_frame (_("No frame selected."));
+
+ /* This is an offset requested by GDB, such as value subscripts.
+ However, due to how synthetic pointers are implemented, this is
+ always presented to us as a pointer type. This means we have to
+ sign-extend it manually as appropriate. Use raw
+ extract_signed_integer directly rather than value_as_address and
+ sign extend afterwards on architectures that would need it
+ (mostly everywhere except MIPS, which has signed addresses) as
+ the later would go through gdbarch_pointer_to_address and thus
+ return a CORE_ADDR with high bits set on architectures that
+ encode address spaces and other things in CORE_ADDR. */
+ enum bfd_endian byte_order = gdbarch_byte_order (get_frame_arch (frame));
+ LONGEST byte_offset
+ = extract_signed_integer (value_contents (value),
+ TYPE_LENGTH (type), byte_order);
+ byte_offset += piece->v.ptr.offset;
+
+ return indirect_synthetic_pointer (piece->v.ptr.die_sect_off,
+ byte_offset, c->per_cu,
+ c->per_objfile, frame, type);
+}
+
+/* Implementation of the coerce_ref method of lval_funcs for synthetic C++
+ references. */
+
+static struct value *
+coerce_pieced_ref (const struct value *value)
+{
+ struct type *type = check_typedef (value_type (value));
+
+ if (value_bits_synthetic_pointer (value, value_embedded_offset (value),
+ TARGET_CHAR_BIT * TYPE_LENGTH (type)))
+ {
+ const struct piece_closure *closure
+ = (struct piece_closure *) value_computed_closure (value);
+ struct frame_info *frame
+ = get_selected_frame (_("No frame selected."));
+
+ /* gdb represents synthetic pointers as pieced values with a single
+ piece. */
+ gdb_assert (closure != NULL);
+ gdb_assert (closure->pieces.size () == 1);
+
+ return indirect_synthetic_pointer
+ (closure->pieces[0].v.ptr.die_sect_off,
+ closure->pieces[0].v.ptr.offset,
+ closure->per_cu, closure->per_objfile, frame, type);
+ }
+ else
+ {
+ /* Else: not a synthetic reference; do nothing. */
+ return NULL;
+ }
+}
+
+static void *
+copy_pieced_value_closure (const struct value *v)
+{
+ struct piece_closure *c
+ = (struct piece_closure *) value_computed_closure (v);
+
+ ++c->refc;
+ return c;
+}
+
+static void
+free_pieced_value_closure (struct value *v)
+{
+ struct piece_closure *c
+ = (struct piece_closure *) value_computed_closure (v);
+
+ --c->refc;
+ if (c->refc == 0)
+ {
+ for (dwarf_expr_piece &p : c->pieces)
+ if (p.location == DWARF_VALUE_STACK)
+ value_decref (p.v.value);
+
+ delete c;
+ }
+}
+
+/* Functions for accessing a variable described by DW_OP_piece. */
+const struct lval_funcs pieced_value_funcs = {
+ read_pieced_value,
+ write_pieced_value,
+ indirect_pieced_value,
+ coerce_pieced_ref,
+ check_pieced_synthetic_pointer,
+ copy_pieced_value_closure,
+ free_pieced_value_closure
+};
+
+/* Given context CTX, section offset SECT_OFF, and compilation unit
+ data PER_CU, execute the "variable value" operation on the DIE
+ found at SECT_OFF. */
+
+static struct value *
+sect_variable_value (sect_offset sect_off,
+ dwarf2_per_cu_data *per_cu,
+ dwarf2_per_objfile *per_objfile)
+{
+ struct type *die_type
+ = dwarf2_fetch_die_type_sect_off (sect_off, per_cu, per_objfile);
+
+ if (die_type == NULL)
+ error (_("Bad DW_OP_GNU_variable_value DIE."));
+
+ /* Note: Things still work when the following test is removed. This
+ test and error is here to conform to the proposed specification. */
+ if (die_type->code () != TYPE_CODE_INT
+ && die_type->code () != TYPE_CODE_PTR)
+ error (_("Type of DW_OP_GNU_variable_value DIE must be an integer or pointer."));
+
+ struct type *type = lookup_pointer_type (die_type);
+ struct frame_info *frame = get_selected_frame (_("No frame selected."));
+ return indirect_synthetic_pointer (sect_off, 0, per_cu, per_objfile, frame,
+ type, true);
+}
+
/* Return the type used for DWARF operations where the type is
unspecified in the DWARF spec. Only certain sizes are
supported. */
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index 35852183fc5..07d12305be9 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -26,6 +26,7 @@
#include "gdbtypes.h"
struct dwarf2_per_objfile;
+struct piece_closure;
/* The location of a value. */
enum dwarf_value_location
@@ -300,4 +301,13 @@ 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/loc.c b/gdb/dwarf2/loc.c
index ef25f3c5e86..9514d5ed668 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -52,13 +52,6 @@ static struct value *dwarf2_evaluate_loc_desc_full
size_t size, dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
struct type *subobj_type, LONGEST subobj_byte_offset);
-static struct value *indirect_synthetic_pointer
- (sect_offset die, LONGEST byte_offset,
- dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile,
- struct frame_info *frame,
- struct type *type, bool resolve_abstract_p = false);
-
/* Until these have formal names, we define these here.
ref: http://gcc.gnu.org/wiki/DebugFission
Each entry in .debug_loc.dwo begins with a byte that describes the entry,
@@ -593,31 +586,6 @@ func_get_frame_base_dwarf_block (struct symbol *framefunc, CORE_ADDR pc,
framefunc->natural_name ());
}
-/* See loc.h. */
-
-struct value *
-sect_variable_value (sect_offset sect_off,
- dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile)
-{
- struct type *die_type
- = dwarf2_fetch_die_type_sect_off (sect_off, per_cu, per_objfile);
-
- if (die_type == NULL)
- error (_("Bad DW_OP_GNU_variable_value DIE."));
-
- /* Note: Things still work when the following test is removed. This
- test and error is here to conform to the proposed specification. */
- if (die_type->code () != TYPE_CODE_INT
- && die_type->code () != TYPE_CODE_PTR)
- error (_("Type of DW_OP_GNU_variable_value DIE must be an integer or pointer."));
-
- struct type *type = lookup_pointer_type (die_type);
- struct frame_info *frame = get_selected_frame (_("No frame selected."));
- return indirect_synthetic_pointer (sect_off, 0, per_cu, per_objfile, frame,
- type, true);
-}
-
/* See dwarf2loc.h. */
unsigned int entry_values_debug = 0;
@@ -1377,403 +1345,6 @@ value_of_dwarf_block_entry (struct type *type, struct frame_info *frame,
"only for single DW_OP_reg* or for DW_OP_fbreg(*)"));
}
-struct piece_closure
-{
- /* Reference count. */
- int refc = 0;
-
- /* The objfile from which this closure's expression came. */
- dwarf2_per_objfile *per_objfile = nullptr;
-
- /* The CU from which this closure's expression came. */
- struct dwarf2_per_cu_data *per_cu = NULL;
-
- /* The pieces describing this variable. */
- std::vector<dwarf_expr_piece> pieces;
-
- /* Frame ID of frame to which a register value is relative, used
- only by DWARF_VALUE_REGISTER. */
- struct frame_id frame_id;
-};
-
-/* Allocate a closure for a value formed from separately-described
- PIECES. */
-
-static 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)
-{
- struct piece_closure *c = new piece_closure;
-
- c->refc = 1;
- /* We must capture this here due to sharing of DWARF state. */
- c->per_objfile = per_objfile;
- c->per_cu = per_cu;
- c->pieces = std::move (pieces);
- if (frame == NULL)
- c->frame_id = null_frame_id;
- else
- c->frame_id = get_frame_id (frame);
-
- for (dwarf_expr_piece &piece : c->pieces)
- if (piece.location == DWARF_VALUE_STACK)
- value_incref (piece.v.value);
-
- return c;
-}
-
-/* Return the number of bytes overlapping a contiguous chunk of N_BITS
- bits whose first bit is located at bit offset START. */
-
-static size_t
-bits_to_bytes (ULONGEST start, ULONGEST n_bits)
-{
- return (start % 8 + n_bits + 7) / 8;
-}
-
-/* Read or write a pieced value V. If FROM != NULL, operate in "write
- mode": copy FROM into the pieces comprising V. If FROM == NULL,
- operate in "read mode": fetch the contents of the (lazy) value V by
- composing it from its pieces. */
-
-static void
-rw_pieced_value (struct value *v, struct value *from)
-{
- int i;
- LONGEST offset = 0, max_offset;
- ULONGEST bits_to_skip;
- gdb_byte *v_contents;
- const gdb_byte *from_contents;
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (v);
- gdb::byte_vector buffer;
- bool bits_big_endian = type_byte_order (value_type (v)) == BFD_ENDIAN_BIG;
-
- if (from != NULL)
- {
- from_contents = value_contents (from);
- v_contents = NULL;
- }
- else
- {
- if (value_type (v) != value_enclosing_type (v))
- internal_error (__FILE__, __LINE__,
- _("Should not be able to create a lazy value with "
- "an enclosing type"));
- v_contents = value_contents_raw (v);
- from_contents = NULL;
- }
-
- bits_to_skip = 8 * value_offset (v);
- if (value_bitsize (v))
- {
- bits_to_skip += (8 * value_offset (value_parent (v))
- + value_bitpos (v));
- if (from != NULL
- && (type_byte_order (value_type (from))
- == BFD_ENDIAN_BIG))
- {
- /* Use the least significant bits of FROM. */
- max_offset = 8 * TYPE_LENGTH (value_type (from));
- offset = max_offset - value_bitsize (v);
- }
- else
- max_offset = value_bitsize (v);
- }
- else
- max_offset = 8 * TYPE_LENGTH (value_type (v));
-
- /* Advance to the first non-skipped piece. */
- for (i = 0; i < c->pieces.size () && bits_to_skip >= c->pieces[i].size; i++)
- bits_to_skip -= c->pieces[i].size;
-
- for (; i < c->pieces.size () && offset < max_offset; i++)
- {
- struct dwarf_expr_piece *p = &c->pieces[i];
- size_t this_size_bits, this_size;
-
- this_size_bits = p->size - bits_to_skip;
- if (this_size_bits > max_offset - offset)
- this_size_bits = max_offset - offset;
-
- switch (p->location)
- {
- case DWARF_VALUE_REGISTER:
- {
- struct frame_info *frame = frame_find_by_id (c->frame_id);
- struct gdbarch *arch = get_frame_arch (frame);
- int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
- ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
- int optim, unavail;
-
- if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && p->offset + p->size < reg_bits)
- {
- /* Big-endian, and we want less than full size. */
- bits_to_skip += reg_bits - (p->offset + p->size);
- }
- else
- bits_to_skip += p->offset;
-
- this_size = bits_to_bytes (bits_to_skip, this_size_bits);
- buffer.resize (this_size);
-
- if (from == NULL)
- {
- /* Read mode. */
- if (!get_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- buffer,
- &optim, &unavail))
- {
- if (optim)
- mark_value_bits_optimized_out (v, offset,
- this_size_bits);
- if (unavail)
- mark_value_bits_unavailable (v, offset,
- this_size_bits);
- break;
- }
-
- copy_bitwise (v_contents, offset,
- buffer.data (), bits_to_skip % 8,
- this_size_bits, bits_big_endian);
- }
- else
- {
- /* Write mode. */
- if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
- {
- /* Data is copied non-byte-aligned into the register.
- Need some bits from original register value. */
- get_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- buffer,
- &optim, &unavail);
- if (optim)
- throw_error (OPTIMIZED_OUT_ERROR,
- _("Can't do read-modify-write to "
- "update bitfield; containing word "
- "has been optimized out"));
- if (unavail)
- throw_error (NOT_AVAILABLE_ERROR,
- _("Can't do read-modify-write to "
- "update bitfield; containing word "
- "is unavailable"));
- }
-
- copy_bitwise (buffer.data (), bits_to_skip % 8,
- from_contents, offset,
- this_size_bits, bits_big_endian);
- put_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- buffer);
- }
- }
- break;
-
- case DWARF_VALUE_MEMORY:
- {
- bits_to_skip += p->offset;
-
- CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
-
- if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
- && offset % 8 == 0)
- {
- /* Everything is byte-aligned; no buffer needed. */
- if (from != NULL)
- write_memory_with_notification (start_addr,
- (from_contents
- + offset / 8),
- this_size_bits / 8);
- else
- read_value_memory (v, offset,
- p->v.mem.in_stack_memory,
- p->v.mem.addr + bits_to_skip / 8,
- v_contents + offset / 8,
- this_size_bits / 8);
- break;
- }
-
- this_size = bits_to_bytes (bits_to_skip, this_size_bits);
- buffer.resize (this_size);
-
- if (from == NULL)
- {
- /* Read mode. */
- read_value_memory (v, offset,
- p->v.mem.in_stack_memory,
- p->v.mem.addr + bits_to_skip / 8,
- buffer.data (), this_size);
- copy_bitwise (v_contents, offset,
- buffer.data (), bits_to_skip % 8,
- this_size_bits, bits_big_endian);
- }
- else
- {
- /* Write mode. */
- if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
- {
- if (this_size <= 8)
- {
- /* Perform a single read for small sizes. */
- read_memory (start_addr, buffer.data (),
- this_size);
- }
- else
- {
- /* Only the first and last bytes can possibly have
- any bits reused. */
- read_memory (start_addr, buffer.data (), 1);
- read_memory (start_addr + this_size - 1,
- &buffer[this_size - 1], 1);
- }
- }
-
- copy_bitwise (buffer.data (), bits_to_skip % 8,
- from_contents, offset,
- this_size_bits, bits_big_endian);
- write_memory_with_notification (start_addr,
- buffer.data (),
- this_size);
- }
- }
- break;
-
- case DWARF_VALUE_STACK:
- {
- if (from != NULL)
- {
- mark_value_bits_optimized_out (v, offset, this_size_bits);
- break;
- }
-
- gdbarch *objfile_gdbarch = c->per_objfile->objfile->arch ();
- ULONGEST stack_value_size_bits
- = 8 * TYPE_LENGTH (value_type (p->v.value));
-
- /* Use zeroes if piece reaches beyond stack value. */
- if (p->offset + p->size > stack_value_size_bits)
- break;
-
- /* Piece is anchored at least significant bit end. */
- if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
- bits_to_skip += stack_value_size_bits - p->offset - p->size;
- else
- bits_to_skip += p->offset;
-
- copy_bitwise (v_contents, offset,
- value_contents_all (p->v.value),
- bits_to_skip,
- this_size_bits, bits_big_endian);
- }
- break;
-
- case DWARF_VALUE_LITERAL:
- {
- if (from != NULL)
- {
- mark_value_bits_optimized_out (v, offset, this_size_bits);
- break;
- }
-
- ULONGEST literal_size_bits = 8 * p->v.literal.length;
- size_t n = this_size_bits;
-
- /* Cut off at the end of the implicit value. */
- bits_to_skip += p->offset;
- if (bits_to_skip >= literal_size_bits)
- break;
- if (n > literal_size_bits - bits_to_skip)
- n = literal_size_bits - bits_to_skip;
-
- copy_bitwise (v_contents, offset,
- p->v.literal.data, bits_to_skip,
- n, bits_big_endian);
- }
- break;
-
- case DWARF_VALUE_IMPLICIT_POINTER:
- if (from != NULL)
- {
- mark_value_bits_optimized_out (v, offset, this_size_bits);
- break;
- }
-
- /* These bits show up as zeros -- but do not cause the value to
- be considered optimized-out. */
- break;
-
- case DWARF_VALUE_OPTIMIZED_OUT:
- mark_value_bits_optimized_out (v, offset, this_size_bits);
- break;
-
- default:
- internal_error (__FILE__, __LINE__, _("invalid location type"));
- }
-
- offset += this_size_bits;
- bits_to_skip = 0;
- }
-}
-
-
-static void
-read_pieced_value (struct value *v)
-{
- rw_pieced_value (v, NULL);
-}
-
-static void
-write_pieced_value (struct value *to, struct value *from)
-{
- rw_pieced_value (to, from);
-}
-
-/* An implementation of an lval_funcs method to see whether a value is
- a synthetic pointer. */
-
-static int
-check_pieced_synthetic_pointer (const struct value *value, LONGEST bit_offset,
- int bit_length)
-{
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (value);
- int i;
-
- bit_offset += 8 * value_offset (value);
- if (value_bitsize (value))
- bit_offset += value_bitpos (value);
-
- for (i = 0; i < c->pieces.size () && bit_length > 0; i++)
- {
- struct dwarf_expr_piece *p = &c->pieces[i];
- size_t this_size_bits = p->size;
-
- if (bit_offset > 0)
- {
- if (bit_offset >= this_size_bits)
- {
- bit_offset -= this_size_bits;
- continue;
- }
-
- bit_length -= this_size_bits - bit_offset;
- bit_offset = 0;
- }
- else
- bit_length -= this_size_bits;
-
- if (p->location != DWARF_VALUE_IMPLICIT_POINTER)
- return 0;
- }
-
- return 1;
-}
-
/* Fetch a DW_AT_const_value through a synthetic pointer. */
static struct value *
@@ -1807,9 +1378,9 @@ fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset,
return result;
}
-/* Fetch the value pointed to by a synthetic pointer. */
+/* See loc.h. */
-static struct value *
+struct value *
indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
dwarf2_per_cu_data *per_cu,
dwarf2_per_objfile *per_objfile,
@@ -1846,154 +1417,6 @@ indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
per_objfile, type);
}
-/* An implementation of an lval_funcs method to indirect through a
- pointer. This handles the synthetic pointer case when needed. */
-
-static struct value *
-indirect_pieced_value (struct value *value)
-{
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (value);
- struct type *type;
- struct frame_info *frame;
- int i, bit_length;
- LONGEST bit_offset;
- struct dwarf_expr_piece *piece = NULL;
- LONGEST byte_offset;
- enum bfd_endian byte_order;
-
- type = check_typedef (value_type (value));
- if (type->code () != TYPE_CODE_PTR)
- return NULL;
-
- bit_length = 8 * TYPE_LENGTH (type);
- bit_offset = 8 * value_offset (value);
- if (value_bitsize (value))
- bit_offset += value_bitpos (value);
-
- for (i = 0; i < c->pieces.size () && bit_length > 0; i++)
- {
- struct dwarf_expr_piece *p = &c->pieces[i];
- size_t this_size_bits = p->size;
-
- if (bit_offset > 0)
- {
- if (bit_offset >= this_size_bits)
- {
- bit_offset -= this_size_bits;
- continue;
- }
-
- bit_length -= this_size_bits - bit_offset;
- bit_offset = 0;
- }
- else
- bit_length -= this_size_bits;
-
- if (p->location != DWARF_VALUE_IMPLICIT_POINTER)
- return NULL;
-
- if (bit_length != 0)
- error (_("Invalid use of DW_OP_implicit_pointer"));
-
- piece = p;
- break;
- }
-
- gdb_assert (piece != NULL);
- frame = get_selected_frame (_("No frame selected."));
-
- /* This is an offset requested by GDB, such as value subscripts.
- However, due to how synthetic pointers are implemented, this is
- always presented to us as a pointer type. This means we have to
- sign-extend it manually as appropriate. Use raw
- extract_signed_integer directly rather than value_as_address and
- sign extend afterwards on architectures that would need it
- (mostly everywhere except MIPS, which has signed addresses) as
- the later would go through gdbarch_pointer_to_address and thus
- return a CORE_ADDR with high bits set on architectures that
- encode address spaces and other things in CORE_ADDR. */
- byte_order = gdbarch_byte_order (get_frame_arch (frame));
- byte_offset = extract_signed_integer (value_contents (value),
- TYPE_LENGTH (type), byte_order);
- byte_offset += piece->v.ptr.offset;
-
- return indirect_synthetic_pointer (piece->v.ptr.die_sect_off,
- byte_offset, c->per_cu,
- c->per_objfile, frame, type);
-}
-
-/* Implementation of the coerce_ref method of lval_funcs for synthetic C++
- references. */
-
-static struct value *
-coerce_pieced_ref (const struct value *value)
-{
- struct type *type = check_typedef (value_type (value));
-
- if (value_bits_synthetic_pointer (value, value_embedded_offset (value),
- TARGET_CHAR_BIT * TYPE_LENGTH (type)))
- {
- const struct piece_closure *closure
- = (struct piece_closure *) value_computed_closure (value);
- struct frame_info *frame
- = get_selected_frame (_("No frame selected."));
-
- /* gdb represents synthetic pointers as pieced values with a single
- piece. */
- gdb_assert (closure != NULL);
- gdb_assert (closure->pieces.size () == 1);
-
- return indirect_synthetic_pointer
- (closure->pieces[0].v.ptr.die_sect_off,
- closure->pieces[0].v.ptr.offset,
- closure->per_cu, closure->per_objfile, frame, type);
- }
- else
- {
- /* Else: not a synthetic reference; do nothing. */
- return NULL;
- }
-}
-
-static void *
-copy_pieced_value_closure (const struct value *v)
-{
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (v);
-
- ++c->refc;
- return c;
-}
-
-static void
-free_pieced_value_closure (struct value *v)
-{
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (v);
-
- --c->refc;
- if (c->refc == 0)
- {
- for (dwarf_expr_piece &p : c->pieces)
- if (p.location == DWARF_VALUE_STACK)
- value_decref (p.v.value);
-
- delete c;
- }
-}
-
-/* Functions for accessing a variable described by DW_OP_piece. */
-static const struct lval_funcs pieced_value_funcs = {
- read_pieced_value,
- write_pieced_value,
- indirect_pieced_value,
- coerce_pieced_ref,
- check_pieced_synthetic_pointer,
- copy_pieced_value_closure,
- free_pieced_value_closure
-};
-
/* Evaluate a location description, starting at DATA and with length
SIZE, to find the current location of variable of TYPE in the
context of FRAME. If SUBOBJ_TYPE is non-NULL, return instead the
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index e79bce6eb75..598dbcd4fbc 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -53,14 +53,6 @@ extern void func_get_frame_base_dwarf_block (struct symbol *framefunc,
const gdb_byte **start,
size_t *length);
-/* Given section offset SECT_OFF, and compilation unit data
- PER_CU, execute the "variable value" operation on the DIE
- found at SECT_OFF. */
-
-struct value *sect_variable_value (sect_offset sect_off,
- dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile);
-
/* Fetch call_site_parameter from caller matching KIND and KIND_U.
FRAME is for callee.
@@ -284,4 +276,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);
+/* Fetch the value pointed to by a synthetic pointer. */
+
+extern struct value *indirect_synthetic_pointer
+ (sect_offset die, LONGEST byte_offset, dwarf2_per_cu_data *per_cu,
+ dwarf2_per_objfile *per_objfile, struct frame_info *frame,
+ struct type *type, bool resolve_abstract_p = false);
+
#endif /* dwarf2loc.h */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 14/17] Make value_copy also copy the stack data member
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (12 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 13/17] Move piece_closure and its support to expr.c Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 15/17] Make DWARF evaluator return a single struct value Zoran Zaric
` (3 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
Fixing a bug where the value_copy function did not copy the stack data
and initialized members of the struct value. This is needed for the
next patch where the DWARF expression evaluator is changed to return a
single struct value object.
* value.c (value_copy): Change to also copy the stack data
and initialized members.
---
gdb/value.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gdb/value.c b/gdb/value.c
index 9822cec209b..dd18a0509f1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1692,6 +1692,8 @@ value_copy (struct value *arg)
val->embedded_offset = value_embedded_offset (arg);
val->pointed_to_offset = arg->pointed_to_offset;
val->modifiable = arg->modifiable;
+ val->stack = arg->stack;
+ val->initialized = arg->initialized;
if (!value_lazy (val))
{
memcpy (value_contents_all_raw (val), value_contents_all_raw (arg),
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (13 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 14/17] Make value_copy also copy the stack data member Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-08-11 17:00 ` Tom Tromey
2021-05-28 15:46 ` [PATCH v3 16/17] Simplify dwarf_expr_context class interface Zoran Zaric
` (2 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
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 | 160 ++++++++++++++++++++++++++++++++++++-
gdb/dwarf2/expr.h | 23 +++---
gdb/dwarf2/frame.c | 19 ++---
gdb/dwarf2/loc.c | 195 ++++++---------------------------------------
gdb/dwarf2/loc.h | 5 ++
5 files changed, 204 insertions(+), 198 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 022a97f1a07..bc74fc1e582 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,
@@ -623,7 +624,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,
@@ -879,6 +880,159 @@ 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)
+ {
+ 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 ();
+
+ piece_closure *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))
+ {
+ /* 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>. */
+ struct value *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:
+ {
+ size_t n = TYPE_LENGTH (subobj_type);
+
+ if (subobj_offset + n > this->len)
+ invalid_synthetic_pointer ();
+
+ retval = allocate_value (subobj_type);
+ bfd_byte *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 07d12305be9..c2138b31d79 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;
@@ -203,6 +206,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,13 +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 78dbb489506..8aaa0af7e91 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -229,8 +229,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 9514d5ed668..6584a1615f7 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -92,7 +92,7 @@ enum debug_loc_kind
/* Helper function which throws an error if a synthetic pointer is
invalid. */
-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)
@@ -1685,6 +1550,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)
{
@@ -1702,29 +1568,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 598dbcd4fbc..06b25fbb78e 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -276,6 +276,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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 16/17] Simplify dwarf_expr_context class interface
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (14 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 15/17] Make DWARF evaluator return a single struct value Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-06-09 1:01 ` Simon Marchi
2021-05-28 15:46 ` [PATCH v3 17/17] Add as_lval argument to expression evaluator Zoran Zaric
2021-06-09 1:04 ` [PATCH v3 00/17] DWARF expression evaluator design cleanup Simon Marchi
17 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
Idea of this patch is to get a clean and simple public interface for
the dwarf_expr_context class, looking like:
- constructor,
- destructor,
- push_address method and
- evaluate method.
Where constructor should only ever require a target architecture
information. This information is held in per object file
(dwarf2_per_objfile) structure, so it makes sense to keep that
structure as a constructor argument. It also makes sense to get the
address size from that structure, but unfortunately that interface
doesn’t exist at the moment, so the dwarf_expr_context class user
needs to provide that information.
The push_address method is used to push a CORE_ADDR as a value on
top of the DWARF stack before the evaluation. This method can be
later changed to push any struct value object on the stack.
The evaluate method is the method that evaluates a DWARF expression
and provides the evaluation result, in a form of a single struct
value object that describes a location. To do this, the method requires
a context of the evaluation, as well as expected result type
information. If the type information is not provided, the DWARF generic
type will be used instead.
To avoid storing the gdbarch information in the evaluator object, that
information is now always acquired from the per_objfile object.
All data members are now private and only visible to the evaluator
class, so a m_ prefix was added to all of their names to reflect that.
To make this distinction clear, they are also accessed through objects
this pointer, wherever that was not the case before.
gdb/ChangeLog:
* dwarf2/expr.c (dwarf_expr_context::dwarf_expr_context): Add
address size argument.
(dwarf_expr_context::read_mem): Change to use property_addr_info
structure.
(dwarf_expr_context::evaluate): New function.
(dwarf_expr_context::execute_stack_op): Change to use
property_addr_info structure.
* dwarf2/expr.h (struct dwarf_expr_context): New evaluate
declaration. Change eval and fetch_result method to private.
(dwarf_expr_context::gdbarch): Remove member.
(dwarf_expr_context::stack): Make private and add m_ prefix.
(dwarf_expr_context::addr_size): Make private and add
m_ prefix.
(dwarf_expr_context::recursion_depth): Make private and add
m_ prefix.
(dwarf_expr_context::max_recursion_depth): Make private and
add m_ prefix.
(dwarf_expr_context::len): Make private and add m_ prefix.
(dwarf_expr_context::data): Make private and add m_ prefix.
(dwarf_expr_context::initialized): Make private and add
m_ prefix.
(dwarf_expr_context::pieces): Make private and add m_ prefix.
(dwarf_expr_context::per_objfile): Make private and add
m_ prefix.
(dwarf_expr_context::frame): Make private and add m_ prefix.
(dwarf_expr_context::per_cu): Make private and add m_ prefix.
(dwarf_expr_context::addr_info): Make private and add
m_ prefix.
* dwarf2/frame.c (execute_stack_op): Change to call evaluate
method.
* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to call
evaluate method.
(dwarf2_locexpr_baton_eval): Change to call evaluate method.
---
gdb/dwarf2/expr.c | 336 ++++++++++++++++++++++++---------------------
gdb/dwarf2/expr.h | 65 +++++----
gdb/dwarf2/frame.c | 17 +--
gdb/dwarf2/loc.c | 39 ++----
4 files changed, 234 insertions(+), 223 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index bc74fc1e582..30b0cef81d2 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -668,25 +668,24 @@ sect_variable_value (sect_offset sect_off,
struct type *
dwarf_expr_context::address_type () const
{
+ struct gdbarch *gdbarch = this->m_per_objfile->objfile->arch ();
struct dwarf_gdbarch_types *types
- = (struct dwarf_gdbarch_types *) gdbarch_data (this->gdbarch,
- dwarf_arch_cookie);
+ = (struct dwarf_gdbarch_types *) gdbarch_data (gdbarch, dwarf_arch_cookie);
int ndx;
- if (this->addr_size == 2)
+ if (this->m_addr_size == 2)
ndx = 0;
- else if (this->addr_size == 4)
+ else if (this->m_addr_size == 4)
ndx = 1;
- else if (this->addr_size == 8)
+ else if (this->m_addr_size == 8)
ndx = 2;
else
error (_("Unsupported address size in DWARF expressions: %d bits"),
- 8 * this->addr_size);
+ 8 * this->m_addr_size);
if (types->dw_types[ndx] == NULL)
types->dw_types[ndx]
- = arch_integer_type (this->gdbarch,
- 8 * this->addr_size,
+ = arch_integer_type (gdbarch, 8 * this->m_addr_size,
0, "<signed DWARF address type>");
return types->dw_types[ndx];
@@ -694,8 +693,10 @@ dwarf_expr_context::address_type () const
/* Create a new context for the expression evaluator. */
-dwarf_expr_context::dwarf_expr_context (dwarf2_per_objfile *per_objfile)
-: per_objfile (per_objfile)
+dwarf_expr_context::dwarf_expr_context (dwarf2_per_objfile *per_objfile,
+ int addr_size)
+: m_addr_size (addr_size),
+ m_per_objfile (per_objfile)
{
}
@@ -704,7 +705,7 @@ dwarf_expr_context::dwarf_expr_context (dwarf2_per_objfile *per_objfile)
void
dwarf_expr_context::push (struct value *value, bool in_stack_memory)
{
- stack.emplace_back (value, in_stack_memory);
+ this->m_stack.emplace_back (value, in_stack_memory);
}
/* Push VALUE onto the stack. */
@@ -720,10 +721,10 @@ dwarf_expr_context::push_address (CORE_ADDR value, bool in_stack_memory)
void
dwarf_expr_context::pop ()
{
- if (stack.empty ())
+ if (this->m_stack.empty ())
error (_("dwarf expression stack underflow"));
- stack.pop_back ();
+ this->m_stack.pop_back ();
}
/* Retrieve the N'th item on the stack. */
@@ -731,11 +732,11 @@ dwarf_expr_context::pop ()
struct value *
dwarf_expr_context::fetch (int n)
{
- if (stack.size () <= n)
+ if (this->m_stack.size () <= n)
error (_("Asked for position %d of stack, "
"stack only has %zu elements on it."),
- n, stack.size ());
- return stack[stack.size () - (1 + n)].value;
+ n, this->m_stack.size ());
+ return this->m_stack[this->m_stack.size () - (1 + n)].value;
}
/* See expr.h. */
@@ -744,9 +745,9 @@ void
dwarf_expr_context::get_frame_base (const gdb_byte **start,
size_t * length)
{
- ensure_have_frame (this->frame, "DW_OP_fbreg");
+ ensure_have_frame (this->m_frame, "DW_OP_fbreg");
- const struct block *bl = get_frame_block (this->frame, NULL);
+ const struct block *bl = get_frame_block (this->m_frame, NULL);
if (bl == NULL)
error (_("frame address is not available."));
@@ -762,7 +763,7 @@ dwarf_expr_context::get_frame_base (const gdb_byte **start,
gdb_assert (framefunc != NULL);
func_get_frame_base_dwarf_block (framefunc,
- get_frame_address_in_block (this->frame),
+ get_frame_address_in_block (this->m_frame),
start, length);
}
@@ -771,11 +772,11 @@ dwarf_expr_context::get_frame_base (const gdb_byte **start,
struct type *
dwarf_expr_context::get_base_type (cu_offset die_cu_off)
{
- if (per_cu == nullptr)
- return builtin_type (this->gdbarch)->builtin_int;
+ if (this->m_per_cu == nullptr)
+ return builtin_type (this->m_per_objfile->objfile->arch ())->builtin_int;
- struct type *result = dwarf2_get_die_type (die_cu_off, this->per_cu,
- this->per_objfile);
+ struct type *result = dwarf2_get_die_type (die_cu_off, this->m_per_cu,
+ this->m_per_objfile);
if (result == nullptr)
error (_("Could not find type for operation"));
@@ -788,9 +789,9 @@ dwarf_expr_context::get_base_type (cu_offset die_cu_off)
void
dwarf_expr_context::dwarf_call (cu_offset die_cu_off)
{
- ensure_have_per_cu (this->per_cu, "DW_OP_call");
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_call");
- struct frame_info *frame = this->frame;
+ struct frame_info *frame = this->m_frame;
auto get_pc_from_frame = [frame] ()
{
@@ -799,11 +800,11 @@ dwarf_expr_context::dwarf_call (cu_offset die_cu_off)
};
struct dwarf2_locexpr_baton block
- = dwarf2_fetch_die_loc_cu_off (die_cu_off, this->per_cu, this->per_objfile,
- get_pc_from_frame);
+ = dwarf2_fetch_die_loc_cu_off (die_cu_off, this->m_per_cu,
+ this->m_per_objfile, get_pc_from_frame);
/* DW_OP_call_ref is currently not supported. */
- gdb_assert (block.per_cu == this->per_cu);
+ gdb_assert (block.per_cu == this->m_per_cu);
this->eval (block.data, block.size);
}
@@ -818,13 +819,16 @@ dwarf_expr_context::read_mem (gdb_byte *buf, CORE_ADDR addr,
return;
/* Prefer the passed-in memory, if it exists. */
- CORE_ADDR offset = addr - this->obj_address;
-
- if (offset < this->data_view.size ()
- && offset + length <= this->data_view.size ())
+ if (this->m_addr_info != nullptr)
{
- memcpy (buf, this->data_view.data (), length);
- return;
+ CORE_ADDR offset = addr - this->m_addr_info->addr;
+
+ if (offset < this->m_addr_info->valaddr.size ()
+ && offset + length <= this->m_addr_info->valaddr.size ())
+ {
+ memcpy (buf, this->m_addr_info->valaddr.data (), length);
+ return;
+ }
}
read_memory (addr, buf, length);
@@ -837,14 +841,14 @@ dwarf_expr_context::push_dwarf_reg_entry_value
(enum call_site_parameter_kind kind, union call_site_parameter_u kind_u,
int deref_size)
{
- ensure_have_per_cu (this->per_cu, "DW_OP_entry_value");
- ensure_have_frame (this->frame, "DW_OP_entry_value");
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_entry_value");
+ ensure_have_frame (this->m_frame, "DW_OP_entry_value");
dwarf2_per_cu_data *caller_per_cu;
dwarf2_per_objfile *caller_per_objfile;
- struct frame_info *caller_frame = get_prev_frame (this->frame);
+ struct frame_info *caller_frame = get_prev_frame (this->m_frame);
struct call_site_parameter *parameter
- = dwarf_expr_reg_to_entry_parameter (this->frame, kind, kind_u,
+ = dwarf_expr_reg_to_entry_parameter (this->m_frame, kind, kind_u,
&caller_per_cu,
&caller_per_objfile);
const gdb_byte *data_src
@@ -863,19 +867,17 @@ dwarf_expr_context::push_dwarf_reg_entry_value
It is possible for the caller to be from a different objfile from the
callee if the call is made through a function pointer. */
- scoped_restore save_frame = make_scoped_restore (&this->frame,
+ scoped_restore save_frame = make_scoped_restore (&this->m_frame,
caller_frame);
- scoped_restore save_per_cu = make_scoped_restore (&this->per_cu,
+ scoped_restore save_per_cu = make_scoped_restore (&this->m_per_cu,
caller_per_cu);
- scoped_restore save_obj_addr = make_scoped_restore (&this->obj_address,
- (CORE_ADDR) 0);
- scoped_restore save_per_objfile = make_scoped_restore (&this->per_objfile,
+ scoped_restore save_addr_info = make_scoped_restore (&this->m_addr_info,
+ nullptr);
+ scoped_restore save_per_objfile = make_scoped_restore (&this->m_per_objfile,
caller_per_objfile);
- scoped_restore save_arch = make_scoped_restore (&this->gdbarch);
- this->gdbarch = this->per_objfile->objfile->arch ();
- scoped_restore save_addr_size = make_scoped_restore (&this->addr_size);
- this->addr_size = this->per_cu->addr_size ();
+ scoped_restore save_addr_size = make_scoped_restore (&this->m_addr_size);
+ this->m_addr_size = this->m_per_cu->addr_size ();
this->eval (data_src, size);
}
@@ -888,6 +890,7 @@ dwarf_expr_context::fetch_result (struct type *type,
LONGEST subobj_offset)
{
struct value *retval = nullptr;
+ struct gdbarch *gdbarch = this->m_per_objfile->objfile->arch ();
if (type == nullptr)
type = address_type ();
@@ -895,11 +898,11 @@ dwarf_expr_context::fetch_result (struct type *type,
if (subobj_type == nullptr)
subobj_type = type;
- if (this->pieces.size () > 0)
+ if (this->m_pieces.size () > 0)
{
ULONGEST bit_size = 0;
- for (dwarf_expr_piece &piece : this->pieces)
+ for (dwarf_expr_piece &piece : this->m_pieces)
bit_size += piece.size;
/* Complain if the expression is larger than the size of the
outer type. */
@@ -907,30 +910,30 @@ dwarf_expr_context::fetch_result (struct type *type,
invalid_synthetic_pointer ();
piece_closure *c
- = allocate_piece_closure (this->per_cu, this->per_objfile,
- std::move (this->pieces), this->frame);
+ = allocate_piece_closure (this->m_per_cu, this->m_per_objfile,
+ std::move (this->m_pieces), this->m_frame);
retval = allocate_computed_value (subobj_type,
&pieced_value_funcs, c);
set_value_offset (retval, subobj_offset);
}
else
{
- switch (this->location)
+ switch (this->m_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,
+ int gdb_regnum = dwarf_reg_to_regnum_or_error (gdbarch,
dwarf_regnum);
if (subobj_offset != 0)
error (_("cannot use offset on synthetic pointer to register"));
- gdb_assert (this->frame != NULL);
+ gdb_assert (this->m_frame != NULL);
retval = value_from_register (subobj_type, gdb_regnum,
- this->frame);
+ this->m_frame);
if (value_optimized_out (retval))
{
/* This means the register has undefined value / was
@@ -965,10 +968,10 @@ dwarf_expr_context::fetch_result (struct type *type,
{
case TYPE_CODE_FUNC:
case TYPE_CODE_METHOD:
- ptr_type = builtin_type (this->gdbarch)->builtin_func_ptr;
+ ptr_type = builtin_type (gdbarch)->builtin_func_ptr;
break;
default:
- ptr_type = builtin_type (this->gdbarch)->builtin_data_ptr;
+ ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
break;
}
address = value_as_address (value_from_pointer (ptr_type, address));
@@ -993,7 +996,7 @@ dwarf_expr_context::fetch_result (struct type *type,
retval = allocate_value (subobj_type);
/* The given offset is relative to the actual object. */
- if (gdbarch_byte_order (this->gdbarch) == BFD_ENDIAN_BIG)
+ if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
subobj_offset += n - max;
memcpy (value_contents_raw (retval),
@@ -1005,12 +1008,12 @@ dwarf_expr_context::fetch_result (struct type *type,
{
size_t n = TYPE_LENGTH (subobj_type);
- if (subobj_offset + n > this->len)
+ if (subobj_offset + n > this->m_len)
invalid_synthetic_pointer ();
retval = allocate_value (subobj_type);
bfd_byte *contents = value_contents_raw (retval);
- memcpy (contents, this->data + subobj_offset, n);
+ memcpy (contents, this->m_data + subobj_offset, n);
}
break;
@@ -1028,11 +1031,30 @@ dwarf_expr_context::fetch_result (struct type *type,
}
}
- set_value_initialized (retval, this->initialized);
+ set_value_initialized (retval, this->m_initialized);
return retval;
}
+/* See expr.h. */
+
+struct value *
+dwarf_expr_context::evaluate (const gdb_byte *addr, size_t len,
+ struct dwarf2_per_cu_data *per_cu,
+ struct frame_info *frame,
+ const struct property_addr_info *addr_info,
+ struct type *type,
+ struct type *subobj_type,
+ LONGEST subobj_offset)
+{
+ this->m_per_cu = per_cu;
+ this->m_frame = frame;
+ this->m_addr_info = addr_info;
+
+ eval (addr, len);
+ return fetch_result (type, subobj_type, subobj_offset);
+}
+
/* Require that TYPE be an integral type; throw an exception if not. */
static void
@@ -1093,8 +1115,9 @@ get_signed_type (struct gdbarch *gdbarch, struct type *type)
CORE_ADDR
dwarf_expr_context::fetch_address (int n)
{
+ struct gdbarch *gdbarch = this->m_per_objfile->objfile->arch ();
struct value *result_val = fetch (n);
- enum bfd_endian byte_order = gdbarch_byte_order (this->gdbarch);
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
ULONGEST result;
dwarf_require_integral (value_type (result_val));
@@ -1108,14 +1131,14 @@ dwarf_expr_context::fetch_address (int n)
extract_unsigned_integer() will not produce a correct
result. Make sure we invoke gdbarch_integer_to_address()
for those architectures which require it. */
- if (gdbarch_integer_to_address_p (this->gdbarch))
+ if (gdbarch_integer_to_address_p (gdbarch))
{
- gdb_byte *buf = (gdb_byte *) alloca (this->addr_size);
- struct type *int_type = get_unsigned_type (this->gdbarch,
+ gdb_byte *buf = (gdb_byte *) alloca (this->m_addr_size);
+ struct type *int_type = get_unsigned_type (gdbarch,
value_type (result_val));
- store_unsigned_integer (buf, this->addr_size, byte_order, result);
- return gdbarch_integer_to_address (this->gdbarch, int_type, buf);
+ store_unsigned_integer (buf, this->m_addr_size, byte_order, result);
+ return gdbarch_integer_to_address (gdbarch, int_type, buf);
}
return (CORE_ADDR) result;
@@ -1126,11 +1149,11 @@ dwarf_expr_context::fetch_address (int n)
bool
dwarf_expr_context::fetch_in_stack_memory (int n)
{
- if (stack.size () <= n)
+ if (this->m_stack.size () <= n)
error (_("Asked for position %d of stack, "
"stack only has %zu elements on it."),
- n, stack.size ());
- return stack[stack.size () - (1 + n)].in_stack_memory;
+ n, this->m_stack.size ());
+ return this->m_stack[this->m_stack.size () - (1 + n)].in_stack_memory;
}
/* Return true if the expression stack is empty. */
@@ -1138,24 +1161,24 @@ dwarf_expr_context::fetch_in_stack_memory (int n)
bool
dwarf_expr_context::stack_empty_p () const
{
- return stack.empty ();
+ return m_stack.empty ();
}
/* Add a new piece to the dwarf_expr_context's piece list. */
void
dwarf_expr_context::add_piece (ULONGEST size, ULONGEST offset)
{
- this->pieces.emplace_back ();
- dwarf_expr_piece &p = this->pieces.back ();
+ this->m_pieces.emplace_back ();
+ dwarf_expr_piece &p = this->m_pieces.back ();
- p.location = this->location;
+ p.location = this->m_location;
p.size = size;
p.offset = offset;
if (p.location == DWARF_VALUE_LITERAL)
{
- p.v.literal.data = this->data;
- p.v.literal.length = this->len;
+ p.v.literal.data = this->m_data;
+ p.v.literal.length = this->m_len;
}
else if (stack_empty_p ())
{
@@ -1164,7 +1187,7 @@ dwarf_expr_context::add_piece (ULONGEST size, ULONGEST offset)
a somewhat strange approach, but this lets us avoid setting
the location to DWARF_VALUE_MEMORY in all the individual
cases in the evaluator. */
- this->location = DWARF_VALUE_OPTIMIZED_OUT;
+ this->m_location = DWARF_VALUE_OPTIMIZED_OUT;
}
else if (p.location == DWARF_VALUE_MEMORY)
{
@@ -1173,7 +1196,7 @@ dwarf_expr_context::add_piece (ULONGEST size, ULONGEST offset)
}
else if (p.location == DWARF_VALUE_IMPLICIT_POINTER)
{
- p.v.ptr.die_sect_off = (sect_offset) this->len;
+ p.v.ptr.die_sect_off = (sect_offset) this->m_len;
p.v.ptr.offset = value_as_long (fetch (0));
}
else if (p.location == DWARF_VALUE_REGISTER)
@@ -1189,13 +1212,13 @@ dwarf_expr_context::add_piece (ULONGEST size, ULONGEST offset)
void
dwarf_expr_context::eval (const gdb_byte *addr, size_t len)
{
- int old_recursion_depth = this->recursion_depth;
+ int old_recursion_depth = this->m_recursion_depth;
execute_stack_op (addr, addr + len);
/* RECURSION_DEPTH becomes invalid if an exception was thrown here. */
- gdb_assert (this->recursion_depth == old_recursion_depth);
+ gdb_assert (this->m_recursion_depth == old_recursion_depth);
}
/* Helper to read a uleb128 value or throw an error. */
@@ -1439,7 +1462,8 @@ void
dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
const gdb_byte *op_end)
{
- enum bfd_endian byte_order = gdbarch_byte_order (this->gdbarch);
+ struct gdbarch *gdbarch = this->m_per_objfile->objfile->arch ();
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
/* Old-style "untyped" DWARF values need special treatment in a
couple of places, specifically DW_OP_mod and DW_OP_shr. We need
a special type for these values so we can distinguish them from
@@ -1449,13 +1473,13 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
CU. */
struct type *address_type = this->address_type ();
- this->location = DWARF_VALUE_MEMORY;
- this->initialized = 1; /* Default is initialized. */
+ this->m_location = DWARF_VALUE_MEMORY;
+ this->m_initialized = 1; /* Default is initialized. */
- if (this->recursion_depth > this->max_recursion_depth)
+ if (this->m_recursion_depth > this->m_max_recursion_depth)
error (_("DWARF-2 expression error: Loop detected (%d)."),
- this->recursion_depth);
- this->recursion_depth++;
+ this->m_recursion_depth);
+ this->m_recursion_depth++;
while (op_ptr < op_end)
{
@@ -1516,32 +1540,32 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_addr:
result = extract_unsigned_integer (op_ptr,
- this->addr_size, byte_order);
- op_ptr += this->addr_size;
+ this->m_addr_size, byte_order);
+ op_ptr += this->m_addr_size;
/* Some versions of GCC emit DW_OP_addr before
DW_OP_GNU_push_tls_address. In this case the value is an
index, not an address. We don't support things like
branching between the address and the TLS op. */
if (op_ptr >= op_end || *op_ptr != DW_OP_GNU_push_tls_address)
- result += this->per_objfile->objfile->text_section_offset ();
+ result += this->m_per_objfile->objfile->text_section_offset ();
result_val = value_from_ulongest (address_type, result);
break;
case DW_OP_addrx:
case DW_OP_GNU_addr_index:
- ensure_have_per_cu (this->per_cu, "DW_OP_addrx");
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_addrx");
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
- result = dwarf2_read_addr_index (this->per_cu, this->per_objfile,
+ result = dwarf2_read_addr_index (this->m_per_cu, this->m_per_objfile,
uoffset);
- result += this->per_objfile->objfile->text_section_offset ();
+ result += this->m_per_objfile->objfile->text_section_offset ();
result_val = value_from_ulongest (address_type, result);
break;
case DW_OP_GNU_const_index:
- ensure_have_per_cu (this->per_cu, "DW_OP_GNU_const_index");
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_GNU_const_index");
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
- result = dwarf2_read_addr_index (this->per_cu, this->per_objfile,
+ result = dwarf2_read_addr_index (this->m_per_cu, this->m_per_objfile,
uoffset);
result_val = value_from_ulongest (address_type, result);
break;
@@ -1635,7 +1659,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
result = op - DW_OP_reg0;
result_val = value_from_ulongest (address_type, result);
- this->location = DWARF_VALUE_REGISTER;
+ this->m_location = DWARF_VALUE_REGISTER;
break;
case DW_OP_regx:
@@ -1644,7 +1668,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
result = reg;
result_val = value_from_ulongest (address_type, result);
- this->location = DWARF_VALUE_REGISTER;
+ this->m_location = DWARF_VALUE_REGISTER;
break;
case DW_OP_implicit_value:
@@ -1654,9 +1678,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
op_ptr = safe_read_uleb128 (op_ptr, op_end, &len);
if (op_ptr + len > op_end)
error (_("DW_OP_implicit_value: too few bytes available."));
- this->len = len;
- this->data = op_ptr;
- this->location = DWARF_VALUE_LITERAL;
+ this->m_len = len;
+ this->m_data = op_ptr;
+ this->m_location = DWARF_VALUE_LITERAL;
op_ptr += len;
dwarf_expr_require_composition (op_ptr, op_end,
"DW_OP_implicit_value");
@@ -1664,7 +1688,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
goto no_push;
case DW_OP_stack_value:
- this->location = DWARF_VALUE_STACK;
+ this->m_location = DWARF_VALUE_STACK;
dwarf_expr_require_composition (op_ptr, op_end, "DW_OP_stack_value");
goto no_push;
@@ -1672,12 +1696,12 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_GNU_implicit_pointer:
{
int64_t len;
- ensure_have_per_cu (this->per_cu, "DW_OP_implicit_pointer");
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_implicit_pointer");
- int ref_addr_size = this->per_cu->ref_addr_size ();
+ int ref_addr_size = this->m_per_cu->ref_addr_size ();
/* The referred-to DIE of sect_offset kind. */
- this->len = extract_unsigned_integer (op_ptr, ref_addr_size,
+ this->m_len = extract_unsigned_integer (op_ptr, ref_addr_size,
byte_order);
op_ptr += ref_addr_size;
@@ -1686,7 +1710,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
result = (ULONGEST) len;
result_val = value_from_ulongest (address_type, result);
- this->location = DWARF_VALUE_IMPLICIT_POINTER;
+ this->m_location = DWARF_VALUE_IMPLICIT_POINTER;
dwarf_expr_require_composition (op_ptr, op_end,
"DW_OP_implicit_pointer");
}
@@ -1726,9 +1750,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);
- ensure_have_frame (this->frame, "DW_OP_breg");
+ ensure_have_frame (this->m_frame, "DW_OP_breg");
- result = read_addr_from_reg (this->frame, op - DW_OP_breg0);
+ result = read_addr_from_reg (this->m_frame, op - DW_OP_breg0);
result += offset;
result_val = value_from_ulongest (address_type, result);
}
@@ -1737,9 +1761,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
{
op_ptr = safe_read_uleb128 (op_ptr, op_end, ®);
op_ptr = safe_read_sleb128 (op_ptr, op_end, &offset);
- ensure_have_frame (this->frame, "DW_OP_bregx");
+ ensure_have_frame (this->m_frame, "DW_OP_bregx");
- result = read_addr_from_reg (this->frame, reg);
+ result = read_addr_from_reg (this->m_frame, reg);
result += offset;
result_val = value_from_ulongest (address_type, result);
}
@@ -1755,19 +1779,19 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
backup the current stack locally and install a new empty stack,
then reset it afterwards, effectively erasing whatever the
recursive call put there. */
- std::vector<dwarf_stack_value> saved_stack = std::move (stack);
- stack.clear ();
+ std::vector<dwarf_stack_value> saved_stack = std::move (this->m_stack);
+ this->m_stack.clear ();
/* FIXME: cagney/2003-03-26: This code should be using
get_frame_base_address(), and then implement a dwarf2
specific this_base method. */
this->get_frame_base (&datastart, &datalen);
eval (datastart, datalen);
- if (this->location == DWARF_VALUE_MEMORY)
+ if (this->m_location == DWARF_VALUE_MEMORY)
result = fetch_address (0);
- else if (this->location == DWARF_VALUE_REGISTER)
+ else if (this->m_location == DWARF_VALUE_REGISTER)
result
- = read_addr_from_reg (this->frame, value_as_long (fetch (0)));
+ = read_addr_from_reg (this->m_frame, value_as_long (fetch (0)));
else
error (_("Not implemented: computing frame "
"base using explicit value operator"));
@@ -1776,9 +1800,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
in_stack_memory = true;
/* Restore the content of the original stack. */
- stack = std::move (saved_stack);
+ this->m_stack = std::move (saved_stack);
- this->location = DWARF_VALUE_MEMORY;
+ this->m_location = DWARF_VALUE_MEMORY;
}
break;
@@ -1799,13 +1823,13 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_swap:
{
- if (stack.size () < 2)
+ if (this->m_stack.size () < 2)
error (_("Not enough elements for "
"DW_OP_swap. Need 2, have %zu."),
- stack.size ());
+ this->m_stack.size ());
- dwarf_stack_value &t1 = stack[stack.size () - 1];
- dwarf_stack_value &t2 = stack[stack.size () - 2];
+ dwarf_stack_value &t1 = this->m_stack[this->m_stack.size () - 1];
+ dwarf_stack_value &t2 = this->m_stack[this->m_stack.size () - 2];
std::swap (t1, t2);
goto no_push;
}
@@ -1817,15 +1841,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_rot:
{
- if (stack.size () < 3)
+ if (this->m_stack.size () < 3)
error (_("Not enough elements for "
"DW_OP_rot. Need 3, have %zu."),
- stack.size ());
-
- dwarf_stack_value temp = stack[stack.size () - 1];
- stack[stack.size () - 1] = stack[stack.size () - 2];
- stack[stack.size () - 2] = stack[stack.size () - 3];
- stack[stack.size () - 3] = temp;
+ this->m_stack.size ());
+
+ dwarf_stack_value temp = this->m_stack[this->m_stack.size () - 1];
+ this->m_stack[this->m_stack.size () - 1]
+ = this->m_stack[this->m_stack.size () - 2];
+ this->m_stack[this->m_stack.size () - 2]
+ = this->m_stack[this->m_stack.size () - 3];
+ this->m_stack[this->m_stack.size () - 3] = temp;
goto no_push;
}
@@ -1834,7 +1860,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_deref_type:
case DW_OP_GNU_deref_type:
{
- int addr_size = (op == DW_OP_deref ? this->addr_size : *op_ptr++);
+ int addr_size = (op == DW_OP_deref ? this->m_addr_size : *op_ptr++);
gdb_byte *buf = (gdb_byte *) alloca (addr_size);
CORE_ADDR addr = fetch_address (0);
struct type *type;
@@ -1956,7 +1982,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
if (orig_type == address_type)
{
struct type *utype
- = get_unsigned_type (this->gdbarch, orig_type);
+ = get_unsigned_type (gdbarch, orig_type);
cast_back = 1;
first = value_cast (utype, first);
@@ -1991,7 +2017,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
if (!value_type (first)->is_unsigned ())
{
struct type *utype
- = get_unsigned_type (this->gdbarch, value_type (first));
+ = get_unsigned_type (gdbarch, value_type (first));
first = value_cast (utype, first);
}
@@ -2008,7 +2034,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
if (value_type (first)->is_unsigned ())
{
struct type *stype
- = get_signed_type (this->gdbarch, value_type (first));
+ = get_signed_type (gdbarch, value_type (first));
first = value_cast (stype, first);
}
@@ -2059,9 +2085,9 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
break;
case DW_OP_call_frame_cfa:
- ensure_have_frame (this->frame, "DW_OP_call_frame_cfa");
+ ensure_have_frame (this->m_frame, "DW_OP_call_frame_cfa");
- result = dwarf2_frame_cfa (this->frame);
+ result = dwarf2_frame_cfa (this->m_frame);
result_val = value_from_ulongest (address_type, result);
in_stack_memory = true;
break;
@@ -2078,7 +2104,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
returned. */
result = value_as_long (fetch (0));
pop ();
- result = target_translate_tls_address (this->per_objfile->objfile,
+ result = target_translate_tls_address (this->m_per_objfile->objfile,
result);
result_val = value_from_ulongest (address_type, result);
break;
@@ -2116,10 +2142,10 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
/* Pop off the address/regnum, and reset the location
type. */
- if (this->location != DWARF_VALUE_LITERAL
- && this->location != DWARF_VALUE_OPTIMIZED_OUT)
+ if (this->m_location != DWARF_VALUE_LITERAL
+ && this->m_location != DWARF_VALUE_OPTIMIZED_OUT)
pop ();
- this->location = DWARF_VALUE_MEMORY;
+ this->m_location = DWARF_VALUE_MEMORY;
}
goto no_push;
@@ -2134,10 +2160,10 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
/* Pop off the address/regnum, and reset the location
type. */
- if (this->location != DWARF_VALUE_LITERAL
- && this->location != DWARF_VALUE_OPTIMIZED_OUT)
+ if (this->m_location != DWARF_VALUE_LITERAL
+ && this->m_location != DWARF_VALUE_OPTIMIZED_OUT)
pop ();
- this->location = DWARF_VALUE_MEMORY;
+ this->m_location = DWARF_VALUE_MEMORY;
}
goto no_push;
@@ -2146,7 +2172,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
error (_("DWARF-2 expression error: DW_OP_GNU_uninit must always "
"be the very last op."));
- this->initialized = 0;
+ this->m_initialized = 0;
goto no_push;
case DW_OP_call2:
@@ -2169,16 +2195,16 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_GNU_variable_value:
{
- ensure_have_per_cu (this->per_cu, "DW_OP_GNU_variable_value");
- int ref_addr_size = this->per_cu->ref_addr_size ();
+ ensure_have_per_cu (this->m_per_cu, "DW_OP_GNU_variable_value");
+ int ref_addr_size = this->m_per_cu->ref_addr_size ();
sect_offset sect_off
= (sect_offset) extract_unsigned_integer (op_ptr,
ref_addr_size,
byte_order);
op_ptr += ref_addr_size;
- result_val = sect_variable_value (sect_off, this->per_cu,
- this->per_objfile);
+ result_val = sect_variable_value (sect_off, this->m_per_cu,
+ this->m_per_objfile);
result_val = value_cast (address_type, result_val);
}
break;
@@ -2210,7 +2236,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
if (kind_u.dwarf_reg != -1)
{
if (deref_size == -1)
- deref_size = this->addr_size;
+ deref_size = this->m_addr_size;
op_ptr += len;
this->push_dwarf_reg_entry_value (CALL_SITE_PARAMETER_DWARF_REG,
kind_u, deref_size);
@@ -2265,13 +2291,13 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
cu_offset type_die_cu_off = (cu_offset) uoffset;
- ensure_have_frame (this->frame, "DW_OP_regval_type");
+ ensure_have_frame (this->m_frame, "DW_OP_regval_type");
struct type *type = get_base_type (type_die_cu_off);
int regnum
- = dwarf_reg_to_regnum_or_error (get_frame_arch (this->frame),
+ = dwarf_reg_to_regnum_or_error (get_frame_arch (this->m_frame),
reg);
- result_val = value_from_register (type, regnum, this->frame);
+ result_val = value_from_register (type, regnum, this->m_frame);
}
break;
@@ -2311,11 +2337,11 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
case DW_OP_push_object_address:
/* Return the address of the object we are currently observing. */
- if (this->data_view.data () == nullptr
- && this->obj_address == 0)
+ if (this->m_addr_info == nullptr)
error (_("Location address is not set."));
- result_val = value_from_ulongest (address_type, this->obj_address);
+ result_val
+ = value_from_ulongest (address_type, this->m_addr_info->addr);
break;
default:
@@ -2332,11 +2358,11 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
/* To simplify our main caller, if the result is an implicit
pointer, then make a pieced value. This is ok because we can't
have implicit pointers in contexts where pieces are invalid. */
- if (this->location == DWARF_VALUE_IMPLICIT_POINTER)
- add_piece (8 * this->addr_size, 0);
+ if (this->m_location == DWARF_VALUE_IMPLICIT_POINTER)
+ add_piece (8 * this->m_addr_size, 0);
- this->recursion_depth--;
- gdb_assert (this->recursion_depth >= 0);
+ this->m_recursion_depth--;
+ gdb_assert (this->m_recursion_depth >= 0);
}
void _initialize_dwarf2expr ();
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index c2138b31d79..f3985a7ecce 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -119,45 +119,49 @@ struct dwarf_stack_value
its current state and its callbacks. */
struct dwarf_expr_context
{
- dwarf_expr_context (dwarf2_per_objfile *per_objfile);
+ dwarf_expr_context (struct dwarf2_per_objfile *per_objfile,
+ int addr_size);
virtual ~dwarf_expr_context () = default;
void push_address (CORE_ADDR value, bool in_stack_memory);
- void eval (const gdb_byte *addr, size_t len);
- /* 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);
+ /* Evaluate the expression at ADDR (LEN bytes long) in a given PER_CU
+ and FRAME context. TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe
+ the expected struct value representation of the evaluation
+ result. The ADDR_INFO property can be specified to override the
+ range of memory addresses with the passed in buffer. */
+ struct value *evaluate (const gdb_byte *addr, size_t len,
+ struct dwarf2_per_cu_data *per_cu,
+ struct frame_info *frame,
+ const struct property_addr_info *addr_info = nullptr,
+ struct type *type = nullptr,
+ struct type *subobj_type = nullptr,
+ LONGEST subobj_offset = 0);
+private:
/* The stack of values. */
- std::vector<dwarf_stack_value> stack;
-
- /* Target architecture to use for address operations. */
- struct gdbarch *gdbarch = nullptr;
+ std::vector<dwarf_stack_value> m_stack;
/* Target address size in bytes. */
- int addr_size = 0;
+ int m_addr_size = 0;
/* The current depth of dwarf expression recursion, via DW_OP_call*,
DW_OP_fbreg, DW_OP_push_object_address, etc., and the maximum
depth we'll tolerate before raising an error. */
- int recursion_depth = 0, max_recursion_depth = 0x100;
+ int m_recursion_depth = 0, m_max_recursion_depth = 0x100;
/* Location of the value. */
- enum dwarf_value_location location = DWARF_VALUE_MEMORY;
+ enum dwarf_value_location m_location = DWARF_VALUE_MEMORY;
/* For DWARF_VALUE_LITERAL, the current literal value's length and
data. For DWARF_VALUE_IMPLICIT_POINTER, LEN is the offset of the
target DIE of sect_offset kind. */
- ULONGEST len = 0;
- const gdb_byte *data = nullptr;
+ ULONGEST m_len = 0;
+ const gdb_byte *m_data = nullptr;
/* Initialization status of variable: Non-zero if variable has been
initialized; zero otherwise. */
- int initialized = 0;
+ int m_initialized = 0;
/* A vector of pieces.
@@ -181,25 +185,21 @@ struct dwarf_expr_context
no DW_OP_piece operations have no value to place in a piece's
'size' field; the size comes from the surrounding data. So the
two cases need to be handled separately.) */
- std::vector<dwarf_expr_piece> pieces;
+ std::vector<dwarf_expr_piece> m_pieces;
/* We evaluate the expression in the context of this objfile. */
- dwarf2_per_objfile *per_objfile;
+ dwarf2_per_objfile *m_per_objfile;
/* Frame information used for the evaluation. */
- struct frame_info *frame = nullptr;
+ struct frame_info *m_frame = nullptr;
/* Compilation unit used for the evaluation. */
- struct dwarf2_per_cu_data *per_cu = nullptr;
-
- /* Object address used for the evaluation. */
- CORE_ADDR obj_address = 0;
+ struct dwarf2_per_cu_data *m_per_cu = nullptr;
- /* The data that was passed in. */
- gdb::array_view<const gdb_byte> data_view;
-
-private:
+ /* Property address info used for the evaluation. */
+ const struct property_addr_info *m_addr_info = nullptr;
+ void eval (const gdb_byte *addr, size_t len);
struct type *address_type () const;
void push (struct value *value, bool in_stack_memory);
bool stack_empty_p () const;
@@ -210,6 +210,13 @@ struct dwarf_expr_context
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,
+ struct type *subobj_type,
+ LONGEST subobj_offset);
+
/* 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. */
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 8aaa0af7e91..568b6366dce 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -229,25 +229,16 @@ 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)
{
- dwarf_expr_context ctx (per_objfile);
+ dwarf_expr_context ctx (per_objfile, addr_size);
scoped_value_mark free_values;
- ctx.frame = this_frame;
- ctx.gdbarch = get_frame_arch (this_frame);
- ctx.addr_size = addr_size;
-
ctx.push_address (initial, initial_in_stack_memory);
- ctx.eval (exp, len);
-
- CORE_ADDR result;
- struct value *result_val = ctx.fetch_result ();
+ struct value *result_val = ctx.evaluate (exp, len, nullptr, this_frame);
if (VALUE_LVAL (result_val) == lval_memory)
- result = value_address (result_val);
+ return value_address (result_val);
else
- result = value_as_address (result_val);
-
- return result;
+ return value_as_address (result_val);
}
\f
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 6584a1615f7..4e93a4d91d0 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1431,8 +1431,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
struct type *subobj_type,
LONGEST subobj_byte_offset)
{
- struct value *retval;
-
if (subobj_type == NULL)
{
subobj_type = type;
@@ -1444,21 +1442,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
if (size == 0)
return allocate_optimized_out_value (subobj_type);
- dwarf_expr_context ctx (per_objfile);
- ctx.frame = frame;
- ctx.per_cu = per_cu;
- ctx.obj_address = 0;
+ dwarf_expr_context ctx (per_objfile, per_cu->addr_size ());
+ struct value *retval;
scoped_value_mark free_values;
- ctx.gdbarch = per_objfile->objfile->arch ();
- ctx.addr_size = per_cu->addr_size ();
-
try
{
- ctx.eval (data, size);
- retval = ctx.fetch_result (type, subobj_type,
- subobj_byte_offset);
+ retval = ctx.evaluate (data, size, per_cu, frame, nullptr, type,
+ subobj_type, subobj_byte_offset);
}
catch (const gdb_exception_error &ex)
{
@@ -1528,29 +1520,24 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
return 0;
dwarf2_per_objfile *per_objfile = dlbaton->per_objfile;
- dwarf_expr_context ctx (per_objfile);
+ struct dwarf2_per_cu_data *per_cu = dlbaton->per_cu;
+ dwarf_expr_context ctx (per_objfile, per_cu->addr_size ());
struct value *result;
scoped_value_mark free_values;
- ctx.frame = frame;
- ctx.per_cu = dlbaton->per_cu;
- if (addr_stack != nullptr)
+ if (push_initial_value)
{
- ctx.obj_address = addr_stack->addr;
- ctx.data_view = addr_stack->valaddr;
+ if (addr_stack != nullptr)
+ ctx.push_address (addr_stack->addr, false);
+ else
+ ctx.push_address (0, false);
}
- ctx.gdbarch = per_objfile->objfile->arch ();
- ctx.addr_size = dlbaton->per_cu->addr_size ();
-
- if (push_initial_value)
- ctx.push_address (ctx.obj_address, false);
-
try
{
- ctx.eval (dlbaton->data, dlbaton->size);
- result = ctx.fetch_result ();
+ result = ctx.evaluate (dlbaton->data, dlbaton->size,
+ per_cu, frame, addr_stack);
}
catch (const gdb_exception_error &ex)
{
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 17/17] Add as_lval argument to expression evaluator
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (15 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 16/17] Simplify dwarf_expr_context class interface Zoran Zaric
@ 2021-05-28 15:46 ` Zoran Zaric
2021-06-09 1:04 ` [PATCH v3 00/17] DWARF expression evaluator design cleanup Simon Marchi
17 siblings, 0 replies; 36+ messages in thread
From: Zoran Zaric @ 2021-05-28 15:46 UTC (permalink / raw)
To: gdb-patches
From: Zoran Zaric <Zoran.Zaric@amd.com>
There are cases where the result of the expression evaluation is
expected to be in a form of a value and not location description.
One place that has this requirement is dwarf_entry_parameter_to_value
function, but more are expected in the future. Until now, this
requirement was fulfilled by extending the evaluated expression with
a DW_OP_stack_value operation at the end.
New implementation, introduces a new evaluation argument instead.
* dwarf2/expr.c (dwarf_expr_context::fetch_result): Add as_lval
argument.
(dwarf_expr_context::eval_exp): Add as_lval argument.
* dwarf2/expr.h (struct dwarf_expr_context): Add as_lval
argument to fetch_result and eval_exp methods.
* dwarf2/frame.c (execute_stack_op): Add as_lval argument.
* dwarf2/loc.c (dwarf_entry_parameter_to_value): Remove
DWARF expression extension.
(dwarf2_evaluate_loc_desc_full): Add as_lval argument support.
(dwarf2_evaluate_loc_desc): Add as_lval argument support.
(dwarf2_locexpr_baton_eval): Add as_lval argument support.
---
gdb/dwarf2/expr.c | 12 +++++++++---
gdb/dwarf2/expr.h | 23 ++++++++++++++++-------
gdb/dwarf2/frame.c | 2 +-
gdb/dwarf2/loc.c | 27 ++++++++++-----------------
gdb/dwarf2/loc.h | 6 ++++--
5 files changed, 40 insertions(+), 30 deletions(-)
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 30b0cef81d2..66c5fe86b49 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -887,7 +887,8 @@ dwarf_expr_context::push_dwarf_reg_entry_value
struct value *
dwarf_expr_context::fetch_result (struct type *type,
struct type *subobj_type,
- LONGEST subobj_offset)
+ LONGEST subobj_offset,
+ bool as_lval)
{
struct value *retval = nullptr;
struct gdbarch *gdbarch = this->m_per_objfile->objfile->arch ();
@@ -918,6 +919,11 @@ dwarf_expr_context::fetch_result (struct type *type,
}
else
{
+ /* If AS_LVAL is false, means that the implicit conversion
+ from a location description to value is expected. */
+ if (!as_lval)
+ this->m_location = DWARF_VALUE_STACK;
+
switch (this->m_location)
{
case DWARF_VALUE_REGISTER:
@@ -1039,7 +1045,7 @@ dwarf_expr_context::fetch_result (struct type *type,
/* See expr.h. */
struct value *
-dwarf_expr_context::evaluate (const gdb_byte *addr, size_t len,
+dwarf_expr_context::evaluate (const gdb_byte *addr, size_t len, bool as_lval,
struct dwarf2_per_cu_data *per_cu,
struct frame_info *frame,
const struct property_addr_info *addr_info,
@@ -1052,7 +1058,7 @@ dwarf_expr_context::evaluate (const gdb_byte *addr, size_t len,
this->m_addr_info = addr_info;
eval (addr, len);
- return fetch_result (type, subobj_type, subobj_offset);
+ return fetch_result (type, subobj_type, subobj_offset, as_lval);
}
/* Require that TYPE be an integral type; throw an exception if not. */
diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
index f3985a7ecce..6918c6c956d 100644
--- a/gdb/dwarf2/expr.h
+++ b/gdb/dwarf2/expr.h
@@ -126,11 +126,17 @@ struct dwarf_expr_context
void push_address (CORE_ADDR value, bool in_stack_memory);
/* Evaluate the expression at ADDR (LEN bytes long) in a given PER_CU
- and FRAME context. TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe
- the expected struct value representation of the evaluation
- result. The ADDR_INFO property can be specified to override the
- range of memory addresses with the passed in buffer. */
- struct value *evaluate (const gdb_byte *addr, size_t len,
+ and FRAME context.
+
+ AS_LVAL defines if the returned struct value is expected to be a
+ value (false) or a location description (true).
+
+ TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe the expected struct
+ value representation of the evaluation result.
+
+ The ADDR_INFO property can be specified to override the range of
+ memory addresses with the passed in buffer. */
+ struct value *evaluate (const gdb_byte *addr, size_t len, bool as_lval,
struct dwarf2_per_cu_data *per_cu,
struct frame_info *frame,
const struct property_addr_info *addr_info = nullptr,
@@ -212,10 +218,13 @@ struct dwarf_expr_context
/* 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. */
+ describe the source level representation of that result.
+ AS_LVAL defines if the fetched struct value is expected to
+ be a value or a location description. */
struct value *fetch_result (struct type *type,
struct type *subobj_type,
- LONGEST subobj_offset);
+ LONGEST subobj_offset,
+ bool as_lval);
/* Return the location expression for the frame base attribute, in
START and LENGTH. The result must be live until the current
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 568b6366dce..8944df631b7 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -233,7 +233,7 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
scoped_value_mark free_values;
ctx.push_address (initial, initial_in_stack_memory);
- struct value *result_val = ctx.evaluate (exp, len, nullptr, this_frame);
+ struct value *result_val = ctx.evaluate (exp, len, true, nullptr, this_frame);
if (VALUE_LVAL (result_val) == lval_memory)
return value_address (result_val);
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 4e93a4d91d0..5bdb31ec48f 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -50,7 +50,7 @@
static struct value *dwarf2_evaluate_loc_desc_full
(struct type *type, struct frame_info *frame, const gdb_byte *data,
size_t size, dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
- struct type *subobj_type, LONGEST subobj_byte_offset);
+ struct type *subobj_type, LONGEST subobj_byte_offset, bool as_lval = true);
/* Until these have formal names, we define these here.
ref: http://gcc.gnu.org/wiki/DebugFission
@@ -1184,7 +1184,6 @@ dwarf_entry_parameter_to_value (struct call_site_parameter *parameter,
dwarf2_per_objfile *per_objfile)
{
const gdb_byte *data_src;
- gdb_byte *data;
size_t size;
data_src = deref_size == -1 ? parameter->value : parameter->data_value;
@@ -1195,15 +1194,8 @@ dwarf_entry_parameter_to_value (struct call_site_parameter *parameter,
throw_error (NO_ENTRY_VALUE_ERROR,
_("Cannot resolve DW_AT_call_data_value"));
- /* DW_AT_call_value is a DWARF expression, not a DWARF
- location. Postprocessing of DWARF_VALUE_MEMORY would lose the type from
- DWARF block. */
- data = (gdb_byte *) alloca (size + 1);
- memcpy (data, data_src, size);
- data[size] = DW_OP_stack_value;
-
- return dwarf2_evaluate_loc_desc (type, caller_frame, data, size + 1, per_cu,
- per_objfile);
+ return dwarf2_evaluate_loc_desc (type, caller_frame, data_src, size, per_cu,
+ per_objfile, false);
}
/* VALUE must be of type lval_computed with entry_data_value_funcs. Perform
@@ -1429,7 +1421,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
dwarf2_per_cu_data *per_cu,
dwarf2_per_objfile *per_objfile,
struct type *subobj_type,
- LONGEST subobj_byte_offset)
+ LONGEST subobj_byte_offset,
+ bool as_lval)
{
if (subobj_type == NULL)
{
@@ -1449,8 +1442,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
try
{
- retval = ctx.evaluate (data, size, per_cu, frame, nullptr, type,
- subobj_type, subobj_byte_offset);
+ retval = ctx.evaluate (data, size, as_lval, per_cu, frame, nullptr,
+ type, subobj_type, subobj_byte_offset);
}
catch (const gdb_exception_error &ex)
{
@@ -1491,10 +1484,10 @@ struct value *
dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
const gdb_byte *data, size_t size,
dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile)
+ dwarf2_per_objfile *per_objfile, bool as_lval)
{
return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu,
- per_objfile, NULL, 0);
+ per_objfile, NULL, 0, as_lval);
}
/* Evaluates a dwarf expression and stores the result in VAL,
@@ -1537,7 +1530,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
try
{
result = ctx.evaluate (dlbaton->data, dlbaton->size,
- per_cu, frame, addr_stack);
+ true, per_cu, frame, addr_stack);
}
catch (const gdb_exception_error &ex)
{
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 06b25fbb78e..e2c5920bdee 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -67,14 +67,16 @@ struct call_site_parameter *dwarf_expr_reg_to_entry_parameter
/* Evaluate a location description, starting at DATA and with length
SIZE, to find the current location of variable of TYPE in the context
- of FRAME. */
+ of FRAME. AS_LVAL defines if the resulting struct value is expected to
+ be a value or a location description. */
struct value *dwarf2_evaluate_loc_desc (struct type *type,
struct frame_info *frame,
const gdb_byte *data,
size_t size,
dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile);
+ dwarf2_per_objfile *per_objfile,
+ bool as_lval = true);
/* A chain of addresses that might be needed to resolve a dynamic
property. */
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 01/17] Replace the symbol needs evaluator with a parser
2021-05-28 15:46 ` [PATCH v3 01/17] Replace the symbol needs evaluator with a parser Zoran Zaric
@ 2021-06-07 21:48 ` Simon Marchi
2021-06-11 17:22 ` Zaric, Zoran (Zare)
0 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2021-06-07 21:48 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches
Hi Zoran,
I left some comments, but to go a bit faster I implemented the changes
in a fixup patch that I pasted below. If you are fine with the changes,
you can just integrate them in your patch.
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index e816f926696..2fa68ff0426 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -2736,151 +2736,430 @@ dwarf2_compile_property_to_c (string_file *stream,
> data, data + size, per_cu, per_objfile);
> }
>
> -\f
> -/* Helper functions and baton for dwarf2_loc_desc_get_symbol_read_needs. */
> +/* Compute the correct symbol_needs_kind value for the location
> + expression in EXPR. */
>
> -class symbol_needs_eval_context : public dwarf_expr_context
> +static enum symbol_needs_kind
> +dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
> + dwarf2_per_cu_data *per_cu,
> + dwarf2_per_objfile *per_objfile,
> + bfd_endian byte_order,
> + int addr_size,
> + int ref_addr_size,
> + int depth = 0)
> {
> -public:
> - symbol_needs_eval_context (dwarf2_per_objfile *per_objfile)
> - : dwarf_expr_context (per_objfile)
> - {}
> + const gdb_byte *expr_end = expr.data () + expr.size ();
> + enum symbol_needs_kind symbol_needs = SYMBOL_NEEDS_NONE;
> + const int max_depth = 256;
> + std::vector <const gdb_byte *> next_op;
> + std::set <const gdb_byte *> visited_op;
"next_op" and "visited_op" should be plural (next_ops and
visited_ops). Although I'd renamed next_ops to ops_to_visit, I find
that a bit clearer.
Please add comments for the important variables here to say what they
are for (at least the set and the vector).
> - enum symbol_needs_kind needs;
> - struct dwarf2_per_cu_data *per_cu;
> + if (depth > max_depth)
> + error (_("DWARF-2 expression error: Loop detected (%d)."), depth);
At first I didn't undersand why this depth checking is needed. Given we
have the visited_ops set, we are sure to converge and not be stuck in an
infinite loop, even if there are loops in the expression. However,
after reading the code, I now understand that this depth check is for
expressions that call another expression with DW_OP_call*. So if for
example expression A calls expression B, which calls expression A, we
wouldn't catch it without this depth check. Is that right? If so,
maybe add a comment to explain this?
As for the error message, I don't think it's useful to print the depth.
It will just appear as "Loop detected (257)" to the user, which just
looks like a random number to them. It would be nice to print some
context, like what was being evaluated, or at least which objfile /
compilation unit contains the problematic expression(s), but that
doesn't seem easy.
> - /* Reads from registers do require a frame. */
> - CORE_ADDR read_addr_from_reg (int regnum) override
> - {
> - needs = SYMBOL_NEEDS_FRAME;
> - return 1;
> - }
> + depth++;
>
> - /* "get_reg_value" callback: Reads from registers do require a
> - frame. */
> + next_op.push_back (expr.data ());
>
> - struct value *get_reg_value (struct type *type, int regnum) override
> - {
> - needs = SYMBOL_NEEDS_FRAME;
> - return value_zero (type, not_lval);
> - }
> + while (!next_op.empty ())
> + {
> + const gdb_byte *op_ptr = next_op.back ();
I'd suggest popping the item out of next_ops and inserting it in
visited_ops right here, because we know we are unconditionally visiting
it now. And then, insert an op in next_ops only if it's not in
visited_ops. This way, we know there are only unvisited ops in
next_ops, and each op goes through next_ops exactly once.
>
> - /* Reads from memory do not require a frame. */
> - void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
> - {
> - memset (buf, 0, len);
> - }
> + if (op_ptr >= expr_end
> + || visited_op.find(op_ptr) != visited_op.end())
Missing spaces.
> + {
> + next_op.pop_back ();
> + continue;
> + }
>
> - /* Frame-relative accesses do require a frame. */
> - void get_frame_base (const gdb_byte **start, size_t *length) override
> - {
> - static gdb_byte lit0 = DW_OP_lit0;
> + enum dwarf_location_atom op
> + = (enum dwarf_location_atom) *op_ptr++;
> + uint64_t uoffset;
> + int64_t offset;
>
> - *start = &lit0;
> - *length = 1;
> + /* The DWARF expression might have a bug causing an infinite
> + loop. In that case, quitting is the only way out. */
> + QUIT;
>
> - needs = SYMBOL_NEEDS_FRAME;
> - }
> + 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:
> + case DW_OP_stack_value:
At first, I thought that when encountering DW_OP_stack_value, we should
not push the following op to the list of ops to visit, because the spec
says:
The DW_OP_stack_value operation terminates the expression.
That sounds to me like when encountering this, you stop evaluating and
whatever is on the stack is the value. But there are cases where a
DW_OP_stack_value is in the middle of the expression:
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.dwarf2/var-access.exp;h=7ccec20db6d877c2defe980bbf4acfe874249624;hb=HEAD#l240
So when I tried to implement that, it broke this test.
> + case DW_OP_call2:
> + {
> + cu_offset cu_off
> + = (cu_offset) extract_unsigned_integer (op_ptr, 2, byte_order);
> + op_ptr += 2;
> +
> + auto get_frame_pc = [&symbol_needs] ()
> + {
> + symbol_needs = SYMBOL_NEEDS_FRAME;
> + return 0;
> + };
> +
> + struct dwarf2_locexpr_baton baton
> + = dwarf2_fetch_die_loc_cu_off (cu_off, per_cu,
> + per_objfile,
> + get_frame_pc);
>
> - ctx.eval (data, size);
> + /* If SYMBOL_NEEDS_FRAME is returned from the previous call,
> + we dont have to check the baton content. */
> + if (symbol_needs != SYMBOL_NEEDS_FRAME)
> + {
> + gdbarch *arch = baton.per_objfile->objfile->arch ();
> + gdb::array_view<const gdb_byte> sub_expr (baton.data,
> + baton.size);
> + symbol_needs
> + = dwarf2_get_symbol_read_needs (sub_expr,
> + baton.per_cu,
> + baton.per_objfile,
> + gdbarch_byte_order (arch),
> + baton.per_cu->addr_size (),
> + baton.per_cu->ref_addr_size (),
> + depth);
> + }
> + break;
> + }
> +
> + case DW_OP_call4:
Since these two cases look alike, I'd suggest merging them.
I forgot to mention above, but I'd also suggest declaring uoffset and
offset in the specific cases that need it. And you can use
safe_skip_leb128 to avoid much of their uses.
And the fixup patch:
From 5ed60b518e01a5ac7ec4e96176cb110078df2ff1 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Mon, 7 Jun 2021 17:05:20 -0400
Subject: [PATCH] Patch 1 proposed fixups
Change-Id: Id6f57ebc96cd74b95465532ce2048b2c28de8642
---
gdb/dwarf2/loc.c | 178 ++++++++++++++++++++++++-----------------------
1 file changed, 91 insertions(+), 87 deletions(-)
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 9f84766d1d2f..802b4457cec1 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2772,45 +2772,76 @@ dwarf2_compile_property_to_c (string_file *stream,
}
/* Compute the correct symbol_needs_kind value for the location
- expression in EXPR. */
+ expression in EXPR.
-static enum symbol_needs_kind
+ Implemented by traversing the logical control flow graph of the
+ expression. */
+
+static symbol_needs_kind
dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
- dwarf2_per_cu_data *per_cu,
- dwarf2_per_objfile *per_objfile,
- bfd_endian byte_order,
- int addr_size,
- int ref_addr_size,
- int depth = 0)
+ dwarf2_per_cu_data *per_cu,
+ dwarf2_per_objfile *per_objfile,
+ bfd_endian byte_order,
+ int addr_size,
+ int ref_addr_size,
+ int depth = 0)
{
- const gdb_byte *expr_end = expr.data () + expr.size ();
enum symbol_needs_kind symbol_needs = SYMBOL_NEEDS_NONE;
- const int max_depth = 256;
- std::vector <const gdb_byte *> next_op;
- std::set <const gdb_byte *> visited_op;
+ /* If the expression is empty, we have nothing to do. */
+ if (expr.empty ())
+ return symbol_needs;
+
+ const gdb_byte *expr_end = &expr[0] + expr.size ();
+
+ /* List of operations to visit. Operations in this list are not visited yet,
+ so are not in VISITED_OPS (and vice-versa). */
+ std::vector<const gdb_byte *> ops_to_visit;
+
+ /* Operations already visited. */
+ std::unordered_set<const gdb_byte *> visited_ops;
+
+ /* Insert OP in OPS_TO_VISIT if it is within the expression's range and
+ hasn't been visited yet. */
+ auto insert_in_ops_to_visit
+ = [expr_end, &visited_ops, &ops_to_visit] (const gdb_byte *op_ptr)
+ {
+ if (op_ptr >= expr_end)
+ return;
+
+ if (visited_ops.find (op_ptr) != visited_ops.end ())
+ return;
+
+ ops_to_visit.push_back (op_ptr);
+ };
+
+ /* Expressions can invoke other expressions with DW_OP_call*. Protect against
+ a loop of calls. */
+ const int max_depth = 256;
if (depth > max_depth)
- error (_("DWARF-2 expression error: Loop detected (%d)."), depth);
+ error (_("DWARF-2 expression error: Loop detected."));
depth++;
- next_op.push_back (expr.data ());
+ /* Initialize the to-visit list with the first operation. */
+ insert_in_ops_to_visit (&expr[0]);
- while (!next_op.empty ())
+ while (!ops_to_visit.empty ())
{
- const gdb_byte *op_ptr = next_op.back ();
+ /* Pop one op to visit, mark it as visited. */
+ const gdb_byte *op_ptr = ops_to_visit.back ();
+ ops_to_visit.pop_back ();
+ gdb_assert (visited_ops.find (op_ptr) == visited_ops.end ());
+ visited_ops.insert (op_ptr);
- if (op_ptr >= expr_end
- || visited_op.find(op_ptr) != visited_op.end())
- {
- next_op.pop_back ();
- continue;
- }
+ dwarf_location_atom op = (dwarf_location_atom) *op_ptr;
- enum dwarf_location_atom op
- = (enum dwarf_location_atom) *op_ptr++;
- uint64_t uoffset;
- int64_t offset;
+ /* Most operations have a single possible following operation (they are
+ not conditional branches). The code below updates OP_PTR to point to
+ that following operation, which is pushed back to OPS_TO_VISIT, if
+ needed, at the bottom. Here, leave OP_PTR pointing just after the
+ operand. */
+ op_ptr++;
/* The DWARF expression might have a bug causing an infinite
loop. In that case, quitting is the only way out. */
@@ -2898,22 +2929,22 @@ dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
case DW_OP_constu:
case DW_OP_plus_uconst:
case DW_OP_piece:
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
break;
case DW_OP_consts:
- op_ptr = safe_read_sleb128 (op_ptr, expr_end, &offset);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
break;
case DW_OP_bit_piece:
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
break;
case DW_OP_deref_type:
case DW_OP_GNU_deref_type:
op_ptr++;
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
break;
case DW_OP_addr:
@@ -3017,14 +3048,17 @@ dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
break;
case DW_OP_implicit_value:
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
- op_ptr += uoffset;
+ {
+ uint64_t uoffset;
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ op_ptr += uoffset;
+ }
break;
case DW_OP_implicit_pointer:
case DW_OP_GNU_implicit_pointer:
op_ptr += ref_addr_size;
- op_ptr = safe_read_sleb128 (op_ptr, expr_end, &offset);
+ op_ptr = safe_skip_leb128 (op_ptr, expr_end);
break;
case DW_OP_deref_size:
@@ -3033,62 +3067,31 @@ dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
break;
case DW_OP_skip:
- offset = extract_signed_integer (op_ptr, 2, byte_order);
- op_ptr += 2;
- op_ptr += offset;
+ {
+ LONGEST offset = extract_signed_integer (op_ptr, 2, byte_order);
+ op_ptr += 2;
+ op_ptr += offset;
+ }
break;
case DW_OP_bra:
- visited_op.insert (next_op.back ());
- next_op.pop_back ();
-
- offset = extract_signed_integer (op_ptr, 2, byte_order);
- op_ptr += 2;
- next_op.push_back (op_ptr + offset);
- next_op.push_back (op_ptr);
- continue;
-
- case DW_OP_call2:
{
- cu_offset cu_off
- = (cu_offset) extract_unsigned_integer (op_ptr, 2, byte_order);
+ /* This is the only operation that pushes two operations in the
+ to-visit list, so handle it all here. */
+ LONGEST offset = extract_signed_integer (op_ptr, 2, byte_order);
op_ptr += 2;
-
- auto get_frame_pc = [&symbol_needs] ()
- {
- symbol_needs = SYMBOL_NEEDS_FRAME;
- return 0;
- };
-
- struct dwarf2_locexpr_baton baton
- = dwarf2_fetch_die_loc_cu_off (cu_off, per_cu,
- per_objfile,
- get_frame_pc);
-
- /* If SYMBOL_NEEDS_FRAME is returned from the previous call,
- we dont have to check the baton content. */
- if (symbol_needs != SYMBOL_NEEDS_FRAME)
- {
- gdbarch *arch = baton.per_objfile->objfile->arch ();
- gdb::array_view<const gdb_byte> sub_expr (baton.data,
- baton.size);
- symbol_needs
- = dwarf2_get_symbol_read_needs (sub_expr,
- baton.per_cu,
- baton.per_objfile,
- gdbarch_byte_order (arch),
- baton.per_cu->addr_size (),
- baton.per_cu->ref_addr_size (),
- depth);
- }
- break;
+ insert_in_ops_to_visit (op_ptr + offset);
+ insert_in_ops_to_visit (op_ptr);
+ continue;
}
+ case DW_OP_call2:
case DW_OP_call4:
{
+ unsigned int len = op == DW_OP_call2 ? 2 : 4;
cu_offset cu_off
- = (cu_offset) extract_unsigned_integer (op_ptr, 4, byte_order);
- op_ptr += 4;
+ = (cu_offset) extract_unsigned_integer (op_ptr, len, byte_order);
+ op_ptr += len;
auto get_frame_pc = [&symbol_needs] ()
{
@@ -3175,10 +3178,13 @@ dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
case DW_OP_const_type:
case DW_OP_GNU_const_type:
- op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
- offset = *op_ptr++;
- op_ptr += offset;
- break;
+ {
+ uint64_t uoffset;
+ op_ptr = safe_read_uleb128 (op_ptr, expr_end, &uoffset);
+ gdb_byte offset = *op_ptr++;
+ op_ptr += offset;
+ break;
+ }
default:
error (_("Unhandled DWARF expression opcode 0x%x"), op);
@@ -3189,9 +3195,7 @@ dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
if (symbol_needs == SYMBOL_NEEDS_FRAME)
break;
- visited_op.insert (next_op.back ());
- next_op.pop_back ();
- next_op.push_back (op_ptr);
+ insert_in_ops_to_visit (op_ptr);
}
return symbol_needs;
--
2.31.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 03/17] Move frame context info to dwarf_expr_context
2021-05-28 15:46 ` [PATCH v3 03/17] Move frame context info to dwarf_expr_context Zoran Zaric
@ 2021-06-09 0:45 ` Simon Marchi
2021-06-11 17:16 ` Zaric, Zoran (Zare)
0 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2021-06-09 0:45 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches
> @@ -56,6 +57,31 @@ dwarf_gdbarch_types_init (struct gdbarch *gdbarch)
> return types;
> }
>
> +/* Ensure that a FRAME is defined, throw an exception otherwise.
> +
> + Throwing NOT_AVAILABLE_ERROR error so that a client can chose
> + to react differently if the evaluation ended because there
> + was a missing context information. */
This comment needs to be updated (maybe just remove the second
paragraph).
LGTM with that fixed.
Simon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context
2021-05-28 15:46 ` [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context Zoran Zaric
@ 2021-06-09 0:50 ` Simon Marchi
2021-06-11 17:19 ` Zaric, Zoran (Zare)
0 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2021-06-09 0:50 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches
> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
> index ced78034fdf..d70f92f27bc 100644
> --- a/gdb/dwarf2/expr.c
> +++ b/gdb/dwarf2/expr.c
> @@ -72,6 +72,20 @@ ensure_have_frame (struct frame_info *frame, const char *op_name)
> _("%s evaluation requires a frame."), op_name);
> }
>
> +/* Ensure that a PER_CU is defined and throw an exception otherwise.
> +
> + Throwing NOT_AVAILABLE_ERROR error so that a client can chose
> + to react differently if the evaluation ended because there
> + was a missing context information. */
This comment is also stale.
> +
> +static void
> +ensure_have_per_cu (struct dwarf2_per_cu_data *per_cu, const char* op_name)
> +{
> + if (per_cu == nullptr)
> + throw_error (GENERIC_ERROR,
> + _("%s evaluation requires a compilation unit."), op_name);
> +}
> +
> /* See expr.h. */
>
> CORE_ADDR
> @@ -201,6 +215,23 @@ dwarf_expr_context::get_frame_base (const gdb_byte **start,
> start, length);
> }
>
> +/* See expr.h. */
> +
> +struct type *
> +dwarf_expr_context::get_base_type (cu_offset die_cu_off)
> +{
> + if (per_cu == nullptr)
> + return builtin_type (this->gdbarch)->builtin_int;
> +
> + struct type *result = dwarf2_get_die_type (die_cu_off, this->per_cu,
> + this->per_objfile);
> +
> + if (result == nullptr)
> + error (_("Could not find type for operation"));
> +
> + return result;
> +}
The size check disappeared between the two versions, from you reply I
expected that it would stay the same. Why was it removed?
Simon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 16/17] Simplify dwarf_expr_context class interface
2021-05-28 15:46 ` [PATCH v3 16/17] Simplify dwarf_expr_context class interface Zoran Zaric
@ 2021-06-09 1:01 ` Simon Marchi
2021-06-11 17:20 ` Zaric, Zoran (Zare)
0 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2021-06-09 1:01 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches
> diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
> index c2138b31d79..f3985a7ecce 100644
> --- a/gdb/dwarf2/expr.h
> +++ b/gdb/dwarf2/expr.h
> @@ -119,45 +119,49 @@ struct dwarf_stack_value
> its current state and its callbacks. */
> struct dwarf_expr_context
> {
> - dwarf_expr_context (dwarf2_per_objfile *per_objfile);
> + dwarf_expr_context (struct dwarf2_per_objfile *per_objfile,
> + int addr_size);
> virtual ~dwarf_expr_context () = default;
>
> void push_address (CORE_ADDR value, bool in_stack_memory);
> - void eval (const gdb_byte *addr, size_t len);
>
> - /* 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);
> + /* Evaluate the expression at ADDR (LEN bytes long) in a given PER_CU
> + and FRAME context. TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe
> + the expected struct value representation of the evaluation
> + result. The ADDR_INFO property can be specified to override the
Two spaces after period.
Simon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/17] DWARF expression evaluator design cleanup
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
` (16 preceding siblings ...)
2021-05-28 15:46 ` [PATCH v3 17/17] Add as_lval argument to expression evaluator Zoran Zaric
@ 2021-06-09 1:04 ` Simon Marchi
2021-08-05 16:38 ` Zaric, Zoran (Zare)
17 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2021-06-09 1:04 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches
On 2021-05-28 11:46 a.m., Zoran Zaric via Gdb-patches wrote:
> The idea of this patch series is to cleanup the design of the DWARF
> expression evaluator (dwarf_expr_context class) and allow a future
> extensions of that evaluator needed for the upcoming vendor specific
> DWARF extensions.
>
> Main motivation behind this series of patches is AMD’s effort to
> improve DWARF support for heavily optimized code, but more
> specifically, optimized code for SIMD and SIMT architectures.
>
> For the interested parties, the AMD’s DWARF standard extensions
> documentation can be found at:
>
> https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html
>
> That being said, the patch series can also be viewed as a standalone
> series that introduces a welcome cleanup of the DWARF expression
> evaluator module.
>
> Zoran Zaric (17):
> Replace the symbol needs evaluator with a parser
> Cleanup of the dwarf_expr_context constructor
> Move frame context info to dwarf_expr_context
> Remove get_frame_cfa from dwarf_expr_context
> Move compilation unit info to dwarf_expr_context
> Move dwarf_call to dwarf_expr_context
> Move get_object_address to dwarf_expr_context
> Move read_mem to dwarf_expr_context
> Move push_dwarf_reg_entry_value to expr.c
> Inline get_reg_value method of dwarf_expr_context
> Remove empty frame and full evaluators
> Merge evaluate_for_locexpr_baton evaluator
> Move piece_closure and its support to expr.c
> Make value_copy also copy the stack data member
> Make DWARF evaluator return a single struct value
> Simplify dwarf_expr_context class interface
> Add as_lval argument to expression evaluator
>
> gdb/dwarf2/expr.c | 1190 ++++++++++--
> gdb/dwarf2/expr.h | 152 +-
> gdb/dwarf2/frame.c | 117 +-
> gdb/dwarf2/loc.c | 1672 +++++------------
> gdb/dwarf2/loc.h | 30 +-
> gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c | 25 +
> .../gdb.dwarf2/symbol_needs_eval_fail.exp | 112 ++
> .../gdb.dwarf2/symbol_needs_eval_timeout.exp | 131 ++
> .../amd64-py-framefilter-invalidarg.S | 1 -
> gdb/testsuite/lib/dwarf.exp | 4 +
> gdb/value.c | 2 +
> 11 files changed, 1927 insertions(+), 1509 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
> create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
>
Hi Zoran,
I sent some minor comments, just a few nits that need to be fixed or
questions answered, but otherwise this LGTM, it's ok to push once all
these are addressed.
Simon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 03/17] Move frame context info to dwarf_expr_context
2021-06-09 0:45 ` Simon Marchi
@ 2021-06-11 17:16 ` Zaric, Zoran (Zare)
0 siblings, 0 replies; 36+ messages in thread
From: Zaric, Zoran (Zare) @ 2021-06-11 17:16 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 6/9/21 1:45 AM, Simon Marchi wrote:
> [CAUTION: External Email]
>
>> @@ -56,6 +57,31 @@ dwarf_gdbarch_types_init (struct gdbarch *gdbarch)
>> return types;
>> }
>>
>> +/* Ensure that a FRAME is defined, throw an exception otherwise.
>> +
>> + Throwing NOT_AVAILABLE_ERROR error so that a client can chose
>> + to react differently if the evaluation ended because there
>> + was a missing context information. */
>
> This comment needs to be updated (maybe just remove the second
> paragraph).
>
> LGTM with that fixed.
>
> Simon
>
Thank you. This will be fixed in the next iteration.
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context
2021-06-09 0:50 ` Simon Marchi
@ 2021-06-11 17:19 ` Zaric, Zoran (Zare)
0 siblings, 0 replies; 36+ messages in thread
From: Zaric, Zoran (Zare) @ 2021-06-11 17:19 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 6/9/21 1:50 AM, Simon Marchi wrote:
> [CAUTION: External Email]
>
>> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
>> index ced78034fdf..d70f92f27bc 100644
>> --- a/gdb/dwarf2/expr.c
>> +++ b/gdb/dwarf2/expr.c
>> @@ -72,6 +72,20 @@ ensure_have_frame (struct frame_info *frame, const char *op_name)
>> _("%s evaluation requires a frame."), op_name);
>> }
>>
>> +/* Ensure that a PER_CU is defined and throw an exception otherwise.
>> +
>> + Throwing NOT_AVAILABLE_ERROR error so that a client can chose
>> + to react differently if the evaluation ended because there
>> + was a missing context information. */
>
> This comment is also stale.
Thank you. This will be fixed in the next iteration.
>
>> +
>> +static void
>> +ensure_have_per_cu (struct dwarf2_per_cu_data *per_cu, const char* op_name)
>> +{
>> + if (per_cu == nullptr)
>> + throw_error (GENERIC_ERROR,
>> + _("%s evaluation requires a compilation unit."), op_name);
>> +}
>> +
>> /* See expr.h. */
>>
>> CORE_ADDR
>> @@ -201,6 +215,23 @@ dwarf_expr_context::get_frame_base (const gdb_byte **start,
>> start, length);
>> }
>>
>> +/* See expr.h. */
>> +
>> +struct type *
>> +dwarf_expr_context::get_base_type (cu_offset die_cu_off)
>> +{
>> + if (per_cu == nullptr)
>> + return builtin_type (this->gdbarch)->builtin_int;
>> +
>> + struct type *result = dwarf2_get_die_type (die_cu_off, this->per_cu,
>> + this->per_objfile);
>> +
>> + if (result == nullptr)
>> + error (_("Could not find type for operation"));
>> +
>> + return result;
>> +}
>
> The size check disappeared between the two versions, from you reply I
> expected that it would stay the same. Why was it removed?
>
> Simon
>
As soon as the code that handles DW_OP_const_type operation specifics is
moved to where that operation is evaluated, the size check also gets
localized to that part of the code, so the 0 size case doesn't have
meaning anymore.
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 16/17] Simplify dwarf_expr_context class interface
2021-06-09 1:01 ` Simon Marchi
@ 2021-06-11 17:20 ` Zaric, Zoran (Zare)
0 siblings, 0 replies; 36+ messages in thread
From: Zaric, Zoran (Zare) @ 2021-06-11 17:20 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 6/9/21 2:01 AM, Simon Marchi wrote:
> [CAUTION: External Email]
>
>> diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
>> index c2138b31d79..f3985a7ecce 100644
>> --- a/gdb/dwarf2/expr.h
>> +++ b/gdb/dwarf2/expr.h
>> @@ -119,45 +119,49 @@ struct dwarf_stack_value
>> its current state and its callbacks. */
>> struct dwarf_expr_context
>> {
>> - dwarf_expr_context (dwarf2_per_objfile *per_objfile);
>> + dwarf_expr_context (struct dwarf2_per_objfile *per_objfile,
>> + int addr_size);
>> virtual ~dwarf_expr_context () = default;
>>
>> void push_address (CORE_ADDR value, bool in_stack_memory);
>> - void eval (const gdb_byte *addr, size_t len);
>>
>> - /* 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);
>> + /* Evaluate the expression at ADDR (LEN bytes long) in a given PER_CU
>> + and FRAME context. TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET describe
>> + the expected struct value representation of the evaluation
>> + result. The ADDR_INFO property can be specified to override the
>
> Two spaces after period.
>
> Simon
>
Thank you, this will be fixed in the next iteration.
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 01/17] Replace the symbol needs evaluator with a parser
2021-06-07 21:48 ` Simon Marchi
@ 2021-06-11 17:22 ` Zaric, Zoran (Zare)
0 siblings, 0 replies; 36+ messages in thread
From: Zaric, Zoran (Zare) @ 2021-06-11 17:22 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
>
> I forgot to mention above, but I'd also suggest declaring uoffset and
> offset in the specific cases that need it. And you can use
> safe_skip_leb128 to avoid much of their uses.
>
> And the fixup patch:
>
Thank you Simon, I will make sure to incorporate all proposed fixes in
my next iteration
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 00/17] DWARF expression evaluator design cleanup
2021-06-09 1:04 ` [PATCH v3 00/17] DWARF expression evaluator design cleanup Simon Marchi
@ 2021-08-05 16:38 ` Zaric, Zoran (Zare)
0 siblings, 0 replies; 36+ messages in thread
From: Zaric, Zoran (Zare) @ 2021-08-05 16:38 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
[AMD Official Use Only]
>
> Hi Zoran,
>
> I sent some minor comments, just a few nits that need to be fixed or
> questions answered, but otherwise this LGTM, it's ok to push once all these
> are addressed.
>
> Simon
This patch set has been submitted just now.
Thanks for all the reviews guys,
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-05-28 15:46 ` [PATCH v3 15/17] Make DWARF evaluator return a single struct value Zoran Zaric
@ 2021-08-11 17:00 ` Tom Tromey
2021-08-12 18:03 ` Tom Tromey
0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2021-08-11 17:00 UTC (permalink / raw)
To: Zoran Zaric via Gdb-patches
>>>>> "Zoran" == Zoran Zaric via Gdb-patches <gdb-patches@sourceware.org> writes:
Zoran> The patch is addressing the issue of class users writing and reading
Zoran> the internal data of the dwarf_expr_context class.
Zoran> At this point, all conditions are met for the DWARF evaluator to return
Zoran> an evaluation result in a form of a single struct value object.
Hi. On an internal test case, using an arm-elf target, this patch
causes a regression. (It doesn't happen for any of the other cross
targets that I test when importing upstream gdb.)
I don't know if there's an upstream gdb test case showing the same
problem... I can only really run native tests with dejagnu AFAIK.
Anyway the failure manifests like this:
Breakpoint 1, file_1.export_1 (param_1=<error reading variable: Unable to access DWARF register number 64>, str="test_gdb_fpregs") at [...]file_1.adb:5
Whereas when it works it looks like:
Breakpoint 1, file_1.export_1 (param_1=99.0, str=...) at [...]/file_1.adb:5
I think the difference is that the new code does:
Zoran> +/* See expr.h. */
Zoran> +
Zoran> +struct value *
Zoran> +dwarf_expr_context::fetch_result (struct type *type,
Zoran> + struct type *subobj_type,
Zoran> + LONGEST subobj_offset)
Zoran> +{
[...]
Zoran> + case DWARF_VALUE_REGISTER:
Zoran> + {
Zoran> + int dwarf_regnum
Zoran> + = longest_to_int (value_as_long (this->fetch (0)));
Zoran> + int gdb_regnum = dwarf_reg_to_regnum_or_error (this->gdbarch,
Zoran> + dwarf_regnum);
... whereas the old code did:
Zoran> - switch (ctx.location)
Zoran> - {
Zoran> - case DWARF_VALUE_REGISTER:
Zoran> - {
Zoran> - struct gdbarch *arch = get_frame_arch (frame);
Zoran> - int dwarf_regnum
Zoran> - = longest_to_int (value_as_long (ctx.fetch (0)));
Zoran> - int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, dwarf_regnum);
That is, the old code used get_frame_arch, which is different from
this->gdbarch.
Now, it seems to me that when working with registers, the frame's
architecture is the one to use. Other spots seem to use
read_addr_from_reg, which does this same thing.
I'm going to try out that change for a number of cross targets and see
if it looks ok. Meanwhile I thought I'd send this email in case anyone
else has more insight.
thanks,
Tom
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-11 17:00 ` Tom Tromey
@ 2021-08-12 18:03 ` Tom Tromey
2021-08-13 9:57 ` Zoran Zaric
0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2021-08-12 18:03 UTC (permalink / raw)
To: Tom Tromey; +Cc: Zoran Zaric via Gdb-patches, Zoran Zaric
Zoran> At this point, all conditions are met for the DWARF evaluator to return
Zoran> an evaluation result in a form of a single struct value object.
Tom> Hi. On an internal test case, using an arm-elf target, this patch
Tom> causes a regression. (It doesn't happen for any of the other cross
Tom> targets that I test when importing upstream gdb.)
I've tested this patch, plus another one related to this series that I
didn't bring up yesterday.
They are here:
https://sourceware.org/pipermail/gdb-patches/2021-August/181432.html
I'd appreciate any input you might have.
thanks,
Tom
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-12 18:03 ` Tom Tromey
@ 2021-08-13 9:57 ` Zoran Zaric
2021-08-13 16:59 ` Tom Tromey
0 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-08-13 9:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: Zoran Zaric via Gdb-patches
Hi Tom,
Thanks for reporting this, I will look into it ASAP.
I am curious about how can the frame arch and the arch given for the
evaluation of the expression while focused on the same frame be different.
Something suspicious is happening here, but I first need to find a way
to reproduce this.
Zoran
On 8/12/21 7:03 PM, Tom Tromey wrote:
> [CAUTION: External Email]
>
> Zoran> At this point, all conditions are met for the DWARF evaluator to return
> Zoran> an evaluation result in a form of a single struct value object.
>
> Tom> Hi. On an internal test case, using an arm-elf target, this patch
> Tom> causes a regression. (It doesn't happen for any of the other cross
> Tom> targets that I test when importing upstream gdb.)
>
> I've tested this patch, plus another one related to this series that I
> didn't bring up yesterday.
>
> They are here:
>
> https://sourceware.org/pipermail/gdb-patches/2021-August/181432.html
>
> I'd appreciate any input you might have.
>
> thanks,
> Tom
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-13 9:57 ` Zoran Zaric
@ 2021-08-13 16:59 ` Tom Tromey
2021-08-13 17:57 ` Zoran Zaric
0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2021-08-13 16:59 UTC (permalink / raw)
To: Zoran Zaric; +Cc: Tom Tromey, Zoran Zaric via Gdb-patches
Zoran> Thanks for reporting this, I will look into it ASAP.
Take a look at the patches I sent.
Zoran> I am curious about how can the frame arch and the arch given for the
Zoran> evaluation of the expression while focused on the same frame be
Zoran> different.
The arch that's currently used here comes from:
gdbarch *arch = this->m_per_objfile->objfile->arch ();
The objfile's arch is just whatever gdb guesses from the executable.
I think it may be fairly non-specific, especially if the architecture
has a lot of variants. In a case like this, the arch returned by
gdbserver, say, may include more details -- the specific registers
available, etc.
Tom
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-13 16:59 ` Tom Tromey
@ 2021-08-13 17:57 ` Zoran Zaric
2021-08-16 15:37 ` Tom Tromey
0 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-08-13 17:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: Zoran Zaric via Gdb-patches
On 8/13/21 5:59 PM, Tom Tromey wrote:
> [CAUTION: External Email]
>
> Zoran> Thanks for reporting this, I will look into it ASAP.
>
> Take a look at the patches I sent.
Thank you Tom, what you just said makes sense. The patches look good to
me considering that information.
I will need to check (in my later patches) where the architecture is
used and if I have the frame arch I should probably prioritize that.
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-13 17:57 ` Zoran Zaric
@ 2021-08-16 15:37 ` Tom Tromey
2021-08-16 16:05 ` Zoran Zaric
0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2021-08-16 15:37 UTC (permalink / raw)
To: Zoran Zaric; +Cc: Tom Tromey, Zoran Zaric via Gdb-patches
Tom> Take a look at the patches I sent.
Zoran> Thank you Tom, what you just said makes sense. The patches look good
Zoran> to me considering that information.
Thanks. I'm going to check them in now.
Tom
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-16 15:37 ` Tom Tromey
@ 2021-08-16 16:05 ` Zoran Zaric
2021-08-16 17:32 ` Tom Tromey
0 siblings, 1 reply; 36+ messages in thread
From: Zoran Zaric @ 2021-08-16 16:05 UTC (permalink / raw)
To: Tom Tromey; +Cc: Zoran Zaric via Gdb-patches
On 8/16/21 4:37 PM, Tom Tromey wrote:
> [CAUTION: External Email]
>
> Tom> Take a look at the patches I sent.
>
> Zoran> Thank you Tom, what you just said makes sense. The patches look good
> Zoran> to me considering that information.
>
> Thanks. I'm going to check them in now.
>
> Tom
>
These look OK to me, but what I would like to try is to always use the
frame architecture if the frame context is available for the evaluation.
This should make all the location descriptions behave in a consistent way.
Would you mind me adding a patch that does that a bit later? Or do you
see a problem with that implementation?
Regards,
Zoran
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 15/17] Make DWARF evaluator return a single struct value
2021-08-16 16:05 ` Zoran Zaric
@ 2021-08-16 17:32 ` Tom Tromey
0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2021-08-16 17:32 UTC (permalink / raw)
To: Zoran Zaric; +Cc: Tom Tromey, Zoran Zaric via Gdb-patches
Zoran> These look OK to me, but what I would like to try is to always use the
Zoran> frame architecture if the frame context is available for the
Zoran> evaluation. This should make all the location descriptions behave in a
Zoran> consistent way.
Zoran> Would you mind me adding a patch that does that a bit later? Or do you
Zoran> see a problem with that implementation?
I think it would probably be fine. It's hard to be certain -- like, if
there is a situation where an expression only needs read-only memory,
then this could be evaluated without needing a frame (or running
inferior) and so then, in theory, this patch could cause a regression.
But does this case occur and does it matter?
The other thing is, looking at dwarf_expr_context::fetch_result, most
uses of the architecture are basic:
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG)
I suppose it's possible for the objfile arch and the frame arch to
disagree about byte order, but does it happen in practice?
Maybe it would be difficult to test such a patch. Though I suppose I'd
be alright with not having a test case for this particular corner.
Not sure what others may think.
Tom
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2021-08-16 17:32 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28 15:46 [PATCH v3 00/17] DWARF expression evaluator design cleanup Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 01/17] Replace the symbol needs evaluator with a parser Zoran Zaric
2021-06-07 21:48 ` Simon Marchi
2021-06-11 17:22 ` Zaric, Zoran (Zare)
2021-05-28 15:46 ` [PATCH v3 02/17] Cleanup of the dwarf_expr_context constructor Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 03/17] Move frame context info to dwarf_expr_context Zoran Zaric
2021-06-09 0:45 ` Simon Marchi
2021-06-11 17:16 ` Zaric, Zoran (Zare)
2021-05-28 15:46 ` [PATCH v3 04/17] Remove get_frame_cfa from dwarf_expr_context Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 05/17] Move compilation unit info to dwarf_expr_context Zoran Zaric
2021-06-09 0:50 ` Simon Marchi
2021-06-11 17:19 ` Zaric, Zoran (Zare)
2021-05-28 15:46 ` [PATCH v3 06/17] Move dwarf_call " Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 07/17] Move get_object_address " Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 08/17] Move read_mem " Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 09/17] Move push_dwarf_reg_entry_value to expr.c Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 10/17] Inline get_reg_value method of dwarf_expr_context Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 11/17] Remove empty frame and full evaluators Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 12/17] Merge evaluate_for_locexpr_baton evaluator Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 13/17] Move piece_closure and its support to expr.c Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 14/17] Make value_copy also copy the stack data member Zoran Zaric
2021-05-28 15:46 ` [PATCH v3 15/17] Make DWARF evaluator return a single struct value Zoran Zaric
2021-08-11 17:00 ` Tom Tromey
2021-08-12 18:03 ` Tom Tromey
2021-08-13 9:57 ` Zoran Zaric
2021-08-13 16:59 ` Tom Tromey
2021-08-13 17:57 ` Zoran Zaric
2021-08-16 15:37 ` Tom Tromey
2021-08-16 16:05 ` Zoran Zaric
2021-08-16 17:32 ` Tom Tromey
2021-05-28 15:46 ` [PATCH v3 16/17] Simplify dwarf_expr_context class interface Zoran Zaric
2021-06-09 1:01 ` Simon Marchi
2021-06-11 17:20 ` Zaric, Zoran (Zare)
2021-05-28 15:46 ` [PATCH v3 17/17] Add as_lval argument to expression evaluator Zoran Zaric
2021-06-09 1:04 ` [PATCH v3 00/17] DWARF expression evaluator design cleanup Simon Marchi
2021-08-05 16:38 ` Zaric, Zoran (Zare)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).