From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26715 invoked by alias); 10 Jul 2002 19:41:30 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26706 invoked from network); 10 Jul 2002 19:41:29 -0000 Received: from unknown (HELO zwingli.cygnus.com) (208.245.165.35) by sources.redhat.com with SMTP; 10 Jul 2002 19:41:29 -0000 Received: by zwingli.cygnus.com (Postfix, from userid 442) id 768795EA11; Wed, 10 Jul 2002 14:41:28 -0500 (EST) To: Daniel Berlin Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: dwarf2expr.[ch] References: From: Jim Blandy Date: Wed, 10 Jul 2002 12:48:00 -0000 In-Reply-To: Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.1 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-07/txt/msg00203.txt.bz2 Daniel Berlin writes: > 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. You're right, it does. I misread 'n' as indexing from bottom to top, not top to bottom. > > > + > > > + 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. I'm worried about about strange addresses being produced because the evaluator is using stack elements wider than officially specified. Since the evaluator is the source of the behavior I'm concerned about, I think it's better to correct it there than to require the surrounding code in GDB to cope with it. You're going to need a truncation function for all the operations that are sensitive to the upper bits anyway (divide, shift right, compare), so it doesn't seem a big deal to drop in an application here, too.