From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15550 invoked by alias); 9 Jan 2015 20:11:35 -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 15532 invoked by uid 89); 9 Jan 2015 20:11:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 09 Jan 2015 20:11:30 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t09KB7lE006786 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 9 Jan 2015 15:11:07 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t09KB4Yj009473; Fri, 9 Jan 2015 15:11:05 -0500 Message-ID: <54B035D8.6010003@redhat.com> Date: Fri, 09 Jan 2015 20:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Mark Kettenis CC: arnez@linux.vnet.ibm.com, jan.kratochvil@redhat.com, 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> <201501091659.t09GxO1q016197@glazunov.sibelius.xs4all.nl> <54B00D92.4050409@redhat.com> <201501091935.t09JZA6f017629@glazunov.sibelius.xs4all.nl> In-Reply-To: <201501091935.t09JZA6f017629@glazunov.sibelius.xs4all.nl> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-01/txt/msg00232.txt.bz2 On 01/09/2015 07:35 PM, Mark Kettenis wrote: >> Date: Fri, 09 Jan 2015 17:19:14 +0000 >> From: Pedro Alves >> >> On 01/09/2015 04:59 PM, Mark Kettenis wrote: >>>> Date: Fri, 09 Jan 2015 16:27:12 +0000 >>>> From: Pedro Alves >>>> >>>>> 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)); >>>> >>>> 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. >>> >>> The code is designed to allow these sections to grow such that the OS >>> kernel can add more registers without breaking GDB. >> >> Not sure what you're disagreeing with. My comment is in that direction >> too (And Andreas' comment I'm quoting). That is, get_core_register_section >> would warn, but still continue processing the section. >> >> The current code clearly does not work that way, given the assertions. > > It shouldn't warn if the sections is bigger that "expected", because > in some cases the "expected" size is really the minimum supported > size, where later versions of the OS added extra information. At > least not unconditionally. I think we're saying the same thing, but what I'm calling "expected", you're calling "maximum". As in, consider the case where GDB about a regset section that is supposed to have size A. GDB is taught about this, with "minimum" == A, and "expected/maximum" == A. Later at some point, a new variant of the machine appears with more registers, and the regset is extended, to size B. A GDB that only knows about A encounters a core dump with B, and thus issues a warning (which suggests that either more info is available that gdb doesn't grok, or the core is broken), but still presents the A registers to the user. Later, someone teaches GDB about B registers, and at that point, "minimum" stays A, but "expected/maximum" is set to B. At some point, if the regset is extended further to C, a GDB that knows about A and B warns when it sees C. And on and on. I think we've already seen something like that with the x86 xsave regset? > I can imagine extending the interface to also specify a maximum size > and interpreting a maximum size of 0 as "no maximum". Continiung > after printing a warning if the section is larger than the maximum > size probably makes sense. Thanks, Pedro Alves