public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* two agent expression nits (one line each)
@ 2014-09-11 17:54 David Taylor
  2014-11-23  3:27 ` Joel Brobecker
  2014-12-02 20:45 ` Stan Shebs
  0 siblings, 2 replies; 11+ messages in thread
From: David Taylor @ 2014-09-11 17:54 UTC (permalink / raw)
  To: gdb-patches

In gdb/doc/agentexpr.texi you''ll find the text:

    @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
    Set trace state variable number @var{n} to the value found on the top
    of the stack.  The stack is unchanged, so that the value is readily
    available if the assignment is part of a larger expression.  The
    handling of @var{n} is as described for @code{getv}.

The @item line and the following text do no agree with one another.  I'm
guessing that the text is correct, in which case this line:

    @item @code{setv} (0x2d) @var{n}: @result{} @var{v}

should be changed to this:

    @item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}

Additionally, in gdb/common/ax.def we find the line:

    DEFOP (setv, 2, 0, 0, 1, 0x2d)

From the comment earlier in the file:

       Each line is of the form:

       DEFOP (name, size, data_size, consumed, produced, opcode)
[...]
       CONSUMED is the number of stack elements consumed.
       PRODUCED is the number of stack elements produced.

which is saying that nothing is consumed and one item is produced.  Both
should be 0 or both should be 1.  Setting them both to 1 seems better
since if nothing is on the stack an error will occur.  So, it should be
changed to:

    DEFOP (setv, 2, 0, 1, 1, 0x2d)

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-09-11 17:54 two agent expression nits (one line each) David Taylor
@ 2014-11-23  3:27 ` Joel Brobecker
  2014-12-02 20:45 ` Stan Shebs
  1 sibling, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-11-23  3:27 UTC (permalink / raw)
  To: Stan Shebs; +Cc: gdb-patches, David Taylor

Hi Stan

Would you mind looking into this? David's comments seem reasonable,
but it would be nice to confirm that what we currently do is what
we had meant to do :-).

Thanks!

On Thu, Sep 11, 2014 at 01:54:10PM -0400, David Taylor wrote:
> In gdb/doc/agentexpr.texi you''ll find the text:
> 
>     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
>     Set trace state variable number @var{n} to the value found on the top
>     of the stack.  The stack is unchanged, so that the value is readily
>     available if the assignment is part of a larger expression.  The
>     handling of @var{n} is as described for @code{getv}.
> 
> The @item line and the following text do no agree with one another.  I'm
> guessing that the text is correct, in which case this line:
> 
>     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> 
> should be changed to this:
> 
>     @item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
> 
> Additionally, in gdb/common/ax.def we find the line:
> 
>     DEFOP (setv, 2, 0, 0, 1, 0x2d)
> 
> >From the comment earlier in the file:
> 
>        Each line is of the form:
> 
>        DEFOP (name, size, data_size, consumed, produced, opcode)
> [...]
>        CONSUMED is the number of stack elements consumed.
>        PRODUCED is the number of stack elements produced.
> 
> which is saying that nothing is consumed and one item is produced.  Both
> should be 0 or both should be 1.  Setting them both to 1 seems better
> since if nothing is on the stack an error will occur.  So, it should be
> changed to:
> 
>     DEFOP (setv, 2, 0, 1, 1, 0x2d)
> 
> David

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-09-11 17:54 two agent expression nits (one line each) David Taylor
  2014-11-23  3:27 ` Joel Brobecker
@ 2014-12-02 20:45 ` Stan Shebs
  2014-12-13 13:44   ` Joel Brobecker
  1 sibling, 1 reply; 11+ messages in thread
From: Stan Shebs @ 2014-12-02 20:45 UTC (permalink / raw)
  To: David Taylor, gdb-patches

On 9/11/14, 10:54 AM, David Taylor wrote:
> In gdb/doc/agentexpr.texi you''ll find the text:
> 
>     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
>     Set trace state variable number @var{n} to the value found on the top
>     of the stack.  The stack is unchanged, so that the value is readily
>     available if the assignment is part of a larger expression.  The
>     handling of @var{n} is as described for @code{getv}.
> 
> The @item line and the following text do no agree with one another.  I'm
> guessing that the text is correct, in which case this line:
> 
>     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> 
> should be changed to this:
> 
>     @item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}

Yes, that is correct.

> Additionally, in gdb/common/ax.def we find the line:
> 
>     DEFOP (setv, 2, 0, 0, 1, 0x2d)
> 
>From the comment earlier in the file:
> 
>        Each line is of the form:
> 
>        DEFOP (name, size, data_size, consumed, produced, opcode)
> [...]
>        CONSUMED is the number of stack elements consumed.
>        PRODUCED is the number of stack elements produced.
> 
> which is saying that nothing is consumed and one item is produced.  Both
> should be 0 or both should be 1.  Setting them both to 1 seems better
> since if nothing is on the stack an error will occur.  So, it should be
> changed to:
> 
>     DEFOP (setv, 2, 0, 1, 1, 0x2d)

Yes to this also.  From the look of things, I cloned the getv definition
and forgot to adjust it.

Although technically an incompatible change to the bytecode language,
anybody who tried to set a state variable within a larger expression was
going to get odd behavior and/or stack overflow, while the common
case of just setting the variable and discarding the result is
unaffected.  So I think we can just make the change directly, and
perhaps add a brief note to NEWS, to make sure that target agents
get updated. (Mentor has one in-house separate from gdbserver, plus at
least two customers with their own agents.)

Stan
stan@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-12-02 20:45 ` Stan Shebs
@ 2014-12-13 13:44   ` Joel Brobecker
  2014-12-15 15:41     ` David Taylor
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2014-12-13 13:44 UTC (permalink / raw)
  To: Stan Shebs; +Cc: David Taylor, gdb-patches

