From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21489 invoked by alias); 5 Mar 2012 14:32:50 -0000 Received: (qmail 21480 invoked by uid 22791); 5 Mar 2012 14:32:49 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_AV,TW_EG,TW_VX,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Mar 2012 14:32:35 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q25EWYeU023891 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 5 Mar 2012 09:32:34 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q25EWWc7000534; Mon, 5 Mar 2012 09:32:33 -0500 Message-ID: <4F54CE80.30204@redhat.com> Date: Mon, 05 Mar 2012 14:32:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: GDB Patches CC: Mark Kettenis , "H.J. Lu" Subject: i387-tdep.c:i387_supply_xsave: Assert the xsave section buffer, is not NULL. References: <4F21A489.2080200@redhat.com> In-Reply-To: <4F21A489.2080200@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: 2012-03/txt/msg00135.txt.bz2 On 01/26/2012 07:07 PM, Pedro Alves wrote: > > I have a doubt in the xsave-in-corefile support bits. There's code in place to > handle a NULL regs (as in no xsave contents to work with), so I'm handling it > as presently: > > + > + Note however, the case when REGS is NULL is a different case. > + That case means we do not have access to the x87 states, so we > + should mark the registers as unavailable (by supplying NULL). */ > + > > but I can't figure out how would we ever get a NULL REGS there. Is there a > convoluted path I missed? amd64-linux-tdep.c unconditionally installs > amd64_linux_regset_sections as gdbarch_core_regset_sections > callback, and this includes the .reg-xstate section. > However, corelow.c:get_core_register_section bails early if > a section is not found in the core, never reaching regset->supply_regset > with a NULL `contents'. On 02/27/2012 06:48 PM, Pedro Alves wrote: > I'll post a separate RFC patch proposing to remove the NULL REGS handling Here's what I'd like to check in, given the analysis quoted above. 2012-03-05 Pedro Alves * i387-tdep.c (i387_supply_xsave): Assert the xsave section buffer is not NULL, and remove resulting dead code. --- gdb/i387-tdep.c | 33 +++++++++++---------------------- 1 files changed, 11 insertions(+), 22 deletions(-) diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index 3fc344d..26e4e50 100644 --- a/gdb/i387-tdep.c +++ b/gdb/i387-tdep.c @@ -790,6 +790,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, all = x87 | sse | avxh } regclass; + gdb_assert (regs != NULL); gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM); gdb_assert (tdep->num_xmm_regs > 0); @@ -807,7 +808,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, else regclass = none; - if (regs != NULL && regclass != none) + if (regclass != none) { /* Get `xstat_bv'. */ const gdb_byte *xstate_bv_p = XSAVE_XSTATE_BV_ADDR (regs); @@ -826,11 +827,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, time in a program. This means that from the user-space programs' perspective, it's the same as if the registers have always been zero from the start of the program. Therefore, the debugger - should provide the same illusion to the user. - - Note however, the case when REGS is NULL is a different case. - That case means we do not have access to the x87 states, so we - should mark the registers as unavailable (by supplying NULL). */ + should provide the same illusion to the user. */ switch (regclass) { @@ -839,7 +836,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, case avxh: if ((clear_bv & I386_XSTATE_AVX)) - regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, regnum, zero); else regcache_raw_supply (regcache, regnum, XSAVE_AVXH_ADDR (tdep, regs, regnum)); @@ -847,7 +844,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, case sse: if ((clear_bv & I386_XSTATE_SSE)) - regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, regnum, zero); else regcache_raw_supply (regcache, regnum, FXSAVE_ADDR (tdep, regs, regnum)); @@ -855,7 +852,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, case x87: if ((clear_bv & I386_XSTATE_X87)) - regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, regnum, zero); else regcache_raw_supply (regcache, regnum, FXSAVE_ADDR (tdep, regs, regnum)); @@ -870,7 +867,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, for (i = I387_YMM0H_REGNUM (tdep); i < I387_YMMENDH_REGNUM (tdep); i++) - regcache_raw_supply (regcache, i, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, i, zero); } else { @@ -890,7 +887,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, for (i = I387_XMM0_REGNUM (tdep); i < I387_MXCSR_REGNUM (tdep); i++) - regcache_raw_supply (regcache, i, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, i, zero); } else { @@ -909,7 +906,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, for (i = I387_ST0_REGNUM (tdep); i < I387_FCTRL_REGNUM (tdep); i++) - regcache_raw_supply (regcache, i, regs == NULL ? NULL : zero); + regcache_raw_supply (regcache, i, zero); } else { @@ -926,12 +923,6 @@ i387_supply_xsave (struct regcache *regcache, int regnum, for (i = I387_FCTRL_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++) if (regnum == -1 || regnum == i) { - if (regs == NULL) - { - regcache_raw_supply (regcache, i, NULL); - continue; - } - /* Most of the FPU control registers occupy only 16 bits in the xsave extended state. Give those a special treatment. */ if (i != I387_FIOFF_REGNUM (tdep) @@ -982,10 +973,8 @@ i387_supply_xsave (struct regcache *regcache, int regnum, } if (regnum == I387_MXCSR_REGNUM (tdep) || regnum == -1) - { - p = regs == NULL ? NULL : FXSAVE_MXCSR_ADDR (regs); - regcache_raw_supply (regcache, I387_MXCSR_REGNUM (tdep), p); - } + regcache_raw_supply (regcache, I387_MXCSR_REGNUM (tdep), + FXSAVE_MXCSR_ADDR (regs)); } /* Similar to i387_collect_fxsave, but use XSAVE extended state. */