From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89940 invoked by alias); 9 Jun 2015 19:36:43 -0000 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 Received: (qmail 89930 invoked by uid 89); 9 Jun 2015 19:36:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_40,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 09 Jun 2015 19:36:41 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 8D6D51163D5; Tue, 9 Jun 2015 15:36:39 -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 qzFMTmpfucz8; Tue, 9 Jun 2015 15:36:39 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 744DC1163CD; Tue, 9 Jun 2015 15:36:39 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 05E254067C; Tue, 9 Jun 2015 15:36:39 -0400 (EDT) Date: Tue, 09 Jun 2015 19:36:00 -0000 From: Joel Brobecker To: Jon TURNEY Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Find debug symbols file by buildid for PE file format also Message-ID: <20150609193638.GM2855@adacore.com> References: <1426520230-18940-1-git-send-email-jon.turney@dronecode.org.uk> <831tko29ct.fsf@gnu.org> <55082466.3010702@dronecode.org.uk> <55251C45.4070006@dronecode.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55251C45.4070006@dronecode.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-06/txt/msg00158.txt.bz2 > bfd/ChangeLog: > > 2015-04-07 Jon Turney > > * elf-bfd.h : Remove struct elf_build_id. > * bfd.c : Add struct bfd_build_id. > * bfd-in2.h: Regenerate. > * elf.c (elfobj_grok_gnu_build_id): Update to use bfd_build_id. > * libpei.h: Add protoype and macros for > bfd_XXi_slurp_codeview_record. > * peXXigen.c (_bfd_XXi_slurp_codeview_record): Make public > * peicode.h (pe_bfd_read_buildid): Add. > (pe_bfd_object_p): Use pe_bfd_read_buildid(). The BFD part of the patch needs to be approved by the binutils maintainers. > gdb/ChangeLog: > > 2015-04-07 Jon Turney > > * build-id.c (build_id_bfd_get): Use bfd_build_id. > (build_id_verify): Ditto. > * build-id.h: Ditto. > (find_separate_debug_file_by_buildid): Ditto. > * python/py-objfile.c (objfpy_get_build_id) > (objfpy_build_id_matches, objfpy_lookup_objfile_by_build_id): Ditto. > * coffread.c (coff_symfile_read): Try > find_separate_debug_file_by_buildid. Can you also mention in the ChangeLog the #include changes (removing and adding)? > > gdb/doc/ChangeLog: > > 2015-04-07 Jon Turney > > * gdb.texinfo (Separate Debug Files): Document that PE is also > supported. > > gdb/testsuite/ChangeLog: > > 2015-04-07 Jon Turney > > * gdb.base/sepdebug.exp: Add EXEEXT where needed. > * lib/gdb.exp (get_build_id): Teach how to extract build-id from a > PE file. > * lib/future.exp (gdb_find_objdump): Add gdb_find_objdump. All GDB changes look mostly OK to me. In fact, the only comments are very minor in nature, so this part is pre-approved pending those tiny issues being addressed. > @@ -29,19 +28,19 @@ > > /* See build-id.h. */ > > -const struct elf_build_id * > +const struct bfd_build_id * > build_id_bfd_get (bfd *abfd) > { > - if (!bfd_check_format (abfd, bfd_object) > - || bfd_get_flavour (abfd) != bfd_target_elf_flavour > - /* Although this is ELF_specific, it is safe to do in generic > - code because it does not rely on any ELF-specific symbols at > - link time, and if the ELF code is not available in BFD, then > - ABFD will not have the ELF flavour. */ > - || elf_tdata (abfd)->build_id == NULL) > + if (!bfd_check_format (abfd, bfd_object)) > return NULL; > > - return elf_tdata (abfd)->build_id; > + if (abfd->build_id != NULL) > + { > + return abfd->build_id; > + } Small GDB-specific coding style: For single-statement if blocks, we decided that curly braces should not be used. Therefore: if (abfd->build_id != NULL) return abfd->build_id; The only exception is when you have a comment in addition to the statement. See: https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards > diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp > index 2fb635b..fd4c153 100644 > --- a/gdb/testsuite/lib/future.exp > +++ b/gdb/testsuite/lib/future.exp > @@ -104,6 +104,16 @@ proc gdb_find_objcopy {} { > return $objcopy > } > > +proc gdb_find_objdump {} { We should document every new function, by adding an introductory comment. -- Joel