Hi Stan,

> > The @item line and the following text do no agree with one another.  I'm
> > guessing that the text is correct, in which case this line:
> > 
> >     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> > 
> > should be changed to this:
> > 
> >     @item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
> 
> Yes, that is correct.
> 
> > Additionally, in gdb/common/ax.def we find the line:
> > 
> >     DEFOP (setv, 2, 0, 0, 1, 0x2d)
> > 
> >>From the comment earlier in the file:
> > 
> >        Each line is of the form:
> > 
> >        DEFOP (name, size, data_size, consumed, produced, opcode)
> > [...]
> >        CONSUMED is the number of stack elements consumed.
> >        PRODUCED is the number of stack elements produced.
> > 
> > which is saying that nothing is consumed and one item is produced.  Both
> > should be 0 or both should be 1.  Setting them both to 1 seems better
> > since if nothing is on the stack an error will occur.  So, it should be
> > changed to:
> > 
> >     DEFOP (setv, 2, 0, 1, 1, 0x2d)
> 
> Yes to this also.  From the look of things, I cloned the getv definition
> and forgot to adjust it.
> 
> Although technically an incompatible change to the bytecode language,
> anybody who tried to set a state variable within a larger expression was
> going to get odd behavior and/or stack overflow, while the common
> case of just setting the variable and discarding the result is
> unaffected.  So I think we can just make the change directly, and
> perhaps add a brief note to NEWS, to make sure that target agents
> get updated. (Mentor has one in-house separate from gdbserver, plus at
> least two customers with their own agents.)

Would you mind taking care of that for us? I kept that message around
in the hope that I'd have a clearer picture with a little extra time,
but I am still not sure whether you're just suggesting a documentation
fix, or if you're saying we should fix the implementation...

