interface to partial support for DW_OP_piece in dwarf2expr.[ch]

interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Full support for the Dwarf DW_OP_piece operator in location
expressions would require major work on 'struct value' and its users.
But in many cases, compilers use DW_OP_piece in a restricted way, such
that all the multi-piece locations actually generated could be
expressed using the 'struct symbol' and 'struct value' we have today.

For example, on the PowerPC E500, the 32-bit upper and lower halves of
the 64-bit gprs have separate register numbers in Dwarf information,
but GDB has (pseudo-)registers that refer to the entire 64 bits.  So a
Dwarf 3 location expression that places one piece in an upper-half
register and another piece in the corresponding lower-half register
could be represented by a 'struct value' V where VALUE_LVAL (V) ==
lval_register and VALUE_REGNO (V) is the pseudo-register number.

Recognizing these cases requires calling out to architecture-specific
code.  When an patch implementing this was first posted, there was
general agreement that dwarf2expr.[ch] ought not call architecture
methods directly, as it did in the patch, but should instead return
the list of pieces; and that dwarf2expr.[ch]'s callers (specifically,
the code in dwarf2loc.c) should take care of whatever wrangling was
needed to turn that list of pieces into the right 'struct value',
architecture-specific or otherwise.  That thread starts here:

    http://sources.redhat.com/ml/gdb-patches/2003-05/msg00425.html

Here's a sketch of how we might extend the Dwarf expression evaluation
interface to return piece lists, meant to implement the suggestions in
that thread.  If this looks reasonable, then I'll do some more work
and post it for further review.



*** dwarf2expr.h.~1.5.~	2003-06-08 13:27:13.000000000 -0500
--- dwarf2expr.h	2004-08-03 02:07:08.000000000 -0500
*************** struct dwarf_expr_context
*** 74,79 ****
--- 74,122 ----
    /* Non-zero if the result is in a register.  The register number
       will be on the expression stack.  */
    int in_reg;
+ 
+   /* An array of pieces.  PIECES points to its first element;
+      NUM_PIECES is its length.
+ 
+      Each time DW_OP_piece is executed, we add a new element to the
+      end of this array, recording the current top of the stack, the
+      current in_reg flag, and the size given as the operand to
+      DW_OP_piece.  We then pop the top value from the stack, clear the
+      in_reg flag, and resume evaluation.
+ 
+      The Dwarf spec doesn't say whether DW_OP_piece pops the top value
+      from the stack.  We do, ensuring that clients of this interface
+      expecting to see a value left on the top of the stack (say, code
+      evaluating frame base expressions or CFA's specified with
+      DW_CFA_def_cfa_expression) will get an error if the expression
+      actually marks all the values it computes as pieces.
+ 
+      If an expression never uses DW_OP_piece, num_pieces will be zero.
+      (It would be nice to present these cases as expressions yeilding
+      a single piece, with in_reg clear, so that callers need not
+      distinguish between the no-DW_OP_piece and one-DW_OP_piece cases.
+      But expressions with 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.)  */
+   int num_pieces;
+   struct dwarf_expr_piece *pieces;
+ };
+ 
+ 
+ /* A piece of an object, as recorded by DW_OP_piece.  */
+ struct dwarf_expr_piece
+ {
+   /* If IN_REG is zero, then the piece is in memory, and VALUE is its address.
+      If IN_REG is non-zero, then the piece is in a register, and VALUE
+      is the register number.  */
+   int in_reg;
+ 
+   /* This piece's address or register number.  */
+   CORE_ADDR value;
+ 
+   /* The length of the piece, in bytes.  */
+   ULONGEST size;
  };
  
  struct dwarf_expr_context *new_dwarf_expr_context (void);

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Andrew Cagney]

> Full support for the Dwarf DW_OP_piece operator in location
> expressions would require major work on 'struct value' and its users.
> But in many cases, compilers use DW_OP_piece in a restricted way, such
> that all the multi-piece locations actually generated could be
> expressed using the 'struct symbol' and 'struct value' we have today.
> 
> For example, on the PowerPC E500, the 32-bit upper and lower halves of
> the 64-bit gprs have separate register numbers in Dwarf information,
> but GDB has (pseudo-)registers that refer to the entire 64 bits.  So a
> Dwarf 3 location expression that places one piece in an upper-half
> register and another piece in the corresponding lower-half register
> could be represented by a 'struct value' V where VALUE_LVAL (V) ==
> lval_register and VALUE_REGNO (V) is the pseudo-register number.
> 
> Recognizing these cases requires calling out to architecture-specific
> code.  When an patch implementing this was first posted, there was
> general agreement that dwarf2expr.[ch] ought not call architecture
> methods directly, as it did in the patch, but should instead return
> the list of pieces; and that dwarf2expr.[ch]'s callers (specifically,
> the code in dwarf2loc.c) should take care of whatever wrangling was
> needed to turn that list of pieces into the right 'struct value',
> architecture-specific or otherwise.  That thread starts here:
> 
>     http://sources.redhat.com/ml/gdb-patches/2003-05/msg00425.html
> 
> Here's a sketch of how we might extend the Dwarf expression evaluation
> interface to return piece lists, meant to implement the suggestions in
> that thread.  If this looks reasonable, then I'll do some more work
> and post it for further review.

