From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25495 invoked by alias); 20 May 2009 23:07:31 -0000 Received: (qmail 25265 invoked by uid 22791); 20 May 2009 23:07:28 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 May 2009 23:07:18 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CC18E2BAB94; Wed, 20 May 2009 19:07:16 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 0czet8lsc-CJ; Wed, 20 May 2009 19:07:16 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 893782BABBC; Wed, 20 May 2009 19:07:16 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 6FBEDF5968; Wed, 20 May 2009 23:16:31 +0200 (CEST) Date: Wed, 20 May 2009 23:07:00 -0000 From: Joel Brobecker To: Eli Zaretskii Cc: Daniel Jacobowitz , gdb-patches@sourceware.org Subject: Re: [RFA] Fix "break foo" when `foo's prologue ends before line table Message-ID: <20090520211631.GD16152@adacore.com> References: <83skjebbef.fsf@gnu.org> <20090511125644.GD14773@adacore.com> <83zldjxzzr.fsf@gnu.org> <20090511192709.GG14773@adacore.com> <83tz3rxt4p.fsf@gnu.org> <83eiupqos5.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <83eiupqos5.fsf@gnu.org> User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-05/txt/msg00441.txt.bz2 > 2009-05-09 Eli Zaretskii > > * symtab.c (skip_prologue_using_lineinfo): New function. > (find_function_start_sal): Use it to get to the first line of > function's body that has an entry in the lineinfo table. The change looks OK to me, overall. I do have a couple of little comments, but do feel free to check it in once the comments are addressed. > + /* Get the range for the function's PC values, or give up if we > + cannot, for some reason. */ > + if (!find_pc_partial_function (func_addr, NULL, &func_start, &func_end)) > + return func_addr; > + l = LINETABLE (symtab); > + if (l == NULL) > + return func_addr; Perhaps you want to check for this before doing the lookup above, since you have access to the linetable directly. Although, in practice, we know it should never fail, since the func_addr comes from a symbol. So we should always find the associated partial symbol. > + /* Linetable entries are ordered by PC values, see the commentary in ^^^^^^ comment > + symtab.h where `struct linetable' is defined. Thus, the first > + entry whose PC is in the range [FUNC_START..FUNC_END] is the ^^^ You probably want to say [FUNC_START..FUNC_END[, as FUNC_END should be excluded from the function range. And therefore: > + the table. See the commentary on symtab.h before the ^^^^^^^^ comment > + definition of struct linetable. */ > + if (item->line > 0 && func_start <= item->pc && item->pc <= func_end) ^^ I believe that the second comparison should be a strict compare: item->pc < func_end > + /* If we still don't have a valid source line, try to find the first > + PC in the lineinfo table that belongs to the same function. */ That's just a suggestion, but I wouldn't mind if we provided a little extra explainations about the situation where we have encountered this. That way, when we later come back to this and try to determine whether this is still relelvant or not, we'll find a little more context. Something saying that this can happen if the first line in lineinfo data for our function starts after the end of the function prologue. This was observed with DJGPP in a function where the prologue was followed by some code realigning the stack, before the "real" code started. The line info for that function started at the beginning of the "real" code. So, there was no line info available for the instruction after the prologue. Something like that. Cheers, -- Joel