Thanks!
-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-12-13 13:44   ` Joel Brobecker
@ 2014-12-15 15:41     ` David Taylor
  2014-12-20 17:19       ` Joel Brobecker
  0 siblings, 1 reply; 11+ messages in thread
From: David Taylor @ 2014-12-15 15:41 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Stan Shebs, gdb-patches

Joel Brobecker <brobecker@adacore.com> wrote:

> Hi Stan,
> 
> > > The @item line and the following text do no agree with one another.  I'm
> > > guessing that the text is correct, in which case this line:
> > > 
> > >     @item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> > > 
> > > should be changed to this:
> > > 
> > >     @item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
> > 
> > Yes, that is correct.
> > 
> > > Additionally, in gdb/common/ax.def we find the line:
> > > 
> > >     DEFOP (setv, 2, 0, 0, 1, 0x2d)
> > > 
> > >>From the comment earlier in the file:
> > > 
> > >        Each line is of the form:
> > > 
> > >        DEFOP (name, size, data_size, consumed, produced, opcode)
> > > [...]
> > >        CONSUMED is the number of stack elements consumed.
> > >        PRODUCED is the number of stack elements produced.
> > > 
> > > which is saying that nothing is consumed and one item is produced.  Both
> > > should be 0 or both should be 1.  Setting them both to 1 seems better
> > > since if nothing is on the stack an error will occur.  So, it should be
> > > changed to:
> > > 
> > >     DEFOP (setv, 2, 0, 1, 1, 0x2d)
> > 
> > Yes to this also.  From the look of things, I cloned the getv definition
> > and forgot to adjust it.
> > 
> > Although technically an incompatible change to the bytecode language,
> > anybody who tried to set a state variable within a larger expression was
> > going to get odd behavior and/or stack overflow, while the common
> > case of just setting the variable and discarding the result is
> > unaffected.  So I think we can just make the change directly, and
> > perhaps add a brief note to NEWS, to make sure that target agents
> > get updated. (Mentor has one in-house separate from gdbserver, plus at
> > least two customers with their own agents.)
> 
> Would you mind taking care of that for us? I kept that message around
> in the hope that I'd have a clearer picture with a little extra time,
> but I am still not sure whether you're just suggesting a documentation
> fix, or if you're saying we should fix the implementation...

The first part is pure documentation -- the description is correct but
the summary line is wrong.  The second part is implementation -- the
consumed and produced fields are used to check for stack underflow and
overflow.

The fields are used by gdb and gdbserver for sanity checking the
bytecode expression -- checking for underflow and overflow.

The underflow check catches bad bytecode sequences generated by GDB.

Since DEFOP has consumed as 0 when it is really 1, that means that GDB
will think that there is more on the stack than there really is.

So, if GDB generates a bytecode sequence that underflows it might not
catch it.  It does not currently generate bad sequences, so fixing it
will merely help catch future lossage.

As I recall, the overflow check is used by gdbserver to verify that it
has sufficent stack space for the expression.  The bad DEFOP line makes
it think that the expression is using one more stack slot than reality.
But, gdbserver has fairly generous bytecode stack space -- so unless the
expression is really complicated, that won't happen.

> Thanks!
> -- 
> Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-12-15 15:41     ` David Taylor
@ 2014-12-20 17:19       ` Joel Brobecker
  2015-02-03 21:58         ` David Taylor
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2014-12-20 17:19 UTC (permalink / raw)
  To: David Taylor; +Cc: Stan Shebs, gdb-patches

Hi David,

> > Would you mind taking care of that for us? I kept that message around
> > in the hope that I'd have a clearer picture with a little extra time,
> > but I am still not sure whether you're just suggesting a documentation
> > fix, or if you're saying we should fix the implementation...
> 
> The first part is pure documentation -- the description is correct but
> the summary line is wrong.  The second part is implementation -- the
> consumed and produced fields are used to check for stack underflow and
> overflow.
> 
> The fields are used by gdb and gdbserver for sanity checking the
> bytecode expression -- checking for underflow and overflow.
> 
> The underflow check catches bad bytecode sequences generated by GDB.
> 
> Since DEFOP has consumed as 0 when it is really 1, that means that GDB
> will think that there is more on the stack than there really is.
> 
> So, if GDB generates a bytecode sequence that underflows it might not
> catch it.  It does not currently generate bad sequences, so fixing it
> will merely help catch future lossage.
> 
> As I recall, the overflow check is used by gdbserver to verify that it
> has sufficent stack space for the expression.  The bad DEFOP line makes
> it think that the expression is using one more stack slot than reality.
> But, gdbserver has fairly generous bytecode stack space -- so unless the
> expression is really complicated, that won't happen.

