From patchwork Fri Jan 9 19:27:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Arnez X-Patchwork-Id: 4598 Received: (qmail 27089 invoked by alias); 9 Jan 2015 19:27:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27074 invoked by uid 89); 9 Jan 2015 19:27:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e06smtp12.uk.ibm.com Received: from e06smtp12.uk.ibm.com (HELO e06smtp12.uk.ibm.com) (195.75.94.108) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 09 Jan 2015 19:27:20 +0000 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 9 Jan 2015 19:27:17 -0000 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 9 Jan 2015 19:27:15 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 05E901B08040 for ; Fri, 9 Jan 2015 19:27:49 +0000 (GMT) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t09JRENa61145266 for ; Fri, 9 Jan 2015 19:27:14 GMT Received: from d06av05.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t09JREJD012389 for ; Fri, 9 Jan 2015 12:27:14 -0700 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-115.boeblingen.de.ibm.com [9.152.212.115]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t09JRDTs012385; Fri, 9 Jan 2015 12:27:14 -0700 From: Andreas Arnez To: Pedro Alves Cc: Jan Kratochvil , gdb-patches@sourceware.org Subject: Re: [testsuite patch] for: [PATCH] [PR corefiles/17808] i386: Fix internal error when prstatus in core file is too big References: <874ms18cyz.fsf@br87z6lw.de.ibm.com> <20150108164327.GA29029@host2.jankratochvil.net> <87zj9s70bh.fsf@br87z6lw.de.ibm.com> <54B00160.5000309@redhat.com> Date: Fri, 09 Jan 2015 20:27:14 +0100 In-Reply-To: <54B00160.5000309@redhat.com> (Pedro Alves's message of "Fri, 09 Jan 2015 16:27:12 +0000") Message-ID: <87vbkf7o19.fsf@br87z6lw.de.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15010919-0009-0000-0000-000002B370E2 X-IsSubscribed: yes On Fri, Jan 09 2015, Pedro Alves wrote: >> Any other comments? > > Do we need to do the same in other places? This grep seems to suggest yes: > > $ grep assert * | grep sizeof | grep regset > amd64obsd-tdep.c: gdb_assert (len >= tdep->sizeof_gregset + I387_SIZEOF_FXSAVE); > amd64-tdep.c: gdb_assert (len == tdep->sizeof_fpregset); > amd64-tdep.c: gdb_assert (len == tdep->sizeof_fpregset); > i386obsd-tdep.c: gdb_assert (len >= tdep->sizeof_gregset + I387_SIZEOF_FSAVE); > i386-tdep.c: gdb_assert (len == tdep->sizeof_gregset); > i386-tdep.c: gdb_assert (len == tdep->sizeof_gregset); > i386-tdep.c: gdb_assert (len == tdep->sizeof_fpregset); > i386-tdep.c: gdb_assert (len == tdep->sizeof_fpregset); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips_elf_gregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips_elf_gregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips_elf_fpregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips_elf_fpregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips64_elf_gregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips64_elf_gregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips64_elf_fpregset_t)); > mips-linux-tdep.c: gdb_assert (len == sizeof (mips64_elf_fpregset_t)); > mn10300-linux-tdep.c: gdb_assert (len == sizeof (mn10300_elf_gregset_t)); > mn10300-linux-tdep.c: gdb_assert (len == sizeof (mn10300_elf_fpregset_t)); > mn10300-linux-tdep.c: gdb_assert (len == sizeof (mn10300_elf_gregset_t)); Right, these should be handled as well. Once we agree on how to handle them, I can provide an updated patch. > On 01/08/2015 04:16 PM, Andreas Arnez wrote: >> Note that this behavior deviates from the default policy: In general, if >> some future kernel adds new registers to a register set, then a GDB >> unaware of this extension would read the known subset and just ignore >> the unknown bytes. > > That's a good point. > > get_core_register_section checks the section size already: > > get_core_register_section (struct regcache *regcache, > const struct regset *regset, > const char *name, > int min_size, > int which, > const char *human_name, > int required) > { > ... > size = bfd_section_size (core_bfd, section); > if (size < min_size) > { > warning (_("Section `%s' in core file too small."), section_name); > return; > } > ... > > Should we remove all those asserts, and make it the > job of get_core_register_section to warn if the section > size is bigger than expected? We may need to pass > the "expected" section size to the callback, in addition > to the "minimum" size though. Good point. Or maybe instead of adding the expected size to the callback we could allow the regset supply functions to pass back their "recognized" size? For instance: (And then, for symmetry, the regset collect functions could also indicate how much data they produced, i.e., how large the resulting register section should be. But I'm not sure whether there is currently any application for this.) A more "lazy" approach would be to assume that the "recognized" size always equals the "minimum" size. Then we could emit the warning like this: if (size < min_size) { warning (_("Section `%s' in core file too small."), section_name); return; } else if (size != min_size) warning (_("Section `%s' in core file has unexpected size."), section_name); This is certainly less flexible, but might be good enough. diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 0750506..3ad491d 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -3720,14 +3720,14 @@ i386_value_to_register (struct frame_info *frame, int regnum, void i386_supply_gregset (const struct regset *regset, struct regcache *regcache, - int regnum, const void *gregs, size_t len) + int regnum, const void *gregs, size_t *len) { struct gdbarch *gdbarch = get_regcache_arch (regcache); const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); const gdb_byte *regs = gregs; int i; - gdb_assert (len == tdep->sizeof_gregset); + *len = tdep->sizeof_gregset; for (i = 0; i < tdep->gregset_num_regs; i++) {