Knowing that GCC 3.5 is going to be a heavy user of DW_OP_piece, means 
that we need to start moving on this, and its good to see this happening.

I'm just having trouble getting my head around how this will affect 
core-gdb, and seeing how this addresses our need to meet GCC 3.5's 
requirements?

Can we expand on that?

--

Looking at the below:

- a single value can have multiple locations (important for store)
- could dwarf2_expr_piece be called ``struct location''? :-)

Andrew

> *** dwarf2expr.h.~1.5.~	2003-06-08 13:27:13.000000000 -0500
> --- dwarf2expr.h	2004-08-03 02:07:08.000000000 -0500
> *************** struct dwarf_expr_context
> *** 74,79 ****
> --- 74,122 ----
>     /* Non-zero if the result is in a register.  The register number
>        will be on the expression stack.  */
>     int in_reg;
> + 
> +   /* An array of pieces.  PIECES points to its first element;
> +      NUM_PIECES is its length.
> + 
> +      Each time DW_OP_piece is executed, we add a new element to the
> +      end of this array, recording the current top of the stack, the
> +      current in_reg flag, and the size given as the operand to
> +      DW_OP_piece.  We then pop the top value from the stack, clear the
> +      in_reg flag, and resume evaluation.
> + 
> +      The Dwarf spec doesn't say whether DW_OP_piece pops the top value
> +      from the stack.  We do, ensuring that clients of this interface
> +      expecting to see a value left on the top of the stack (say, code
> +      evaluating frame base expressions or CFA's specified with
> +      DW_CFA_def_cfa_expression) will get an error if the expression
> +      actually marks all the values it computes as pieces.
> + 
> +      If an expression never uses DW_OP_piece, num_pieces will be zero.
> +      (It would be nice to present these cases as expressions yeilding
> +      a single piece, with in_reg clear, so that callers need not
> +      distinguish between the no-DW_OP_piece and one-DW_OP_piece cases.
> +      But expressions with 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.)  */
> +   int num_pieces;
> +   struct dwarf_expr_piece *pieces;
> + };
> + 
> + 
> + /* A piece of an object, as recorded by DW_OP_piece.  */
> + struct dwarf_expr_piece
> + {
> +   /* If IN_REG is zero, then the piece is in memory, and VALUE is its address.
> +      If IN_REG is non-zero, then the piece is in a register, and VALUE
> +      is the register number.  */
> +   int in_reg;
> + 
> +   /* This piece's address or register number.  */
> +   CORE_ADDR value;
> + 
> +   /* The length of the piece, in bytes.  */
> +   ULONGEST size;
>   };
>   
>   struct dwarf_expr_context *new_dwarf_expr_context (void);

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Andrew Cagney <cagney@gnu.org> writes:
> I'm just having trouble getting my head around how this will affect
> core-gdb, and seeing how this addresses our need to meet GCC 3.5's
> requirements?
> 
> Can we expand on that?
>
> Looking at the below:
> 
> - a single value can have multiple locations (important for store)
> - could dwarf2_expr_piece be called ``struct location''? :-)

That's where we're heading, yes.  But I think one of the valuable
things about dwarf2expr.[ch] is that it does its job --- evaluating
Dwarf location expressions --- and only that job.  We ought to leave
the work of constructing GDB-specific data structures to its clients,
like dwarf2loc.c and dwarf2-frame.c.  That separation is important to
preserve; I see this as just another aspect of the priorities that led
to me revising Kevin's 2003 patch in the first place.

So in my view, dwarf2expr.[ch] should construct a data structure that
accurately reflects the result of evaluating a Dwarf 2 location
expression; it should be a Dwarf-specific data structure.  The
construction of something like a 'struct location' should be left to
dwarf2loc.c.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Andrew Cagney]

> Andrew Cagney <cagney@gnu.org> writes:
> 
>>> I'm just having trouble getting my head around how this will affect
>>> core-gdb, and seeing how this addresses our need to meet GCC 3.5's
>>> requirements?
>>> 
>>> Can we expand on that?

?

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Andrew Cagney <cagney@gnu.org> writes:

> > Andrew Cagney <cagney@gnu.org> writes:
> >
> >>> I'm just having trouble getting my head around how this will affect
> >>> core-gdb, and seeing how this addresses our need to meet GCC 3.5's
> >>> requirements?
> >>> Can we expand on that?
> 
> ?

I answered this.  The dwarf2.expr.h change I posted doesn't get into
GCC 3.5's requirements, and it shouldn't.  It's just meant to allow
dwarf2expr.[ch] to return the results of the computation it did; it's
up to its caller what to do with that.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Andrew Cagney]