Since you understand what should be done, would you mind sending
a patch in for Stan to review?

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2014-12-20 17:19       ` Joel Brobecker
@ 2015-02-03 21:58         ` David Taylor
  2015-02-11  7:49           ` Joel Brobecker
  2015-02-11 17:28           ` Stan Shebs
  0 siblings, 2 replies; 11+ messages in thread
From: David Taylor @ 2015-02-03 21:58 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Joel Brobecker, gdb-patches

Joel Brobecker <brobecker@adacore.com> wrote:

> Hi David,

[...]

> Since you understand what should be done, would you mind sending
> a patch in for Stan to review?
> 
> -- 
> Joel

Joel,

Sorry for the delay.  Here's the patch.  Also, I haven't forgotten that
I owe you an updated patch for bad structure offsets.  And I have some
others in the queue to finish up and post, as well.

Stan,

Here's the ChangeLog entries:

gdb:

2015-02-03  David Taylor  <dtaylor@emc.com>

	* common/ax.def (setv): Fix consumed entry in setv DEFOP.

gdb/doc:

2015-02-03  David Taylor  <dtaylor@emc.com>

	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.


and patch, as requested by Joel:

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 351ccdd..0118d7d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-02-03  David Taylor  <dtaylor@emc.com>
+
+	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
+
 2015-02-02  Joel Brobecker  <brobecker@adacore.com>
 
 	* dwarf2loc.c (dwarf2_evaluate_property): Add i18n marker.
diff --git a/gdb/common/ax.def b/gdb/common/ax.def
index 8b27725..27c97cc 100644
--- a/gdb/common/ax.def
+++ b/gdb/common/ax.def
@@ -83,7 +83,7 @@ DEFOP (pop, 0, 0, 1, 0, 0x29)
 DEFOP (zero_ext, 1, 0, 1, 1, 0x2a)
 DEFOP (swap, 0, 0, 2, 2, 0x2b)
 DEFOP (getv, 2, 0, 0, 1, 0x2c)
-DEFOP (setv, 2, 0, 0, 1, 0x2d)
+DEFOP (setv, 2, 0, 1, 1, 0x2d)
 DEFOP (tracev, 2, 0, 0, 1, 0x2e)
 DEFOP (tracenz, 0, 0, 2, 0, 0x2f)
 DEFOP (trace16, 2, 0, 1, 1, 0x30)
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 9c12d9a..34fee48 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2015-02-03  David Taylor  <dtaylor@emc.com>
+
+	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
+
 2015-01-31  Gary Benson <gbenson@redhat.com>
 	    Doug Evans  <dje@google.com>
 
diff --git a/gdb/doc/agentexpr.texi b/gdb/doc/agentexpr.texi
index 788de1c..297cd5e 100644
--- a/gdb/doc/agentexpr.texi
+++ b/gdb/doc/agentexpr.texi
@@ -461,7 +461,7 @@ alignment within the bytecode stream; thus, on machines where fetching a
 16-bit on an unaligned address raises an exception, you should fetch the
 register number one byte at a time.
 
