Message ID | 1503329347-26711-4-git-send-email-yao.qi@linaro.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 126614 invoked by alias); 21 Aug 2017 15:29:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 126380 invoked by uid 89); 21 Aug 2017 15:29:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:sk:static. X-HELO: mail-it0-f66.google.com Received: from mail-it0-f66.google.com (HELO mail-it0-f66.google.com) (209.85.214.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 21 Aug 2017 15:29:19 +0000 Received: by mail-it0-f66.google.com with SMTP id s132so10060434ita.1 for <gdb-patches@sourceware.org>; Mon, 21 Aug 2017 08:29:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=tVENVkHHdHDM8e1Ar1oRINvAud0iZm/rnbBioVP+M9A=; b=eGjgX7KiMVTjDYa6gx+KzFXf/bBjQLiZ+al5c0TEHFzi+rvSmxmEyh6mDUob20r/7v YEIbIN6uy00fH+L6NnMLtZ8Fb+T1H0LCPhXD5WRGxVsZQ0s45+u4JQU4uiMC9HEcy6iG CJaKcn7znxgNmO4bqTs0tGTYD0U4Sm6Vtb9RBUS0ySQJHmM5tDoxfQbgxkkFY1CkoGPn gvSJbEA1Rc0e6IXC1EIwcPoV8o5/o2dcQfpMZc9H+NEXcZQM99LWV4UVvf1yj9+4csC0 9QjsX8/vGxTiZMyPfXd3BJE8eL/D00WufxkuN1WnpcEtr5NwArxggAfPUMb10YJ46zo4 7XOw== X-Gm-Message-State: AHYfb5hHgWkdlVjxDtj368dWlqAq1x/PvMjI6zwlaRn99gdQYLZ/QX5+ 4+OSin6Dd0nl5tB+ X-Received: by 10.36.167.5 with SMTP id a5mr577716itf.184.1503329357878; Mon, 21 Aug 2017 08:29:17 -0700 (PDT) Received: from E107787-LIN.cambridge.arm.com (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id p62sm5831589ioe.34.2017.08.21.08.29.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 21 Aug 2017 08:29:17 -0700 (PDT) From: Yao Qi <qiyaoltc@gmail.com> X-Google-Original-From: Yao Qi <yao.qi@linaro.org> To: gdb-patches@sourceware.org Cc: jhb@freebsd.org Subject: [PATCH 03/22] Return X86_XSTATE_SSE_MASK instead of 0 in i386fbsd_core_read_xcr0 Date: Mon, 21 Aug 2017 16:28:48 +0100 Message-Id: <1503329347-26711-4-git-send-email-yao.qi@linaro.org> In-Reply-To: <1503329347-26711-1-git-send-email-yao.qi@linaro.org> References: <1503329347-26711-1-git-send-email-yao.qi@linaro.org> X-IsSubscribed: yes |
Commit Message
Yao Qi
Aug. 21, 2017, 3:28 p.m. UTC
i386fbsd_core_read_xcr0 reads the value of xcr0 from the corefile. If it fails, returns 0. This makes its caller {i386,amd64}_target_description has to handle this special value. IMO, i386fbsd_core_read_xcr0 should return the default xcr0 in case of error. gdb: 2017-08-21 Yao Qi <yao.qi@linaro.org> * i386-fbsd-tdep.c (i386fbsd_core_read_xcr0): Return X86_XSTATE_SSE_MASK instead of 0. --- gdb/i386-fbsd-tdep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Monday, August 21, 2017 04:28:48 PM Yao Qi wrote: > i386fbsd_core_read_xcr0 reads the value of xcr0 from the corefile. If > it fails, returns 0. This makes its caller {i386,amd64}_target_description > has to handle this special value. IMO, i386fbsd_core_read_xcr0 should > return the default xcr0 in case of error. > > gdb: > > 2017-08-21 Yao Qi <yao.qi@linaro.org> > > * i386-fbsd-tdep.c (i386fbsd_core_read_xcr0): Return > X86_XSTATE_SSE_MASK instead of 0. > --- > gdb/i386-fbsd-tdep.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c > index 594b8f6..baca978 100644 > --- a/gdb/i386-fbsd-tdep.c > +++ b/gdb/i386-fbsd-tdep.c > @@ -248,14 +248,14 @@ i386fbsd_core_read_xcr0 (bfd *abfd) > { > warning (_("Couldn't read `xcr0' bytes from " > "`.reg-xstate' section in core file.")); > - return 0; > + return X86_XSTATE_SSE_MASK; > } > > xcr0 = bfd_get_64 (abfd, contents); > } > } > else > - xcr0 = 0; > + xcr0 = X86_XSTATE_SSE_MASK; > > return xcr0; > } I think this should actually be X86_XSTATE_MMX_MASK. Core dumps on FreeBSD/i386 only include the original 387 FPU state in .reg2, they do not write out SSE state in a separate note as Linux does. For i386 native FreeBSD (and probably other *BSD) targets the logic needs to similarly be a bit more complicated, though I can help. In particular, the 'static int have_ptrace_xmmregs' in i386-bsd-nat.c probably needs to be made non-static with an extern in 'i386-bsd-nat.h', and i386fbsd_read_description should try to use PT_GETXMMREGS once to probe it if it isn't set (it can just fetch the gdb process' registers to test the flag) and then select X86_XSTATE_SSE_MASK if there is no XSAVE support for PT_GETXMMREGS works, else use X86_XSTATE_MMX_MASK. Other BSD's don't have a target read description target method, so only i386-fbsd-nat.c would need to have its method updated. I could always work on this as a followup.
John Baldwin <jhb@freebsd.org> writes: > I think this should actually be X86_XSTATE_MMX_MASK. Core dumps on FreeBSD/i386 s/X86_XSTATE_MMX_MASK/X86_XSTATE_X87_MASK/ ? > only include the original 387 FPU state in .reg2, they do not write > out SSE state > in a separate note as Linux does. > FAOD, the existing code (without my patches) get SSE target description tdesc_i386 in default. If we should use MMX target description tdesc_i386_mmx in this case, we can change it after my patch #4, in which i386_target_description returns tdesc_i386_mmx for X86_XSTATE_X87_MASK. > For i386 native FreeBSD (and probably other *BSD) targets the logic needs to > similarly be a bit more complicated, though I can help. In particular, the > 'static int have_ptrace_xmmregs' in i386-bsd-nat.c probably needs to be made > non-static with an extern in 'i386-bsd-nat.h', and i386fbsd_read_description > should try to use PT_GETXMMREGS once to probe it if it isn't set (it can just > fetch the gdb process' registers to test the flag) and then select > X86_XSTATE_SSE_MASK if there is no XSAVE support for PT_GETXMMREGS works, > else use X86_XSTATE_MMX_MASK. Other BSD's don't have a target read description > target method, so only i386-fbsd-nat.c would need to have its method updated. > I could always work on this as a followup. To be clear, can I commit this patch as-is?
On Monday, August 21, 2017 05:45:50 PM Yao Qi wrote: > John Baldwin <jhb@freebsd.org> writes: > > > I think this should actually be X86_XSTATE_MMX_MASK. Core dumps on FreeBSD/i386 > > s/X86_XSTATE_MMX_MASK/X86_XSTATE_X87_MASK/ ? Yes. > > only include the original 387 FPU state in .reg2, they do not write > > out SSE state > > in a separate note as Linux does. > > > > FAOD, the existing code (without my patches) get SSE target description > tdesc_i386 in default. If we should use MMX target description > tdesc_i386_mmx in this case, we can change it after my patch #4, in > which i386_target_description returns tdesc_i386_mmx for X86_XSTATE_X87_MASK. Yes, this is an old bug. I think it is fine to just reorder this after patch 4 so that returning X87_MASK will do the right thing. > > For i386 native FreeBSD (and probably other *BSD) targets the logic needs to > > similarly be a bit more complicated, though I can help. In particular, the > > 'static int have_ptrace_xmmregs' in i386-bsd-nat.c probably needs to be made > > non-static with an extern in 'i386-bsd-nat.h', and i386fbsd_read_description > > should try to use PT_GETXMMREGS once to probe it if it isn't set (it can just > > fetch the gdb process' registers to test the flag) and then select > > X86_XSTATE_SSE_MASK if there is no XSAVE support for PT_GETXMMREGS works, > > else use X86_XSTATE_MMX_MASK. Other BSD's don't have a target read description > > target method, so only i386-fbsd-nat.c would need to have its method updated. > > I could always work on this as a followup. > > To be clear, can I commit this patch as-is? Yes.
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c index 594b8f6..baca978 100644 --- a/gdb/i386-fbsd-tdep.c +++ b/gdb/i386-fbsd-tdep.c @@ -248,14 +248,14 @@ i386fbsd_core_read_xcr0 (bfd *abfd) { warning (_("Couldn't read `xcr0' bytes from " "`.reg-xstate' section in core file.")); - return 0; + return X86_XSTATE_SSE_MASK; } xcr0 = bfd_get_64 (abfd, contents); } } else - xcr0 = 0; + xcr0 = X86_XSTATE_SSE_MASK; return xcr0; }