From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31923 invoked by alias); 9 Sep 2009 05:58:42 -0000 Received: (qmail 31839 invoked by uid 22791); 9 Sep 2009 05:58:40 -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:58:33 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7DFE62BAB82; Wed, 9 Sep 2009 01:58:31 -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 zQDSB5XnAWlZ; Wed, 9 Sep 2009 01:58:31 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 1B1F32BAB22; Wed, 9 Sep 2009 01:58:31 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5F2F4F589B; Tue, 8 Sep 2009 22:58:24 -0700 (PDT) Date: Wed, 09 Sep 2009 05:58:00 -0000 From: Joel Brobecker To: Paul Pluzhnikov Cc: Ulrich Weigand , Ulrich Weigand , gdb-patches ml , Tom Tromey , Jan Kratochvil Subject: Re: [patch] Speed up find_pc_section Message-ID: <20090909055824.GB11738@adacore.com> References: <8ac60eac0908201340k6b759eb5o9bb73c8f473d8785@mail.gmail.com> <200908211130.n7LBUCJc011108@d12av02.megacenter.de.ibm.com> <8ac60eac0908231548x135edf2doa04fa59a49455bcd@mail.gmail.com> <8ac60eac0908260020l4200cf84v2686a76b5858d13@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8ac60eac0908260020l4200cf84v2686a76b5858d13@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/msg00235.txt.bz2 > 2009-08-25 Paul Pluzhnikov > > * objfiles.c (qsort_cmp): Remove asserts. > (insert_section_p, filter_debuginfo_sections): New function. > (filter_overlapping_sections): Likewise. > (update_section_map): Adjust. Looks good to me, save for the few little comments I wrote below. Please wait until Friday to give Ulrich a chance to comment on this patch, since he's been pretty involved in that discussion. But, based on all the information you and him provided, I believe that the patch incorporates all suggestions and should be correct. > +/* Return 1 if SECTION should be inserted into the section map. > + We want to insert only non-overlay and non-TLS section. */ Can you explain what we do not want to add TLS sections? Is that just an optimization (code addresses should never point to TLS)? > +/* Filter out overlapping sections where one section came from the real > + objfile, and the other from a separate debuginfo file. > + Return the size of table after redundant sections have been eliminated. */ > + if (sect1_addr == sect2_addr > + && (objfile1->separate_debug_objfile == objfile2 > + || objfile2->separate_debug_objfile == objfile1)) Looks like "overlapping" above also means start at the same address? Is that normal? Or good enough for our purpose? > +/* Filter out overlapping sections, issuing a warning if any are found. > + Overlapping sections could really be overlay sections which we didn't > + classify as such in insert_section_p, or we could be dealing with a > + corrupt binary. */ I think we should also mention the MacOS port where we load all sections of all .o files instead of just the debugging info. It looks like a design flaw in the MacOS port, but it was really a shortcut in getting things to work (aka a hack). I believe Tristan is planning on fixing this in the relatively near future, but in the meantime, it might be a useful comment. > + warning (_("Unexpected overlap between " > + "section `%s' from `%s' [%s, %s) and " > + "section `%s' from `%s' [%s, %s)"), > + bfd_section_name (abfd1, bfds1), objf1->name, > + paddress (gdbarch, sect1_addr), > + paddress (gdbarch, sect1_endaddr), > + bfd_section_name (abfd2, bfds2), objf2->name, > + paddress (gdbarch, sect2_addr), > + paddress (gdbarch, sect2_endaddr)); Let's please use a complaint rather than a warning. As explained in one of my other messages, the warning causes too much output on MacOS. But I also see, now, after reading Ulrich's messages, that he suggested the same thing. > /* Update PMAP, PMAP_SIZE with non-TLS sections from all objfiles. */ We should update the comment to explain that overlay sections are also eliminated, as well as overlapping sections. -- Joel