From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29624 invoked by alias); 23 May 2009 10:20:35 -0000 Received: (qmail 29616 invoked by uid 22791); 23 May 2009 10:20:33 -0000 X-SWARE-Spam-Status: No, hits=-0.3 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_32,J_CHICKENPOX_36,KAM_STOCKGEN,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout6.012.net.il (HELO mtaout6.012.net.il) (84.95.2.16) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 23 May 2009 10:20:26 +0000 Received: from conversion-daemon.i-mtaout6.012.net.il by i-mtaout6.012.net.il (HyperSendmail v2007.08) id <0KK300300DYHD900@i-mtaout6.012.net.il> for gdb-patches@sourceware.org; Sat, 23 May 2009 13:18:55 +0300 (IDT) Received: from HOME-C4E4A596F7 ([84.228.115.215]) by i-mtaout6.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0KK300KBADZIJ570@i-mtaout6.012.net.il>; Sat, 23 May 2009 13:18:55 +0300 (IDT) Date: Sat, 23 May 2009 10:20:00 -0000 From: Eli Zaretskii Subject: Re: [RFA] Fix "break foo" when `foo's prologue ends before line table In-reply-to: <20090520211631.GD16152@adacore.com> To: Joel Brobecker Cc: drow@false.org, gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83k548t94h.fsf@gnu.org> 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> <20090520211631.GD16152@adacore.com> X-IsSubscribed: yes 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/msg00504.txt.bz2 > Date: Wed, 20 May 2009 14:16:31 -0700 > From: Joel Brobecker > Cc: Daniel Jacobowitz , gdb-patches@sourceware.org > > > 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. Thanks. The patch I actually committed is reproduced below. > > + /* 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. I moved the test of l == NULL before the call to find_pc_partial_function. As for always finding the partial symbol: a single test instruction is not expensive enough, IMO, to justify giving up defensive code, even if it should always succeed. So I left it in place. > > + /* Linetable entries are ordered by PC values, see the commentary in > ^^^^^^ comment Hmm? are you saying that "commentary" is incorrect English? If so, I beg to differ. ;-) > > + 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: Right, fixed. > 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. Is the comment I added before the call to skip_prologue_using_lineinfo good enough? 2009-05-23 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. --- symtab.c~0 2009-04-01 02:21:07.000000000 +0300 +++ symtab.c 2009-05-23 12:52:12.609375000 +0300 @@ -2599,6 +2599,47 @@ find_function_start_pc (struct gdbarch * return pc; } +/* Given a function start address FUNC_ADDR and SYMTAB, find the first + address for that function that has an entry in SYMTAB's line info + table. If such an entry cannot be found, return FUNC_ADDR + unaltered. */ +CORE_ADDR +skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab) +{ + CORE_ADDR func_start, func_end; + struct linetable *l; + int ind, i, len; + int best_lineno = 0; + CORE_ADDR best_pc = func_addr; + + /* Give up if this symbol has no lineinfo table. */ + l = LINETABLE (symtab); + if (l == NULL) + return func_addr; + + /* 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; + + /* Linetable entries are ordered by PC values, see the commentary in + symtab.h where `struct linetable' is defined. Thus, the first + entry whose PC is in the range [FUNC_START..FUNC_END[ is the + address we are looking for. */ + for (i = 0; i < l->nitems; i++) + { + struct linetable_entry *item = &(l->item[i]); + + /* Don't use line numbers of zero, they mark special entries in + the table. See the commentary on symtab.h before the + definition of struct linetable. */ + if (item->line > 0 && func_start <= item->pc && item->pc < func_end) + return item->pc; + } + + return func_addr; +} + /* Given a function symbol SYM, find the symtab and line for the start of the function. If the argument FUNFIRSTLINE is nonzero, we want the first line @@ -2649,6 +2690,21 @@ find_function_start_sal (struct symbol * sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0); } + /* 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. This + happens with COFF debug info, which does not seem to have an + entry in lineinfo table for the code after the prologue which has + no direct relation to source. For example, this was found to be + the case with the DJGPP target using "gcc -gcoff" when the + compiler inserted code after the prologue to make sure the stack + is aligned. */ + if (funfirstline && sal.symtab == NULL) + { + pc = skip_prologue_using_lineinfo (pc, SYMBOL_SYMTAB (sym)); + /* Recalculate the line number. */ + sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0); + } + sal.pc = pc; return sal;