Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Stan Shebs <stanshebs@earthlink.net>
Cc: David Taylor <dtaylor@emc.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: two agent expression nits (one line each)
Date: Sat, 13 Dec 2014 13:44:00 -0000	[thread overview]
Message-ID: <20141213134428.GF5457@adacore.com> (raw)
In-Reply-To: <547E24E5.8050908@earthlink.net>

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


  reply	other threads:[~2014-12-13 13:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-11 17:54 David Taylor
2014-11-23  3:27 ` Joel Brobecker
2014-12-02 20:45 ` Stan Shebs
2014-12-13 13:44   ` Joel Brobecker [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141213134428.GF5457@adacore.com \
    --to=brobecker@adacore.com \
    --cc=dtaylor@emc.com \
    --cc=gdb-patches@sourceware.org \
    --cc=stanshebs@earthlink.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox