From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-16790-listarch-gdb-patches=sourceware.cygnus.com@sources.redhat.com>
Received: (qmail 11675 invoked by alias); 10 Jul 2002 18:01:54 -0000
Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm
Precedence: bulk
List-Subscribe: <mailto:gdb-patches-subscribe@sources.redhat.com>
List-Archive: <http://sources.redhat.com/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sources.redhat.com>
List-Help: <mailto:gdb-patches-help@sources.redhat.com>, <http://sources.redhat.com/ml/#faqs>
Sender: gdb-patches-owner@sources.redhat.com
Received: (qmail 11653 invoked from network); 10 Jul 2002 18:01:53 -0000
Received: from unknown (HELO www.dberlin.org) (138.88.47.164)
  by sources.redhat.com with SMTP; 10 Jul 2002 18:01:53 -0000
Received: by www.dberlin.org (Postfix, from userid 503)
	id 4509D18003C1; Wed, 10 Jul 2002 14:01:53 -0400 (EDT)
Received: from localhost (localhost.localdomain [127.0.0.1])
	by www.dberlin.org (Postfix) with ESMTP
	id 950C41818961; Wed, 10 Jul 2002 14:01:47 -0400 (EDT)
Date: Wed, 10 Jul 2002 11:18:00 -0000
From: Daniel Berlin <dberlin@dberlin.org>
To: Jim Blandy <jimb@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: dwarf2expr.[ch]
In-Reply-To: <npznx036c6.fsf@zwingli.cygnus.com>
Message-ID: <Pine.LNX.4.44.0207101343380.12859-100000@dberlin.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
X-Spam-Status: No, hits=-1.6 required=5.0
	tests=IN_REP_TO,NO_MX_FOR_FROM,AWL
	version=2.31
X-Spam-Level: 
X-SW-Source: 2002-07/txt/msg00193.txt.bz2

Just some questions and statements:
> > +   return ctx->stack[ctx->stack_len - (1+n)];
> > + 
> > + }     
> 
> This should check for underflow, too.  Look at what DW_OP_rot will do
> on an empty stack.
It does.
Look at the lines above it.
(Yours won't have internal_error, just (ctx->error))
if (ctx->stack_len < n)
     internal_error (__FILE__, __LINE__, "Asked for position %d of stack, 
stack only has %d elements on it\n",
                     n, ctx->stack_len);

If you ask for one item, and the stack has 0, this will catch it.


> > + 
> > + 	  switch (op)
> > + 	    {
> > + 	    case DW_OP_deref:
> > + 	      {
> > + 		result = (CORE_ADDR) 
> > + 		  (ctx->read_mem) (ctx->read_mem_baton, 
> > + 				   result, 
> > + 				   TARGET_PTR_BIT / TARGET_CHAR_BIT);
> 
> Since CORE_ADDR may be wider than the target's address,
> I think this
> should mask off and/or sign extend as appropriate, depending on the
> current gdbarch.  Same anywhere we call ctx->read_mem, I think.
Shouldn't the read_mem function do this for us?
read_mem is returning a CORE_ADDR (the cast is pointless, i'll remove 
it) anyway, so it would seem to be *it's* job to make sure the 
CORE_ADDR it gives us is the right thing.