diff mbox

[4/9] Remove addr_size field from struct piece_closure

Message ID 1491586736-21296-5-git-send-email-arnez@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez April 7, 2017, 5:38 p.m. UTC 
  The addr_size field in the piece_closure data structure is a relic from
before introducing the typed DWARF stack.  It is obsolete now.  This patch
removes it.

gdb/ChangeLog:

	* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
	(allocate_piece_closure): Drop addr_size parameter.
	(dwarf2_evaluate_loc_desc_full): Adjust call to
	allocate_piece_closure.
---
 gdb/dwarf2loc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Yao Qi April 13, 2017, 9:10 a.m. UTC | #1 
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> The addr_size field in the piece_closure data structure is a relic from
> before introducing the typed DWARF stack.  It is obsolete now.  This patch
> removes it.
>
> gdb/ChangeLog:
>
> 	* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
> 	(allocate_piece_closure): Drop addr_size parameter.
> 	(dwarf2_evaluate_loc_desc_full): Adjust call to
> 	allocate_piece_closure.

LGTM.
Simon Marchi April 14, 2017, 3:38 a.m. UTC | #2 
On 2017-04-13 05:10, Yao Qi wrote:
> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> 
>> The addr_size field in the piece_closure data structure is a relic 
>> from
>> before introducing the typed DWARF stack.  It is obsolete now.  This 
>> patch
>> removes it.
>> 
>> gdb/ChangeLog:
>> 
>> 	* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
>> 	(allocate_piece_closure): Drop addr_size parameter.
>> 	(dwarf2_evaluate_loc_desc_full): Adjust call to
>> 	allocate_piece_closure.
> 
> LGTM.

Since you are planning on making a v2, I think it's better if you check 
it in right now to get it out of the way (it could be obvious anyway).

Simon
Andreas Arnez April 18, 2017, 5:24 p.m. UTC | #3 
On Thu, Apr 13 2017, Simon Marchi wrote:

> On 2017-04-13 05:10, Yao Qi wrote:
>> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>>
>>> The addr_size field in the piece_closure data structure is a relic from
>>> before introducing the typed DWARF stack.  It is obsolete now.  This
>>> patch
>>> removes it.
>>>
>>> gdb/ChangeLog:
>>>
>>> 	* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
>>> 	(allocate_piece_closure): Drop addr_size parameter.
>>> 	(dwarf2_evaluate_loc_desc_full): Adjust call to
>>> 	allocate_piece_closure.
>>
>> LGTM.
>
> Since you are planning on making a v2, I think it's better if you check it
> in right now to get it out of the way (it could be obvious anyway).

I would, but it depends on the patch before -- "[PATCH 3/9] PR
gdb/21226: Take DWARF stack value pieces from LSB end".

(Maybe it wasn't clear, but the current code for handling DWARF stack
values is slightly more broken than just not taking them from the
correct end.  It also uses the relic "addr_size" instead of the actual
DWARF stack value size to determine whether the piece is contained in
its underlying object.  And if it's *partially* contained, like taking a
1MB piece from a 64-bit stack value, the current code accesses invalid
memory anyway.)

--
Andreas
Simon Marchi April 18, 2017, 6:49 p.m. UTC | #4 
On 2017-04-18 13:24, Andreas Arnez wrote:
>> Since you are planning on making a v2, I think it's better if you 
>> check it
>> in right now to get it out of the way (it could be obvious anyway).
> 
> I would, but it depends on the patch before -- "[PATCH 3/9] PR
> gdb/21226: Take DWARF stack value pieces from LSB end".
> 
> (Maybe it wasn't clear, but the current code for handling DWARF stack
> values is slightly more broken than just not taking them from the
> correct end.  It also uses the relic "addr_size" instead of the actual
> DWARF stack value size to determine whether the piece is contained in
> its underlying object.  And if it's *partially* contained, like taking 
> a
> 1MB piece from a 64-bit stack value, the current code accesses invalid
> memory anyway.)

Ah yes indeed, my bad.
diff mbox

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 09938c4..76a58a3 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1485,9 +1485,6 @@  struct piece_closure
   /* The number of pieces used to describe this variable.  */
   int n_pieces;
 
-  /* The target address size, used only for DWARF_VALUE_STACK.  */
-  int addr_size;
-
   /* The pieces themselves.  */
   struct dwarf_expr_piece *pieces;
 
@@ -1502,7 +1499,7 @@  struct piece_closure
 static struct piece_closure *
 allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
 			int n_pieces, struct dwarf_expr_piece *pieces,
-			int addr_size, struct frame_info *frame)
+			struct frame_info *frame)
 {
   struct piece_closure *c = XCNEW (struct piece_closure);
   int i;
@@ -1510,7 +1507,6 @@  allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
   c->refc = 1;
   c->per_cu = per_cu;
   c->n_pieces = n_pieces;
-  c->addr_size = addr_size;
   c->pieces = XCNEWVEC (struct dwarf_expr_piece, n_pieces);
   if (frame == NULL)
     c->frame_id = null_frame_id;
@@ -2412,7 +2408,7 @@  dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	invalid_synthetic_pointer ();
 
       c = allocate_piece_closure (per_cu, ctx.num_pieces, ctx.pieces,
-				  ctx.addr_size, frame);
+				  frame);
       /* We must clean up the value chain after creating the piece
 	 closure but before allocating the result.  */
       free_values.free_to_mark ();