Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: "Ulrich Weigand" <uweigand@de.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: RFC: implement typed DWARF stack
Date: Wed, 11 May 2011 14:59:00 -0000	[thread overview]
Message-ID: <m3ipthp9kr.fsf@fleche.redhat.com> (raw)
In-Reply-To: <201105110015.p4B0FPSB032445@d06av02.portsmouth.uk.ibm.com>	(Ulrich Weigand's message of "Wed, 11 May 2011 02:15:25 +0200 (CEST)")

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> However, you don't really need to look specifically into Cell tests;
Ulrich> with your patch applied, I'm seeing a whole bunch of test suite
Ulrich> regressions in an -m32 test on ppc64.

Thanks, I will look into this.

Ulrich> I see.  However, the code as implemented casts *all* signed values to
Ulrich> unsigned for DW_OP_shr, not just old-style values.  That's what got
Ulrich> me confused ...

Oops, thanks for noticing that.  I fixed it locally.

Tom> What if we add a cast to dwarf_expr_fetch_address, like the appended?
Tom> I am not really sure whether this is ok.

Ulrich> Huh, interesting approach.  In a sense, that might be OK, since
Ulrich> it mirrors what we're doing in dwarf_expr_read_reg by calling
Ulrich> address_from_register.  On the other hand, I'm not sure
Ulrich> value_cast always does the right thing if the size of a pointer
Ulrich> type differs from the size of the DWARF address type ...

I had not considered that as a possibility.  I think the most obviously
safe thing to do is just revert dwarf_expr_fetch_address to (mostly)
resemble its pre-patch state.  I will do that and test it.

Ulrich> Another issue that just occurred to me: your patch creates
Ulrich> possibly many temporary struct value objects.  I'm wondering
Ulrich> whether those ought to be released from the value chain at some
Ulrich> point ...

I considered this but talked myself out of it using the following
reasoning:

1. Most DWARF expressions are simple, so in practice not many values
   will be released;
2. The unwinder code is value based but does not seem to call
   value_free_to_mark, so it must not be significant there;
3. In other (expression-evaluation) contexts, some caller is going to
   free the values anyway;
4. The watchpoint code looks at the value stack to determine what
   intermediate values to watch, and perhaps the values from the DWARF
   expression are relevant (though ... it occurs to me just now that
   this approach must be pretty broken in the presence of location
   lists).

I am actually not sure if #4 is an argument for or against.  Maybe those
intermediate values confuse things; there is a comment in
value_fetch_lazy indicating that this may be the case.

Tom


  reply	other threads:[~2011-05-11 14:59 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-04 20:48 Tom Tromey
2011-05-05 16:47 ` Tom Tromey
2011-05-05 18:07 ` Ulrich Weigand
2011-05-05 18:38   ` Tom Tromey
2011-05-05 20:15     ` Tom Tromey
2011-05-09 22:02       ` Ulrich Weigand
2011-05-10 14:15         ` Tom Tromey
2011-05-11  0:15           ` Ulrich Weigand
2011-05-11 14:59             ` Tom Tromey [this message]
2011-05-11 19:44               ` Tom Tromey
2011-05-12  0:03               ` Ulrich Weigand
2011-05-12 16:33                 ` Tom Tromey
2011-05-13  7:52                   ` Regression: " Jan Kratochvil
2011-05-13 15:44                     ` Tom Tromey
2011-05-15  8:26                       ` gdbindex crash: " Jan Kratochvil
2011-05-16 17:37                         ` Tom Tromey
2011-05-17 17:01                           ` Tom Tromey
2011-05-13 17:17                     ` Tom Tromey
2011-05-13 17:34                       ` Jan Kratochvil
2011-05-12 19:32             ` Tom Tromey
2011-05-16 15:50               ` Ulrich Weigand
2011-05-16 18:09                 ` Tom Tromey
2011-05-17  8:35                   ` Jakub Jelinek
2011-06-03 13:52                     ` Tom Tromey
2011-05-10 16:39         ` Tom Tromey

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=m3ipthp9kr.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=uweigand@de.ibm.com \
    /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