-@item @code{setv} (0x2d) @var{n}: @result{} @var{v}
+@item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
 Set trace state variable number @var{n} to the value found on the top
 of the stack.  The stack is unchanged, so that the value is readily
 available if the assignment is part of a larger expression.  The

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2015-02-03 21:58         ` David Taylor
@ 2015-02-11  7:49           ` Joel Brobecker
  2015-02-11 17:28           ` Stan Shebs
  1 sibling, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2015-02-11  7:49 UTC (permalink / raw)
  To: David Taylor, Stan Shebs; +Cc: Stan Shebs, gdb-patches

Hi Stan,

Would you mind reviewing this patch for us, please? If we are going
to change a spec, we might as well do it earlier than later and so
this could be a good candidate for the 7.9 branch.

Thanks!

On Tue, Feb 03, 2015 at 04:57:24PM -0500, David Taylor wrote:
> Joel Brobecker <brobecker@adacore.com> wrote:
> 
> > Hi David,
> 
> [...]
> 
> > Since you understand what should be done, would you mind sending
> > a patch in for Stan to review?
> > 
> > -- 
> > Joel
> 
> Joel,
> 
> Sorry for the delay.  Here's the patch.  Also, I haven't forgotten that
> I owe you an updated patch for bad structure offsets.  And I have some
> others in the queue to finish up and post, as well.
> 
> Stan,
> 
> Here's the ChangeLog entries:
> 
> gdb:
> 
> 2015-02-03  David Taylor  <dtaylor@emc.com>
> 
> 	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> 
> gdb/doc:
> 
> 2015-02-03  David Taylor  <dtaylor@emc.com>
> 
> 	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> 
> 
> and patch, as requested by Joel:
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 351ccdd..0118d7d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-02-03  David Taylor  <dtaylor@emc.com>
> +
> +	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> +
>  2015-02-02  Joel Brobecker  <brobecker@adacore.com>
>  
>  	* dwarf2loc.c (dwarf2_evaluate_property): Add i18n marker.
> diff --git a/gdb/common/ax.def b/gdb/common/ax.def
> index 8b27725..27c97cc 100644
> --- a/gdb/common/ax.def
> +++ b/gdb/common/ax.def
> @@ -83,7 +83,7 @@ DEFOP (pop, 0, 0, 1, 0, 0x29)
>  DEFOP (zero_ext, 1, 0, 1, 1, 0x2a)
>  DEFOP (swap, 0, 0, 2, 2, 0x2b)
>  DEFOP (getv, 2, 0, 0, 1, 0x2c)
> -DEFOP (setv, 2, 0, 0, 1, 0x2d)
> +DEFOP (setv, 2, 0, 1, 1, 0x2d)
>  DEFOP (tracev, 2, 0, 0, 1, 0x2e)
>  DEFOP (tracenz, 0, 0, 2, 0, 0x2f)
>  DEFOP (trace16, 2, 0, 1, 1, 0x30)
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 9c12d9a..34fee48 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-02-03  David Taylor  <dtaylor@emc.com>
> +
> +	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> +
>  2015-01-31  Gary Benson <gbenson@redhat.com>
>  	    Doug Evans  <dje@google.com>
>  
> diff --git a/gdb/doc/agentexpr.texi b/gdb/doc/agentexpr.texi
> index 788de1c..297cd5e 100644
> --- a/gdb/doc/agentexpr.texi
> +++ b/gdb/doc/agentexpr.texi
> @@ -461,7 +461,7 @@ alignment within the bytecode stream; thus, on machines where fetching a
>  16-bit on an unaligned address raises an exception, you should fetch the
>  register number one byte at a time.
>  
> -@item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> +@item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
>  Set trace state variable number @var{n} to the value found on the top
>  of the stack.  The stack is unchanged, so that the value is readily
>  available if the assignment is part of a larger expression.  The

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2015-02-03 21:58         ` David Taylor
  2015-02-11  7:49           ` Joel Brobecker
@ 2015-02-11 17:28           ` Stan Shebs
  2015-02-13 19:21             ` David Taylor
  1 sibling, 1 reply; 11+ messages in thread
From: Stan Shebs @ 2015-02-11 17:28 UTC (permalink / raw)
  To: David Taylor; +Cc: Joel Brobecker, gdb-patches

On 2/3/15 1:57 PM, David Taylor wrote:
> Joel Brobecker <brobecker@adacore.com> wrote:
> 
>> Hi David,
> 
> [...]
> 
>> Since you understand what should be done, would you mind sending
>> a patch in for Stan to review?
>>
>> -- 
>> Joel
> 
> Joel,
> 
> Sorry for the delay.  Here's the patch.  Also, I haven't forgotten that
> I owe you an updated patch for bad structure offsets.  And I have some
> others in the queue to finish up and post, as well.

These changes are both fine as posted, thanks!

And I agree that they would be good candidates for 7.9 branch as well.

Stan

> 
> Stan,
> 
> Here's the ChangeLog entries:
> 
> gdb:
> 
> 2015-02-03  David Taylor  <dtaylor@emc.com>
> 
> 	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> 
> gdb/doc:
> 
> 2015-02-03  David Taylor  <dtaylor@emc.com>
> 
> 	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> 
> 
> and patch, as requested by Joel:
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 351ccdd..0118d7d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-02-03  David Taylor  <dtaylor@emc.com>
> +
> +	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> +
>  2015-02-02  Joel Brobecker  <brobecker@adacore.com>
>  
>  	* dwarf2loc.c (dwarf2_evaluate_property): Add i18n marker.
> diff --git a/gdb/common/ax.def b/gdb/common/ax.def
> index 8b27725..27c97cc 100644
> --- a/gdb/common/ax.def
> +++ b/gdb/common/ax.def
> @@ -83,7 +83,7 @@ DEFOP (pop, 0, 0, 1, 0, 0x29)
>  DEFOP (zero_ext, 1, 0, 1, 1, 0x2a)
>  DEFOP (swap, 0, 0, 2, 2, 0x2b)
>  DEFOP (getv, 2, 0, 0, 1, 0x2c)
> -DEFOP (setv, 2, 0, 0, 1, 0x2d)
> +DEFOP (setv, 2, 0, 1, 1, 0x2d)
>  DEFOP (tracev, 2, 0, 0, 1, 0x2e)
>  DEFOP (tracenz, 0, 0, 2, 0, 0x2f)
>  DEFOP (trace16, 2, 0, 1, 1, 0x30)
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 9c12d9a..34fee48 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-02-03  David Taylor  <dtaylor@emc.com>
> +
> +	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> +
>  2015-01-31  Gary Benson <gbenson@redhat.com>
>  	    Doug Evans  <dje@google.com>
>  
> diff --git a/gdb/doc/agentexpr.texi b/gdb/doc/agentexpr.texi
> index 788de1c..297cd5e 100644
> --- a/gdb/doc/agentexpr.texi
> +++ b/gdb/doc/agentexpr.texi
> @@ -461,7 +461,7 @@ alignment within the bytecode stream; thus, on machines where fetching a
>  16-bit on an unaligned address raises an exception, you should fetch the
>  register number one byte at a time.
>  
> -@item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> +@item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
>  Set trace state variable number @var{n} to the value found on the top
>  of the stack.  The stack is unchanged, so that the value is readily
>  available if the assignment is part of a larger expression.  The
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: two agent expression nits (one line each)
  2015-02-11 17:28           ` Stan Shebs
@ 2015-02-13 19:21             ` David Taylor
  2015-02-20  3:05               ` pushed (master+branch): " Joel Brobecker
  0 siblings, 1 reply; 11+ messages in thread
From: David Taylor @ 2015-02-13 19:21 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Joel Brobecker, gdb-patches

Stan Shebs <stanshebs@earthlink.net> wrote:

> On 2/3/15 1:57 PM, David Taylor wrote:
> > Joel Brobecker <brobecker@adacore.com> wrote:
> > 
> >> Hi David,
> > 
> > [...]
> > 
> >> Since you understand what should be done, would you mind sending
> >> a patch in for Stan to review?
> >>
> >> -- 
> >> Joel
> > 
> > Joel,
> > 
> > Sorry for the delay.  Here's the patch.  Also, I haven't forgotten that
> > I owe you an updated patch for bad structure offsets.  And I have some
> > others in the queue to finish up and post, as well.
> 
> These changes are both fine as posted, thanks!
> 
> And I agree that they would be good candidates for 7.9 branch as well.
> 
> Stan

Will someone please commit these for me (as I do not have commit privs)?

Thanks.

> > 
> > Stan,
> > 
> > Here's the ChangeLog entries:
> > 
> > gdb:
> > 
> > 2015-02-03  David Taylor  <dtaylor@emc.com>
> > 
> > 	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> > 
> > gdb/doc:
> > 
> > 2015-02-03  David Taylor  <dtaylor@emc.com>
> > 
> > 	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> > 
> > 
> > and patch, as requested by Joel:
> > 
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 351ccdd..0118d7d 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2015-02-03  David Taylor  <dtaylor@emc.com>
> > +
> > +	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> > +
> >  2015-02-02  Joel Brobecker  <brobecker@adacore.com>
> >  
> >  	* dwarf2loc.c (dwarf2_evaluate_property): Add i18n marker.
> > diff --git a/gdb/common/ax.def b/gdb/common/ax.def
> > index 8b27725..27c97cc 100644
> > --- a/gdb/common/ax.def
> > +++ b/gdb/common/ax.def
> > @@ -83,7 +83,7 @@ DEFOP (pop, 0, 0, 1, 0, 0x29)
> >  DEFOP (zero_ext, 1, 0, 1, 1, 0x2a)
> >  DEFOP (swap, 0, 0, 2, 2, 0x2b)
> >  DEFOP (getv, 2, 0, 0, 1, 0x2c)
> > -DEFOP (setv, 2, 0, 0, 1, 0x2d)
> > +DEFOP (setv, 2, 0, 1, 1, 0x2d)
> >  DEFOP (tracev, 2, 0, 0, 1, 0x2e)
> >  DEFOP (tracenz, 0, 0, 2, 0, 0x2f)
> >  DEFOP (trace16, 2, 0, 1, 1, 0x30)
> > diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> > index 9c12d9a..34fee48 100644
> > --- a/gdb/doc/ChangeLog
> > +++ b/gdb/doc/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2015-02-03  David Taylor  <dtaylor@emc.com>
> > +
> > +	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.
> > +
> >  2015-01-31  Gary Benson <gbenson@redhat.com>
> >  	    Doug Evans  <dje@google.com>
> >  
> > diff --git a/gdb/doc/agentexpr.texi b/gdb/doc/agentexpr.texi
> > index 788de1c..297cd5e 100644
> > --- a/gdb/doc/agentexpr.texi
> > +++ b/gdb/doc/agentexpr.texi
> > @@ -461,7 +461,7 @@ alignment within the bytecode stream; thus, on machines where fetching a
> >  16-bit on an unaligned address raises an exception, you should fetch the
> >  register number one byte at a time.
> >  
> > -@item @code{setv} (0x2d) @var{n}: @result{} @var{v}
> > +@item @code{setv} (0x2d) @var{n}: @var{v} @result{} @var{v}
> >  Set trace state variable number @var{n} to the value found on the top
> >  of the stack.  The stack is unchanged, so that the value is readily
> >  available if the assignment is part of a larger expression.  The
> > 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* pushed (master+branch): two agent expression nits (one line each)
  2015-02-13 19:21             ` David Taylor
@ 2015-02-20  3:05               ` Joel Brobecker
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2015-02-20  3:05 UTC (permalink / raw)
  To: David Taylor; +Cc: gdb-patches