> Andrew Cagney <cagney@gnu.org> writes:
> 
> 
>>>> > Andrew Cagney <cagney@gnu.org> writes:
>>>> >
>>>
>>>>>> >>> I'm just having trouble getting my head around how this will affect
>>>>>> >>> core-gdb, and seeing how this addresses our need to meet GCC 3.5's
>>>>>> >>> requirements?
>>>>>> >>> Can we expand on that?
>>
>>> 
>>> ?
> 
> 
> I answered this.  The dwarf2.expr.h change I posted doesn't get into
> GCC 3.5's requirements, and it shouldn't.  It's just meant to allow
> dwarf2expr.[ch] to return the results of the computation it did; it's
> up to its caller what to do with that.

Ok.  Can we just expand a little on how/where the caller, core-gdb, will 
use this then?

Andrew

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Andrew Cagney <cagney@gnu.org> writes:

> > Andrew Cagney <cagney@gnu.org> writes:
> >
> >>>> > Andrew Cagney <cagney@gnu.org> writes:
> >>>> >
> >>>
> >>>>>> >>> I'm just having trouble getting my head around how this will affect
> >>>>>> >>> core-gdb, and seeing how this addresses our need to meet GCC 3.5's
> >>>>>> >>> requirements?
> >>>>>> >>> Can we expand on that?
> >>
> >>> ?
> > I answered this.  The dwarf2.expr.h change I posted doesn't get into
> > GCC 3.5's requirements, and it shouldn't.  It's just meant to allow
> > dwarf2expr.[ch] to return the results of the computation it did; it's
> > up to its caller what to do with that.
> 
> Ok.  Can we just expand a little on how/where the caller, core-gdb,
> will use this then?

Well, in the immediate term, dwarf2loc.c should simply reject
locations with pieces, and generate an error message.  This is the
same behavior we have now, but instead of dwarf2expr.c saying
"Unrecognized opcode 0x93", dwarf2loc.c can say something more
informative like "The value of variable 'foo' is distributed across
several locations, and GDB cannot access its value."

The next step is to have something that will recognize when a series
of pieces is actually a reference to a single register --- presumably
one that GDB has a number for, but one that Dwarf only has numbers for
pieces of.  That needs to be an arch method that recognizes the cases
that can be simplified, and passes on the cases it can't handle.  This
will address all the current uses of DW_OP_piece, I believe.

In the long run, it's just as you said: there should be some kind of
'struct location' that 'struct value' can refer to, and that things
like value_fetch_lazy and value_assign understand and operate on.  I
don't have a lot of insight into how that all ought to fit together.
But I don't think that affects the form of the best interface to
dwarf2expr.c: that should simply provide all the information it has,
and leave it to the caller to decide what to do with it.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Daniel Jacobowitz]

On Wed, Aug 04, 2004 at 05:19:30PM -0500, Jim Blandy wrote:
> Well, in the immediate term, dwarf2loc.c should simply reject
> locations with pieces, and generate an error message.  This is the
> same behavior we have now, but instead of dwarf2expr.c saying
> "Unrecognized opcode 0x93", dwarf2loc.c can say something more
> informative like "The value of variable 'foo' is distributed across
> several locations, and GDB cannot access its value."
> 
> The next step is to have something that will recognize when a series
> of pieces is actually a reference to a single register --- presumably
> one that GDB has a number for, but one that Dwarf only has numbers for
> pieces of.  That needs to be an arch method that recognizes the cases
> that can be simplified, and passes on the cases it can't handle.  This
> will address all the current uses of DW_OP_piece, I believe.

No, it won't.  GCC already generates more than that.

I'm dubious as to whether this step is even worthwhile.  Another
possible intermediate step is to immediately combine the pieces,
marking the value as not_lval; read them all and piece them together in
GDB's memory.  Yes, this means that they can't be assigned to, but at
least we'd be able to inspect them.

> In the long run, it's just as you said: there should be some kind of
> 'struct location' that 'struct value' can refer to, and that things
> like value_fetch_lazy and value_assign understand and operate on.  I
> don't have a lot of insight into how that all ought to fit together.
> But I don't think that affects the form of the best interface to
> dwarf2expr.c: that should simply provide all the information it has,
> and leave it to the caller to decide what to do with it.

This just means auditing every site that references a memory variable's
"address" and making them either fail (conservative) or use some helper
which understands a list of pieces.  There's a large number, but most
of them are trivial.  I don't think it will really take very long to
do, on the GDB timescale.

-- 
Daniel Jacobowitz

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Wed, Aug 04, 2004 at 05:19:30PM -0500, Jim Blandy wrote:
> > Well, in the immediate term, dwarf2loc.c should simply reject
> > locations with pieces, and generate an error message.  This is the
> > same behavior we have now, but instead of dwarf2expr.c saying
> > "Unrecognized opcode 0x93", dwarf2loc.c can say something more
> > informative like "The value of variable 'foo' is distributed across
> > several locations, and GDB cannot access its value."
> > 
> > The next step is to have something that will recognize when a series
> > of pieces is actually a reference to a single register --- presumably
> > one that GDB has a number for, but one that Dwarf only has numbers for
> > pieces of.  That needs to be an arch method that recognizes the cases
> > that can be simplified, and passes on the cases it can't handle.  This
> > will address all the current uses of DW_OP_piece, I believe.
> 
> No, it won't.  GCC already generates more than that.

