From d185219da329805075ba5e0e72ec4c89c925cff2 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 14 Sep 2017 22:36:57 +0200 Subject: [PATCH] Make dwarf_expr_context::stack an std::vector Replace the manually managed array with a vector. It is mostly straightforward, except maybe one thing in execute_stack_op, in the handling of DW_OP_fbreg. When the code stumbles on that opcode while evaluating an expression, it needs to evaluate a subexpression to find where the fb reg has been saved. Rather than creating a new context, it reuses the current context. It saves the size of the stack before and restores the stack to that size after. I think we can do a little bit better by saving the current stack locally and installing a new empty stack. This way, if the subexpression is malformed and underflows, we'll get an exception. Before, it would have overwritten the top elements of the top-level expression. The evaluation of the top-level expression would have then resumed with the same stack size, but possibly some corrupted elements. gdb/ChangeLog: * dwarf2expr.h (dwarf_stack_value): Add constructor. (dwarf_expr_context) <~dwarf_expr_context>: Define as default. : Change type to std::vector. : Remove. : Remove. * dwarf2expr.c (dwarf_expr_context::dwarf_expr_context): Adjust. (dwarf_expr_context::~dwarf_expr_context): Remove. (dwarf_expr_context::grow_stack): Remove. (dwarf_expr_context::push): Adjust. (dwarf_expr_context::pop): Adjust. (dwarf_expr_context::fetch): Adjust. (dwarf_expr_context::fetch_in_stack_memory): Adjust. (dwarf_expr_context::stack_empty_p): Adjust. (dwarf_expr_context::execute_stack_op): Adjust. --- gdb/ChangeLog | 17 ++++++++ gdb/dwarf2expr.c | 110 +++++++++++++++++------------------------------ gdb/dwarf2expr.h | 15 +++---- 3 files changed, 64 insertions(+), 78 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f88cd6e1a45..c562f7b7207 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,20 @@ +2017-09-14 Simon Marchi + + * dwarf2expr.h (dwarf_stack_value): Add constructor. + (dwarf_expr_context) <~dwarf_expr_context>: Define as default. + : Change type to std::vector. + : Remove. + : Remove. + * dwarf2expr.c (dwarf_expr_context::dwarf_expr_context): Adjust. + (dwarf_expr_context::~dwarf_expr_context): Remove. + (dwarf_expr_context::grow_stack): Remove. + (dwarf_expr_context::push): Adjust. + (dwarf_expr_context::pop): Adjust. + (dwarf_expr_context::fetch): Adjust. + (dwarf_expr_context::fetch_in_stack_memory): Adjust. + (dwarf_expr_context::stack_empty_p): Adjust. + (dwarf_expr_context::execute_stack_op): Adjust. + 2017-09-14 Simon Marchi * dwarf2expr.h (dwarf_expr_context) : Change diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c index 3a388ac6fa1..f5e0e4c1fdb 100644 --- a/gdb/dwarf2expr.c +++ b/gdb/dwarf2expr.c @@ -88,10 +88,7 @@ dwarf_expr_context::address_type () const /* Create a new context for the expression evaluator. */ dwarf_expr_context::dwarf_expr_context () -: stack (NULL), - stack_len (0), - stack_allocated (10), - gdbarch (NULL), +: gdbarch (NULL), addr_size (0), ref_addr_size (0), offset (0), @@ -102,29 +99,6 @@ dwarf_expr_context::dwarf_expr_context () data (NULL), initialized (0) { - this->stack = XNEWVEC (struct dwarf_stack_value, this->stack_allocated); -} - -/* Clean up a dwarf_expr_context. */ - -dwarf_expr_context::~dwarf_expr_context () -{ - xfree (this->stack); -} - -/* Expand the memory allocated stack to contain at least - NEED more elements than are currently used. */ - -void -dwarf_expr_context::grow_stack (size_t need) -{ - if (this->stack_len + need > this->stack_allocated) - { - size_t newlen = this->stack_len + need + 10; - - this->stack = XRESIZEVEC (struct dwarf_stack_value, this->stack, newlen); - this->stack_allocated = newlen; - } } /* Push VALUE onto the stack. */ @@ -132,12 +106,7 @@ dwarf_expr_context::grow_stack (size_t need) void dwarf_expr_context::push (struct value *value, bool in_stack_memory) { - struct dwarf_stack_value *v; - - grow_stack (1); - v = &this->stack[this->stack_len++]; - v->value = value; - v->in_stack_memory = in_stack_memory; + stack.emplace_back (value, in_stack_memory); } /* Push VALUE onto the stack. */ @@ -153,9 +122,10 @@ dwarf_expr_context::push_address (CORE_ADDR value, bool in_stack_memory) void dwarf_expr_context::pop () { - if (this->stack_len <= 0) + if (stack.empty ()) error (_("dwarf expression stack underflow")); - this->stack_len--; + + stack.pop_back (); } /* Retrieve the N'th item on the stack. */ @@ -163,11 +133,11 @@ dwarf_expr_context::pop () struct value * dwarf_expr_context::fetch (int n) { - if (this->stack_len <= n) + if (stack.size () <= n) error (_("Asked for position %d of stack, " - "stack only has %d elements on it."), - n, this->stack_len); - return this->stack[this->stack_len - (1 + n)].value; + "stack only has %zu elements on it."), + n, stack.size ()); + return stack[stack.size () - (1 + n)].value; } /* Require that TYPE be an integral type; throw an exception if not. */ @@ -263,11 +233,11 @@ dwarf_expr_context::fetch_address (int n) bool dwarf_expr_context::fetch_in_stack_memory (int n) { - if (this->stack_len <= n) + if (stack.size () <= n) error (_("Asked for position %d of stack, " - "stack only has %d elements on it."), - n, this->stack_len); - return this->stack[this->stack_len - (1 + n)].in_stack_memory; + "stack only has %zu elements on it."), + n, stack.size ()); + return stack[stack.size () - (1 + n)].in_stack_memory; } /* Return true if the expression stack is empty. */ @@ -275,7 +245,7 @@ dwarf_expr_context::fetch_in_stack_memory (int n) bool dwarf_expr_context::stack_empty_p () const { - return this->stack_len == 0; + return stack.empty (); } /* Add a new piece to the dwarf_expr_context's piece list. */ @@ -875,14 +845,16 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, { const gdb_byte *datastart; size_t datalen; - unsigned int before_stack_len; op_ptr = safe_read_sleb128 (op_ptr, op_end, &offset); + /* Rather than create a whole new context, we simply - record the stack length before execution, then reset it - afterwards, effectively erasing whatever the recursive - call put there. */ - before_stack_len = this->stack_len; + 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 saved_stack = std::move (stack); + 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. */ @@ -898,7 +870,10 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, result = result + offset; result_val = value_from_ulongest (address_type, result); in_stack_memory = true; - this->stack_len = before_stack_len; + + /* Restore the content of the original stack. */ + stack = std::move (saved_stack); + this->location = DWARF_VALUE_MEMORY; } break; @@ -920,16 +895,14 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, case DW_OP_swap: { - struct dwarf_stack_value t1, t2; - - if (this->stack_len < 2) + if (stack.size () < 2) error (_("Not enough elements for " - "DW_OP_swap. Need 2, have %d."), - this->stack_len); - t1 = this->stack[this->stack_len - 1]; - t2 = this->stack[this->stack_len - 2]; - this->stack[this->stack_len - 1] = t2; - this->stack[this->stack_len - 2] = t1; + "DW_OP_swap. Need 2, have %zu."), + stack.size ()); + + dwarf_stack_value &t1 = stack[stack.size () - 1]; + dwarf_stack_value &t2 = stack[stack.size () - 2]; + std::swap (t1, t2); goto no_push; } @@ -940,18 +913,15 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, case DW_OP_rot: { - struct dwarf_stack_value t1, t2, t3; - - if (this->stack_len < 3) + if (stack.size () < 3) error (_("Not enough elements for " - "DW_OP_rot. Need 3, have %d."), - this->stack_len); - t1 = this->stack[this->stack_len - 1]; - t2 = this->stack[this->stack_len - 2]; - t3 = this->stack[this->stack_len - 3]; - this->stack[this->stack_len - 1] = t2; - this->stack[this->stack_len - 2] = t3; - this->stack[this->stack_len - 3] = t1; + "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; goto no_push; } diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index 0a57beef915..a8d6ae111dc 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -100,6 +100,10 @@ struct dwarf_expr_piece struct dwarf_stack_value { + dwarf_stack_value (struct value *value_, int in_stack_memory_) + : value (value_), in_stack_memory (in_stack_memory_) + {} + struct value *value; /* True if the piece is in memory and is known to be on the program's stack. @@ -114,7 +118,7 @@ struct dwarf_stack_value struct dwarf_expr_context { dwarf_expr_context (); - virtual ~dwarf_expr_context (); + virtual ~dwarf_expr_context () = default; void push_address (CORE_ADDR value, bool in_stack_memory); void eval (const gdb_byte *addr, size_t len); @@ -122,12 +126,8 @@ struct dwarf_expr_context CORE_ADDR fetch_address (int n); bool fetch_in_stack_memory (int n); - /* The stack of values, allocated with xmalloc. */ - struct dwarf_stack_value *stack; - - /* The number of values currently pushed on the stack, and the - number of elements allocated to the stack. */ - int stack_len, stack_allocated; + /* The stack of values. */ + std::vector stack; /* Target architecture to use for address operations. */ struct gdbarch *gdbarch; @@ -249,7 +249,6 @@ struct dwarf_expr_context private: struct type *address_type () const; - void grow_stack (size_t need); void push (struct value *value, bool in_stack_memory); bool stack_empty_p () const; void add_piece (ULONGEST size, ULONGEST offset); -- 2.30.2