> > These changes are both fine as posted, thanks!
> > 
> > And I agree that they would be good candidates for 7.9 branch as well.
> 
> Will someone please commit these for me (as I do not have commit privs)?

I have now pushed them to both master and gdb-7.9-branch, after having
re-tested them on both branches.

For the future, it would be helpful if you sent the whole commit,
rather than just a ChangeLog, because I had to write the commit's
revision log, which wasn't completely obvious since I didn't grasp
everything. Fortunately, I was able to cook something up from your
initial message (which I had to fish back) and your discussions
with Stan.

Thank you nonetheless for the fixes, and for your patience!

> > > 2015-02-03  David Taylor  <dtaylor@emc.com>
> > > 
> > > 	* common/ax.def (setv): Fix consumed entry in setv DEFOP.
> > > 
> > > gdb/doc:
> > > 
> > > 2015-02-03  David Taylor  <dtaylor@emc.com>
> > > 
> > > 	* agentexpr.texi (Bytecode Descriptions): Fix summary line for setv.

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-02-20  3:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11 17:54 two agent expression nits (one line each) David Taylor
2014-11-23  3:27 ` Joel Brobecker
2014-12-02 20:45 ` Stan Shebs
2014-12-13 13:44   ` Joel Brobecker
2014-12-15 15:41     ` David Taylor
2014-12-20 17:19       ` Joel Brobecker
2015-02-03 21:58         ` David Taylor
2015-02-11  7:49           ` Joel Brobecker
2015-02-11 17:28           ` Stan Shebs
2015-02-13 19:21             ` David Taylor
2015-02-20  3:05               ` pushed (master+branch): " Joel Brobecker

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).