Okay --- that's good to know.

> I'm dubious as to whether this step is even worthwhile.  Another
> possible intermediate step is to immediately combine the pieces,
> marking the value as not_lval; read them all and piece them together in
> GDB's memory.  Yes, this means that they can't be assigned to, but at
> least we'd be able to inspect them.

It seems worthwhile to me, because it will allow GDB to work
correctly, assignments and all, in the cases where the current value
structures can handle it, and it's very little work.  At some point,
resources will materialize to do the larger project, but what's the
harm in getting the best behavior we can out of the current structures
until then?

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Mark Kettenis]

   From: Jim Blandy <jimb@redhat.com>
   Date: 04 Aug 2004 18:14:41 -0500

   > > The next step is to have something that will recognize when a series
   > > of pieces is actually a reference to a single register --- presumably
   > > one that GDB has a number for, but one that Dwarf only has numbers for
   > > pieces of.  That needs to be an arch method that recognizes the cases
   > > that can be simplified, and passes on the cases it can't handle.  This
   > > will address all the current uses of DW_OP_piece, I believe.

So we'll have to *temporary* add an arch method to several targets,
and then remove it again when we fix things properly?  I don't think
that's a good idea.

   It seems worthwhile to me, because it will allow GDB to work
   correctly, assignments and all, in the cases where the current value
   structures can handle it, and it's very little work.  At some point,
   resources will materialize to do the larger project, but what's the
   harm in getting the best behavior we can out of the current structures
   until then?

The harm is introducing hacks that are very hard to remove in the
feature.  GDB is full of such hacks, and apart from Andrew nobody
seems to actively work on trying to remove these.  We really don't
need any more of them.

Mark

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Mark Kettenis <kettenis@jive.nl> writes:
>    It seems worthwhile to me, because it will allow GDB to work
>    correctly, assignments and all, in the cases where the current value
>    structures can handle it, and it's very little work.  At some point,
>    resources will materialize to do the larger project, but what's the
>    harm in getting the best behavior we can out of the current structures
>    until then?
> 
> The harm is introducing hacks that are very hard to remove in the
> feature.  GDB is full of such hacks, and apart from Andrew nobody
> seems to actively work on trying to remove these.  We really don't
> need any more of them.

That is a good argument for not adding something.  But this will be
trivial to remove.  You'll just delete it.

Once 'struct value' can handle arbitrary piece lists, there will be no
need to try to reduce Dwarf expression results to a simpler form.
You'll instead have arch-independent code that takes any result of
Dwarf location expression evaluation and produces an assignable value.
Every case that the reduction arch method would handle will be
supported by arch-independent code instead.  So if the intermediate
step of reducing piece lists we couldn't formerly handle to locations
we could simply stops happening, there will be no visible change in
behavior.

We know the cases supported by the arch-independent code will be a
superset of the cases supported by the arch-dependent reducer.  The
latter can only support cases that can be reduced to our current
'struct value', and the arch-independent implementation will extend
that.

Again, I don't see any need for GDB to be broken until the larger
project is complete.  GDB is broken for targets today that could use
this reduction method.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Joel Brobecker]

> Again, I don't see any need for GDB to be broken until the larger
> project is complete.  GDB is broken for targets today that could use
> this reduction method.

I must say I am pretty convinced by Jim's argument. Once we have a fully
functional op_piece mechanism in place, removing the temporary hack
should have no effect at all. I would have been hesitant to go that
route too, except that Jim said it's not much work at all.

-- 
Joel

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Without stopping discussion of the best way to handle scattered values
in core GDB, I'd still like to hear comments on my suggested change to
dwarf2expr.[ch]'s interface, if there are any.

Because dwarf2expr is supposed to be concerned with evaluating Dwarf
expressions, and *avoid* building GDB data structures directly, I
think that change can be assessed separately from the larger (and
harder, and more important) question.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Daniel Jacobowitz]

On Thu, Aug 05, 2004 at 01:42:25PM -0500, Jim Blandy wrote:
> 
> Without stopping discussion of the best way to handle scattered values
> in core GDB, I'd still like to hear comments on my suggested change to
> dwarf2expr.[ch]'s interface, if there are any.
> 
> Because dwarf2expr is supposed to be concerned with evaluating Dwarf
> expressions, and *avoid* building GDB data structures directly, I
> think that change can be assessed separately from the larger (and
> harder, and more important) question.

I think it's fine.  You spelled "yield" wrong.

-- 
Daniel Jacobowitz

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Andrew Cagney]

