From b249d2c2c01775fb015b38b272389b8693e414f6 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 24 Apr 2020 13:40:31 -0600 Subject: [PATCH] Prefer existing data when evaluating DWARF expression When evaluating a DWARF expression, the dynamic type resolution code will pass in a buffer of bytes via the property_addr_info. However, the DWARF expression evaluator will then proceed to read memory from the inferior, even when the request could be filled from this buffer. This, in turn, is a problem in some cases; and specifically when trying to handle the Ada scenario of extracting a variable-length value from a packed array. Here, the ordinary DWARF expression cannot be directly evaluated, because the data may appear at some arbitrary bit offset. So, it is unpacked into a staging area and then the expression is evaluated -- using an address of 0. This patch fixes the problem by arranging for the DWARF evaluator, in this case, to prefer passed-in memory when possible. The type of the buffer in the property_addr_info is changed to an array_view so that bounds checking can be done. gdb/ChangeLog 2020-04-24 Tom Tromey * ada-lang.c (ada_discrete_type_high_bound, ada_discrete_type_low) (ada_value_primitive_packed_val): Update. * ada-valprint.c (ada_value_print_1): Update. * dwarf2/loc.c (evaluate_for_locexpr_baton): New struct. (dwarf2_locexpr_baton_eval): Take a property_addr_info rather than just an address. Use evaluate_for_locexpr_baton. (dwarf2_evaluate_property): Update. * dwarf2/loc.h (struct property_addr_info) : Now an array_view. * findvar.c (default_read_var_value): Update. * gdbtypes.c (compute_variant_fields_inner) (resolve_dynamic_type_internal): Update. (resolve_dynamic_type): Change type of valaddr parameter. * gdbtypes.h (resolve_dynamic_type): Update. * valarith.c (value_subscripted_rvalue): Update. * value.c (value_from_contents_and_address): Update. --- gdb/ChangeLog | 19 ++++++++++++ gdb/ada-lang.c | 6 ++-- gdb/ada-valprint.c | 4 ++- gdb/dwarf2/loc.c | 73 ++++++++++++++++++++++++++++++++++++---------- gdb/dwarf2/loc.h | 2 +- gdb/findvar.c | 6 ++-- gdb/gdbtypes.c | 15 ++++++---- gdb/gdbtypes.h | 6 ++-- gdb/valarith.c | 2 +- gdb/value.c | 5 +++- 10 files changed, 104 insertions(+), 34 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5db26a6094..305b133b4f 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,22 @@ +2020-04-24 Tom Tromey + + * ada-lang.c (ada_discrete_type_high_bound, ada_discrete_type_low) + (ada_value_primitive_packed_val): Update. + * ada-valprint.c (ada_value_print_1): Update. + * dwarf2/loc.c (evaluate_for_locexpr_baton): New struct. + (dwarf2_locexpr_baton_eval): Take a property_addr_info rather than + just an address. Use evaluate_for_locexpr_baton. + (dwarf2_evaluate_property): Update. + * dwarf2/loc.h (struct property_addr_info) : Now an + array_view. + * findvar.c (default_read_var_value): Update. + * gdbtypes.c (compute_variant_fields_inner) + (resolve_dynamic_type_internal): Update. + (resolve_dynamic_type): Change type of valaddr parameter. + * gdbtypes.h (resolve_dynamic_type): Update. + * valarith.c (value_subscripted_rvalue): Update. + * value.c (value_from_contents_and_address): Update. + 2020-04-24 Tom Tromey * dwarf2/loc.c (dwarf2_locexpr_baton_eval): Add diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 49f2280198..bfbc69084e 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -752,7 +752,7 @@ min_of_type (struct type *t) LONGEST ada_discrete_type_high_bound (struct type *type) { - type = resolve_dynamic_type (type, NULL, 0); + type = resolve_dynamic_type (type, {}, 0); switch (TYPE_CODE (type)) { case TYPE_CODE_RANGE: @@ -773,7 +773,7 @@ ada_discrete_type_high_bound (struct type *type) LONGEST ada_discrete_type_low_bound (struct type *type) { - type = resolve_dynamic_type (type, NULL, 0); + type = resolve_dynamic_type (type, {}, 0); switch (TYPE_CODE (type)) { case TYPE_CODE_RANGE: @@ -2508,7 +2508,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr, staging.data (), staging.size (), is_big_endian, has_negatives (type), is_scalar); - type = resolve_dynamic_type (type, staging.data (), 0); + type = resolve_dynamic_type (type, staging, 0); if (TYPE_LENGTH (type) < (bit_size + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT) { /* This happens when the length of the object is dynamic, diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c index 2768829cdb..474b079991 100644 --- a/gdb/ada-valprint.c +++ b/gdb/ada-valprint.c @@ -1057,7 +1057,9 @@ ada_value_print_1 (struct value *val, struct ui_file *stream, int recurse, const gdb_byte *valaddr = value_contents_for_printing (val); CORE_ADDR address = value_address (val); - type = ada_check_typedef (resolve_dynamic_type (type, valaddr, address)); + gdb::array_view view + = gdb::make_array_view (valaddr, TYPE_LENGTH (type)); + type = ada_check_typedef (resolve_dynamic_type (type, view, address)); if (type != saved_type) { val = value_copy (val); diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index 8df655f660..5b690ca927 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -2384,18 +2384,56 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame, NULL, 0); } -/* 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 evaluated. ADDR is a context (location of - a variable) and might be needed to evaluate the location expression. - PUSH_INITIAL_VALUE is true if ADDR should be pushed on the stack - before evaluating the expression; this is required by certain - forms of DWARF expression. Returns 1 on success, 0 otherwise. */ +/* A specialization of dwarf_evaluate_loc_desc 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 +{ + /* The data that was passed in. */ + gdb::array_view 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 + evaluated. ADDR_STACK is a context (location of a variable) and + might be needed to evaluate the location expression. + PUSH_INITIAL_VALUE is true if the address (either from ADDR_STACK, + or the default of 0) should be pushed on the DWARF expression + evaluation stack before evaluating the expression; this is required + by certain forms of DWARF expression. Returns 1 on success, 0 + otherwise. */ static int dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton, struct frame_info *frame, - CORE_ADDR addr, + const struct property_addr_info *addr_stack, CORE_ADDR *valp, bool push_initial_value) { @@ -2404,11 +2442,17 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton, if (dlbaton == NULL || dlbaton->size == 0) return 0; - dwarf_evaluate_loc_desc ctx; + evaluate_for_locexpr_baton ctx; ctx.frame = frame; ctx.per_cu = dlbaton->per_cu; - ctx.obj_address = addr; + if (addr_stack == nullptr) + ctx.obj_address = 0; + else + { + ctx.obj_address = addr_stack->addr; + ctx.data_view = addr_stack->valaddr; + } objfile = dlbaton->per_cu->objfile (); @@ -2418,7 +2462,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton, ctx.offset = dlbaton->per_cu->text_offset (); if (push_initial_value) - ctx.push_address (addr, false); + ctx.push_address (ctx.obj_address, false); try { @@ -2485,8 +2529,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop, = (const struct dwarf2_property_baton *) prop->data.baton; gdb_assert (baton->property_type != NULL); - if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame, - addr_stack ? addr_stack->addr : 0, + if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame, addr_stack, value, push_initial_value)) { if (baton->locexpr.is_reference) @@ -2569,10 +2612,10 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop, } if (pinfo == NULL) error (_("cannot find reference address for offset property")); - if (pinfo->valaddr != NULL) + if (pinfo->valaddr.data () != NULL) val = value_from_contents (baton->offset_info.type, - pinfo->valaddr + baton->offset_info.offset); + pinfo->valaddr.data () + baton->offset_info.offset); else val = value_at (baton->offset_info.type, pinfo->addr + baton->offset_info.offset); diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h index 6ff9b79dc0..9815368d62 100644 --- a/gdb/dwarf2/loc.h +++ b/gdb/dwarf2/loc.h @@ -73,7 +73,7 @@ struct property_addr_info struct type *type; /* If not NULL, a buffer containing the object's value. */ - const gdb_byte *valaddr; + gdb::array_view valaddr; /* The address of that object. */ CORE_ADDR addr; diff --git a/gdb/findvar.c b/gdb/findvar.c index a836c63dc5..ac4f5c3997 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -615,7 +615,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block, if (is_dynamic_type (type)) { /* Value is a constant byte-sequence and needs no memory access. */ - type = resolve_dynamic_type (type, NULL, /* Unused address. */ 0); + type = resolve_dynamic_type (type, {}, /* Unused address. */ 0); } /* Put the constant back in target format. */ v = allocate_value (type); @@ -647,7 +647,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block, if (is_dynamic_type (type)) { /* Value is a constant byte-sequence and needs no memory access. */ - type = resolve_dynamic_type (type, NULL, /* Unused address. */ 0); + type = resolve_dynamic_type (type, {}, /* Unused address. */ 0); } v = allocate_value (type); memcpy (value_contents_raw (v), SYMBOL_VALUE_BYTES (var), @@ -788,7 +788,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block, case LOC_OPTIMIZED_OUT: if (is_dynamic_type (type)) - type = resolve_dynamic_type (type, NULL, /* Unused address. */ 0); + type = resolve_dynamic_type (type, {}, /* Unused address. */ 0); return allocate_optimized_out_value (type); default: diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index c2a1d22137..1bd9d8a55c 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -2298,8 +2298,9 @@ compute_variant_fields_inner (struct type *type, error (_("Cannot determine struct field location" " (invalid location kind)")); - if (addr_stack->valaddr != NULL) - discr_value = unpack_field_as_long (type, addr_stack->valaddr, idx); + if (addr_stack->valaddr.data () != NULL) + discr_value = unpack_field_as_long (type, addr_stack->valaddr.data (), + idx); else { CORE_ADDR addr = (addr_stack->addr @@ -2516,9 +2517,10 @@ resolve_dynamic_type_internal (struct type *type, struct property_addr_info pinfo; pinfo.type = check_typedef (TYPE_TARGET_TYPE (type)); - pinfo.valaddr = NULL; - if (addr_stack->valaddr != NULL) - pinfo.addr = extract_typed_address (addr_stack->valaddr, type); + pinfo.valaddr = {}; + if (addr_stack->valaddr.data () != NULL) + pinfo.addr = extract_typed_address (addr_stack->valaddr.data (), + type); else pinfo.addr = read_memory_typed_address (addr_stack->addr, type); pinfo.next = addr_stack; @@ -2566,7 +2568,8 @@ resolve_dynamic_type_internal (struct type *type, /* See gdbtypes.h */ struct type * -resolve_dynamic_type (struct type *type, const gdb_byte *valaddr, +resolve_dynamic_type (struct type *type, + gdb::array_view valaddr, CORE_ADDR addr) { struct property_addr_info pinfo diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 8b4da6e3f9..f686e5407b 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -2140,9 +2140,9 @@ extern void get_signed_type_minmax (struct type *, LONGEST *, LONGEST *); ADDR specifies the location of the variable the type is bound to. If TYPE has no dynamic properties return TYPE; otherwise a new type with static properties is returned. */ -extern struct type *resolve_dynamic_type (struct type *type, - const gdb_byte *valaddr, - CORE_ADDR addr); +extern struct type *resolve_dynamic_type + (struct type *type, gdb::array_view valaddr, + CORE_ADDR addr); /* * Predicate if the type has dynamic values, which are not resolved yet. */ extern int is_dynamic_type (struct type *type); diff --git a/gdb/valarith.c b/gdb/valarith.c index 07cb5014bb..504264b1d8 100644 --- a/gdb/valarith.c +++ b/gdb/valarith.c @@ -220,7 +220,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound CORE_ADDR address; address = value_address (array) + elt_offs; - elt_type = resolve_dynamic_type (elt_type, NULL, address); + elt_type = resolve_dynamic_type (elt_type, {}, address); } return value_from_component (array, elt_type, elt_offs); diff --git a/gdb/value.c b/gdb/value.c index 5ae0b32f4f..7ea39af555 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -3477,7 +3477,10 @@ value_from_contents_and_address (struct type *type, const gdb_byte *valaddr, CORE_ADDR address) { - struct type *resolved_type = resolve_dynamic_type (type, valaddr, address); + gdb::array_view view; + if (valaddr != nullptr) + view = gdb::make_array_view (valaddr, TYPE_LENGTH (type)); + struct type *resolved_type = resolve_dynamic_type (type, view, address); struct type *resolved_type_no_typedef = check_typedef (resolved_type); struct value *v;