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: Tue, 10 May 2011 14:15:00 -0000	[thread overview]
Message-ID: <m3hb92skvb.fsf@fleche.redhat.com> (raw)
In-Reply-To: <201105092202.p49M2anm000447@d06av02.portsmouth.uk.ibm.com>	(Ulrich Weigand's message of "Tue, 10 May 2011 00:02:35 +0200 (CEST)")

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

Ulrich> I guess I still find it a bit odd why we need those
Ulrich> DWARF-specific types in the first place (apparently solely for
Ulrich> DW_OP_mod), but if we really want to be completely compatible
Ulrich> with the prior behaviour, it seems we have to.

Yes, the reason is DW_OP_mod.

"Old-style" DWARF values are untyped.  We normally represent them as
signed ints of the appropriate size (DWARF says unsigned, but it is
simpler to use signed and gets the same results as long as the
arithmetic respects word size).  For DW_OP_mod, we must convert the
values to unsigned -- but we must not do this for values on the stack
that have an explicit type.  So, we need a special type we can
recognize, one that cannot be the same as any base type from the CU.

I will add a comment about this to the code.

Ulrich> B.t.w. your patch always performs an unsigned shift for
Ulrich> DW_OP_shr, even on signed operands.  However, for DW_OP_shra,
Ulrich> your patch respects the sign of the operands and might actually
Ulrich> perform an unsigned shift (even though the opcode explicitly
Ulrich> says "arithmetic right shift" ...)  This looks like another of
Ulrich> those signed/unsigned inconsistencies with the proposal to me.

Yes.  My understanding is that for new-style typed values, DW_OP_shr and
DW_OP_shra are actually the same -- the type indicates the operation to
perform.  But, for old-style values, we must cast to unsigned for
DW_OP_shr.

Ulrich> Looking into that, your code does run through value_as_address, but
Ulrich> since the platform does not define gdbarch_integer_to_address, this
Ulrich> falls through to unpack_long, and since the underlying type is a
Ulrich> TYPE_CODE_INT and not a pointer type, this in turn now respects
Ulrich> the TYPE_UNSIGNED flag and does a signed conversion ...

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

I will try to find a suitable PPC machine for testing.

Tom

diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index 3f6f277..bd3d4d7 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -215,6 +215,8 @@ dwarf_expr_fetch_address (struct dwarf_expr_context *ctx, int n)
   struct value *result_val = dwarf_expr_fetch (ctx, n);
 
   dwarf_require_integral (value_type (result_val));
+  result_val = value_cast (builtin_type (ctx->gdbarch)->builtin_data_ptr,
+			   result_val);
   return value_as_address (result_val);
 }
 


  reply	other threads:[~2011-05-10 14:15 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 [this message]
2011-05-11  0:15           ` Ulrich Weigand
2011-05-11 14:59             ` Tom Tromey
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=m3hb92skvb.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