> On Thu, Aug 05, 2004 at 01:42:25PM -0500, Jim Blandy wrote:
> 
>>> 
>>> Without stopping discussion of the best way to handle scattered values
>>> in core GDB, I'd still like to hear comments on my suggested change to
>>> dwarf2expr.[ch]'s interface, if there are any.
>>> 
>>> Because dwarf2expr is supposed to be concerned with evaluating Dwarf
>>> expressions, and *avoid* building GDB data structures directly, I
>>> think that change can be assessed separately from the larger (and
>>> harder, and more important) question.
> 
> 
> I think it's fine.  You spelled "yield" wrong.

Did this get answered?
> - a single value can have multiple locations (important for store)

Andrew

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Daniel Jacobowitz]

On Thu, Aug 05, 2004 at 03:55:50PM -0400, Andrew Cagney wrote:
> >On Thu, Aug 05, 2004 at 01:42:25PM -0500, Jim Blandy wrote:
> >
> >>>
> >>>Without stopping discussion of the best way to handle scattered values
> >>>in core GDB, I'd still like to hear comments on my suggested change to
> >>>dwarf2expr.[ch]'s interface, if there are any.
> >>>
> >>>Because dwarf2expr is supposed to be concerned with evaluating Dwarf
> >>>expressions, and *avoid* building GDB data structures directly, I
> >>>think that change can be assessed separately from the larger (and
> >>>harder, and more important) question.
> >
> >
> >I think it's fine.  You spelled "yield" wrong.
> 
> Did this get answered?
> >- a single value can have multiple locations (important for store)

No, and it's not relevant for Jim's request - focusing on the
dwarf2expr interface.  dwarf2loc, which deals with the lists of
locations, will need to be aware of this; but a single location
expression only has one location.

-- 
Daniel Jacobowitz

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Andrew Cagney]

> On Thu, Aug 05, 2004 at 03:55:50PM -0400, Andrew Cagney wrote:
> 
>>>> >On Thu, Aug 05, 2004 at 01:42:25PM -0500, Jim Blandy wrote:
>>>> >
>>>
>>>>>> >>>
>>>>>> >>>Without stopping discussion of the best way to handle scattered values
>>>>>> >>>in core GDB, I'd still like to hear comments on my suggested change to
>>>>>> >>>dwarf2expr.[ch]'s interface, if there are any.
>>>>>> >>>
>>>>>> >>>Because dwarf2expr is supposed to be concerned with evaluating Dwarf
>>>>>> >>>expressions, and *avoid* building GDB data structures directly, I
>>>>>> >>>think that change can be assessed separately from the larger (and
>>>>>> >>>harder, and more important) question.
>>>
>>>> >
>>>> >
>>>> >I think it's fine.  You spelled "yield" wrong.
>>
>>> 
>>> Did this get answered?
>>
>>>> >- a single value can have multiple locations (important for store)
> 
> 
> No.

I was worried I'd missed something.

> dwarf2loc, which deals with the lists of
> locations, will need to be aware of this; but a single location
> expression only has one location.

ah, thank you.

Andrew

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Eli Zaretskii]

> From: Jim Blandy <jimb@redhat.com>
> Date: 05 Aug 2004 09:28:21 -0500
> 
> We know the cases supported by the arch-independent code will be a
> superset of the cases supported by the arch-dependent reducer.  The
> latter can only support cases that can be reduced to our current
> 'struct value', and the arch-independent implementation will extend
> that.
> 
> Again, I don't see any need for GDB to be broken until the larger
> project is complete.  GDB is broken for targets today that could use
> this reduction method.

I tend to agree with Jim.  Here's another data point: the x86 support
for hardware watchpoints.  It was initially added to go32-nat.c for
the DJGPP port, and only later generalized to cover any i386 target.
(Of course, the initial code was committed without asking anyone,
since the DJGPP target was my responsibility ;-)  When the general x86
code was approved and committed, the DJGPP-specific code was deleted.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

To put (part of) the discussion of the Dwarf piece support on a
concrete basis, here's a patch that adds the gdbarch method; it
depends on the dwarf2expr piece interface patch I posted earlier.

This change, or something like it, would allow immediate improvements
over GDB's current behavior, and I don't see that it has much
structural cost:

- It doesn't affect the design of full scattered values: when that is
  implemented, dwarf2_evaluate_loc_desc will just become able to
  construct 'struct value' objects for more kinds of Dwarf expression
  results.

- When we do support full scattered values, the clause this patch adds
  to dwarf2_evaluate_loc_desc, the gdbarch method, and whatever
  arch-specific implementations people have written for it can be
  simply deleted, with no change in user-visible behavior.

2004-08-09  Jim Blandy  <jimb@redhat.com>

	* gdbarch.sh (dwarf_simplify_register_pieces): New method.
	Add forward declaration for 'struct dwarf_expr_piece'.
	* gdbarch.c, gdbarch.h: Regenerated.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc): Try to simplify
	multi-piece expression results before giving up.
	* arch-utils.c (dwarf_never_simplify_pieces): New function.
	* arch-utils.h (struct dwarf_expr_piece): Forward decl for struct
	type, used in ...
	(dwarf_never_simplify_pieces): New declaration.

