From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13725 invoked by alias); 9 Sep 2009 05:39:06 -0000 Received: (qmail 13530 invoked by uid 22791); 9 Sep 2009 05:39:04 -0000 X-SWARE-Spam-Status: No, hits=-2.5 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, 09 Sep 2009 05:38:58 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 2EE632BAB8B; Wed, 9 Sep 2009 01:38:56 -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 b8KRV9e-t+Rd; Wed, 9 Sep 2009 01:38:56 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 9F3E42BAB8D; Wed, 9 Sep 2009 01:38:55 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 45A03F589B; Tue, 8 Sep 2009 22:38:48 -0700 (PDT) Date: Wed, 09 Sep 2009 05:39:00 -0000 From: Joel Brobecker To: Paul Pluzhnikov Cc: Ulrich Weigand , Ulrich Weigand , gdb-patches ml , Tom Tromey Subject: Re: [patch] Speed up find_pc_section Message-ID: <20090909053848.GA11738@adacore.com> References: <8ac60eac0908201340k6b759eb5o9bb73c8f473d8785@mail.gmail.com> <200908211130.n7LBUCJc011108@d12av02.megacenter.de.ibm.com> <8ac60eac0908231548x135edf2doa04fa59a49455bcd@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8ac60eac0908231548x135edf2doa04fa59a49455bcd@mail.gmail.com> 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-09/txt/msg00234.txt.bz2 > > (A cleaner fix might be to fix all callers to never pass in a NULL > > section argument --most already don't-- and simply rely on it.) > > Attached patch does that ... > Not tested (I still haven't figured out how to test overlays). This is just an informal review of the patch, since it has not been tested (I assume one would like to test it in an overlay environment as well - maybe Ulrich could take care of that; otherwise we'll have to do without). I feel like a dummy, sometimes, but it took me a while to understand why find_pc_section might return NULL if PC is inside an overlay section. I guess I overlooked the fact that the section might not be mapped... > 2009-08-23 Paul Pluzhnikov > > * minsyms.c (lookup_minimal_symbol_by_pc_section_1): Assume > non-NULL section. The function description needs to be updated. I would also add an assertion that section is not NULL. > (lookup_minimal_symbol_by_pc_section): Check for NULL section. > (lookup_minimal_symbol_by_pc): Adjust. Looks like lookup_minimal_symbol_by_pc is now almost a duplicate of lookup_minimal_symbol_by_pc_section. We could replace its implementation by: struct minimal_symbol * lookup_minimal_symbol_by_pc (CORE_ADDR pc) { return lookup_minimal_symbol_by_pc_section (pc, NULL); } The comment from AndrewC needs to be preserved, but applies equally to lookup_minimal_symbol_by_pc_section, I believe. > /* We can not require the symbol found to be in pc_section, because ^^^^^^^^^^ section > e.g. IRIX 6.5 mdebug relies on this code returning an absolute Otherwise, the patch seems fine. I would love some input from Ulrich, though. -- Joel