diff -crp -x '*~' -x '.#*' -x CVS gdb/gdbarch.sh gdb/gdbarch.sh
*** gdb/gdbarch.sh	2004-08-09 19:29:28.000000000 -0500
--- gdb/gdbarch.sh	2004-08-10 11:31:22.000000000 -0500
*************** f:=:int:dwarf_reg_to_regnum:int dwarf_re
*** 451,456 ****
--- 451,480 ----
  # Convert from an sdb register number to an internal gdb register number.
  f:=:int:sdb_reg_to_regnum:int sdb_regnr:sdb_regnr::no_op_reg_to_regnum::0
  f:=:int:dwarf2_reg_to_regnum:int dwarf2_regnr:dwarf2_regnr::no_op_reg_to_regnum::0
+ 
+ # On some architectures, GDB has registers that Dwarf treats as the
+ # concatenation of two separate registers.  For example, PowerPC
+ # variants implementing the SPE APU have 64-bit general-purpose
+ # registers.  GDB refers to the lower 32 bits of each register as 'r0'
+ # -- 'r31', and the full 64-bit registers as 'ev0' -- 'ev31'.
+ # However, the Dwarf register numbering treats the upper halves as
+ # separate registers.
+ #
+ # Dwarf location expressions describe variables allocated to such
+ # registers using a series of DW_OP_piece operations.  In the case
+ # above, the expressions would have the form:
+ #
+ #   <upper half> DW_OP_piece 4 <lower half> DW_OP_piece 4.
+ #
+ # However, since GDB does have a register that corresponds to the
+ # entire variable, it can simply say the variable lives in that
+ # register; it needn't use a complicated location description.
+ #
+ # Given an array of NUM_PIECES pieces PIECES, return the number of the
+ # register that is equivalent to those pieces, or -1 if there is no
+ # such register.
+ m::int:dwarf_simplify_register_pieces:int num_pieces, struct dwarf_expr_piece *pieces:num_pieces, pieces::dwarf_never_simplify_pieces
+ 
  f:=:const char *:register_name:int regnr:regnr
  
  # REGISTER_TYPE is a direct replacement for DEPRECATED_REGISTER_VIRTUAL_TYPE.
*************** struct regset;
*** 773,778 ****
--- 797,803 ----
  struct disassemble_info;
  struct target_ops;
  struct obstack;
+ struct dwarf_expr_piece;
  
  extern struct gdbarch *current_gdbarch;
  
diff -crp -x '*~' -x '.#*' -x CVS gdb/dwarf2loc.c gdb/dwarf2loc.c
*** gdb/dwarf2loc.c	2004-08-10 02:01:24.000000000 -0500
--- gdb/dwarf2loc.c	2004-08-10 11:34:39.000000000 -0500
*************** dwarf2_evaluate_loc_desc (struct symbol 
*** 230,240 ****
    dwarf_expr_eval (ctx, data, size);
    if (ctx->num_pieces > 0)
      {
!       /* We haven't implemented splicing together pieces from
!          arbitrary sources yet.  */
!       error ("The value of variable '%s' is distributed across several\n"
!              "locations, and GDB cannot access its value.\n",
!              SYMBOL_NATURAL_NAME (var));
      }
    else if (ctx->in_reg)
      {
--- 230,247 ----
    dwarf_expr_eval (ctx, data, size);
    if (ctx->num_pieces > 0)
      {
!       CORE_ADDR simplified
!         = gdbarch_dwarf_simplify_register_pieces (arch, ctx->num_pieces,
!                                                   ctx->pieces);
!       if (simplified >= 0)
!         retval = value_from_register (SYMBOL_TYPE (var), simplified, frame);
! 
!       /* We haven't implemented the more complex case of splicing
!          together pieces from arbitrary sources yet.  */
!       else
!         error ("The value of variable '%s' is distributed across several\n"
!                "locations, and GDB cannot access its value.\n",
!                SYMBOL_NATURAL_NAME (var));
      }
    else if (ctx->in_reg)
      {
diff -crp -x '*~' -x '.#*' -x CVS gdb/arch-utils.h gdb/arch-utils.h
*** gdb/arch-utils.h	2004-08-09 19:29:27.000000000 -0500
--- gdb/arch-utils.h	2004-08-10 11:32:03.000000000 -0500
*************** struct frame_info;
*** 28,33 ****
--- 28,34 ----
  struct minimal_symbol;
  struct type;
  struct gdbarch_info;
+ struct dwarf_expr_piece;
  
  /* gdbarch trace variable */
  extern int gdbarch_debug;
*************** extern gdbarch_convert_from_func_ptr_add
*** 72,77 ****
--- 73,82 ----
  
  extern int no_op_reg_to_regnum (int reg);
  
+ /* Dwarf piece simplifier that never simplifies anything.  */
+ extern int dwarf_never_simplify_pieces (int num_pieces,
+                                         struct dwarf_expr_piece *pieces);
+ 
  /* Do nothing version of elf_make_msymbol_special. */
  
  void default_elf_make_msymbol_special (asymbol *sym, struct minimal_symbol *msym);
diff -crp -x '*~' -x '.#*' -x CVS gdb/arch-utils.c gdb/arch-utils.c
*** gdb/arch-utils.c	2004-08-09 19:29:27.000000000 -0500
--- gdb/arch-utils.c	2004-08-10 11:32:17.000000000 -0500
*************** no_op_reg_to_regnum (int reg)
*** 228,233 ****
--- 228,240 ----
    return reg;
  }
  
+ int
+ dwarf_never_simplify_pieces (int num_pieces,
+                              struct dwarf_expr_piece *pieces)
+ {
+   return -1;
+ }
+ 
  void
  default_elf_make_msymbol_special (asymbol *sym, struct minimal_symbol *msym)
  {
diff -crp -x '*~' -x '.#*' -x CVS gdb/gdbarch.c gdb/gdbarch.c
*** gdb/gdbarch.c	2004-08-09 19:29:28.000000000 -0500
--- gdb/gdbarch.c	2004-08-10 11:28:42.000000000 -0500
*************** struct gdbarch
*** 155,160 ****
--- 155,161 ----
    gdbarch_dwarf_reg_to_regnum_ftype *dwarf_reg_to_regnum;
    gdbarch_sdb_reg_to_regnum_ftype *sdb_reg_to_regnum;
    gdbarch_dwarf2_reg_to_regnum_ftype *dwarf2_reg_to_regnum;
+   gdbarch_dwarf_simplify_register_pieces_ftype *dwarf_simplify_register_pieces;
    gdbarch_register_name_ftype *register_name;
    gdbarch_register_type_ftype *register_type;
    gdbarch_deprecated_register_byte_ftype *deprecated_register_byte;
*************** struct gdbarch startup_gdbarch =
*** 281,286 ****
--- 282,288 ----
    0,  /* dwarf_reg_to_regnum */
    0,  /* sdb_reg_to_regnum */
    0,  /* dwarf2_reg_to_regnum */
+   dwarf_never_simplify_pieces,  /* dwarf_simplify_register_pieces */
    0,  /* register_name */
    0,  /* register_type */
    generic_register_byte,  /* deprecated_register_byte */
*************** gdbarch_alloc (const struct gdbarch_info
*** 414,419 ****
--- 416,422 ----
    current_gdbarch->dwarf_reg_to_regnum = no_op_reg_to_regnum;
    current_gdbarch->sdb_reg_to_regnum = no_op_reg_to_regnum;
    current_gdbarch->dwarf2_reg_to_regnum = no_op_reg_to_regnum;
+   current_gdbarch->dwarf_simplify_register_pieces = dwarf_never_simplify_pieces;
    current_gdbarch->deprecated_register_byte = generic_register_byte;
    current_gdbarch->deprecated_fp_regnum = -1;
    current_gdbarch->call_dummy_location = AT_ENTRY_POINT;
*************** verify_gdbarch (struct gdbarch *current_
*** 541,546 ****
--- 544,552 ----
    /* Skip verify of dwarf_reg_to_regnum, invalid_p == 0 */
    /* Skip verify of sdb_reg_to_regnum, invalid_p == 0 */
    /* Skip verify of dwarf2_reg_to_regnum, invalid_p == 0 */
+   if ((GDB_MULTI_ARCH > GDB_MULTI_ARCH_PARTIAL)
+       && (current_gdbarch->dwarf_simplify_register_pieces == dwarf_never_simplify_pieces))
+     fprintf_unfiltered (log, "\n\tdwarf_simplify_register_pieces");
    /* Skip verify of register_type, has predicate */
    /* Skip verify of deprecated_register_byte, has predicate */
    /* Skip verify of unwind_dummy_id, has predicate */
*************** gdbarch_dump (struct gdbarch *current_gd
*** 1027,1032 ****
--- 1033,1041 ----
    fprintf_unfiltered (file,
                        "gdbarch_dump: dwarf_reg_to_regnum = <0x%lx>\n",
                        (long) current_gdbarch->dwarf_reg_to_regnum);
+   fprintf_unfiltered (file,
+                       "gdbarch_dump: dwarf_simplify_register_pieces = <0x%lx>\n",
+                       (long) current_gdbarch->dwarf_simplify_register_pieces);
  #ifdef ECOFF_REG_TO_REGNUM
    fprintf_unfiltered (file,
                        "gdbarch_dump: %s # %s\n",
*************** set_gdbarch_dwarf2_reg_to_regnum (struct
*** 2220,2225 ****
--- 2229,2251 ----
    gdbarch->dwarf2_reg_to_regnum = dwarf2_reg_to_regnum;
  }
  
+ int
+ gdbarch_dwarf_simplify_register_pieces (struct gdbarch *gdbarch, int num_pieces, struct dwarf_expr_piece *pieces)
+ {
+   gdb_assert (gdbarch != NULL);
+   gdb_assert (gdbarch->dwarf_simplify_register_pieces != NULL);
+   if (gdbarch_debug >= 2)
+     fprintf_unfiltered (gdb_stdlog, "gdbarch_dwarf_simplify_register_pieces called\n");
+   return gdbarch->dwarf_simplify_register_pieces (gdbarch, num_pieces, pieces);
+ }
+ 
+ void
+ set_gdbarch_dwarf_simplify_register_pieces (struct gdbarch *gdbarch,
+                                             gdbarch_dwarf_simplify_register_pieces_ftype dwarf_simplify_register_pieces)
+ {
+   gdbarch->dwarf_simplify_register_pieces = dwarf_simplify_register_pieces;
+ }
+ 
  const char *
  gdbarch_register_name (struct gdbarch *gdbarch, int regnr)
  {
diff -crp -x '*~' -x '.#*' -x CVS gdb/gdbarch.h gdb/gdbarch.h
*** gdb/gdbarch.h	2004-08-09 19:29:28.000000000 -0500
--- gdb/gdbarch.h	2004-08-10 11:28:42.000000000 -0500
*************** struct regset;
*** 49,54 ****
--- 49,55 ----
  struct disassemble_info;
  struct target_ops;
  struct obstack;
+ struct dwarf_expr_piece;
  
  extern struct gdbarch *current_gdbarch;
  
*************** extern void set_gdbarch_dwarf2_reg_to_re
*** 461,466 ****
--- 462,493 ----
  #define DWARF2_REG_TO_REGNUM(dwarf2_regnr) (gdbarch_dwarf2_reg_to_regnum (current_gdbarch, dwarf2_regnr))
  #endif
  
+ /* On some architectures, GDB has registers that Dwarf treats as the
+    concatenation of two separate registers.  For example, PowerPC
+    variants implementing the SPE APU have 64-bit general-purpose
+    registers.  GDB refers to the lower 32 bits of each register as 'r0'
+    -- 'r31', and the full 64-bit registers as 'ev0' -- 'ev31'.
+    However, the Dwarf register numbering treats the upper halves as
+    separate registers.
+   
+    Dwarf location expressions describe variables allocated to such
+    registers using a series of DW_OP_piece operations.  In the case
+    above, the expressions would have the form:
+   
+      <upper half> DW_OP_piece 4 <lower half> DW_OP_piece 4.
+   
+    However, since GDB does have a register that corresponds to the
+    entire variable, it can simply say the variable lives in that
+    register; it needn't use a complicated location description.
+   
+    Given an array of NUM_PIECES pieces PIECES, return the number of the
+    register that is equivalent to those pieces, or -1 if there is no
+    such register. */
+ 
+ typedef int (gdbarch_dwarf_simplify_register_pieces_ftype) (struct gdbarch *gdbarch, int num_pieces, struct dwarf_expr_piece *pieces);
+ extern int gdbarch_dwarf_simplify_register_pieces (struct gdbarch *gdbarch, int num_pieces, struct dwarf_expr_piece *pieces);
+ extern void set_gdbarch_dwarf_simplify_register_pieces (struct gdbarch *gdbarch, gdbarch_dwarf_simplify_register_pieces_ftype *dwarf_simplify_register_pieces);
+ 
  typedef const char * (gdbarch_register_name_ftype) (int regnr);
  extern const char * gdbarch_register_name (struct gdbarch *gdbarch, int regnr);
  extern void set_gdbarch_register_name (struct gdbarch *gdbarch, gdbarch_register_name_ftype *register_name);

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Jim Blandy]

Dan, does that patch correctly implement the suggestions in this
message:

    http://sources.redhat.com/ml/gdb-patches/2003-05/msg00431.html

?

The answer doesn't bear directly on the central question in this
thread; whether I correctly understood Dan's suggestions from a year
ago doesn't really affect whether an arch-specific piece simplifier is
a good thing to do today.  I'm just looking for a sanity check on
whether I properly understood what was requested.

Re: interface to partial support for DW_OP_piece in dwarf2expr.[ch]

[Daniel Jacobowitz]

On Wed, Aug 11, 2004 at 03:28:05PM -0500, Jim Blandy wrote:
> 
> Dan, does that patch correctly implement the suggestions in this
> message:
> 
>     http://sources.redhat.com/ml/gdb-patches/2003-05/msg00431.html
> 
> ?
> 
> The answer doesn't bear directly on the central question in this
> thread; whether I correctly understood Dan's suggestions from a year
> ago doesn't really affect whether an arch-specific piece simplifier is
> a good thing to do today.  I'm just looking for a sanity check on
> whether I properly understood what was requested.

Absolutely it does.  Thank you!

-- 
Daniel Jacobowitz