From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53692 invoked by alias); 19 Jun 2017 21:27:06 -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 53673 invoked by uid 89); 19 Jun 2017 21:27:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Jun 2017 21:26:55 +0000 Received: by simark.ca (Postfix, from userid 33) id 9EAEC1E560; Mon, 19 Jun 2017 17:26:58 -0400 (EDT) To: Yao Qi Subject: Re: [PATCH 04/25] Centralize i386 linux target descriptions X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 19 Jun 2017 21:27:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <1497256916-4958-5-git-send-email-yao.qi@linaro.org> References: <1497256916-4958-1-git-send-email-yao.qi@linaro.org> <1497256916-4958-5-git-send-email-yao.qi@linaro.org> Message-ID: <16d66959fc676c209337007bb10449bc@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00549.txt.bz2 On 2017-06-12 10:41, Yao Qi wrote: > This patch moves all the tdesc_i386*_linux target descriptions to a > function i386_linux_read_description, which returns the right target > description according to xcr0. This also remove the duplication in > getting target descriptions in corefile and native target. > > gdb: > > 2017-04-27 Yao Qi > > * i386-linux-tdep.c (i386_linux_read_description): New function. > (i386_linux_core_read_description): Call > i386_linux_read_description. > * i386-linux-tdep.h (i386_linux_read_description): Declare. > (tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations. > (tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise > (tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise. > (tdesc_i386_avx_mpx_avx512_pku_linux): Likewise. > * x86-linux-nat.c (x86_linux_read_description): Call > i386_linux_read_description. > --- > gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------ > gdb/i386-linux-tdep.h | 10 ++-------- > gdb/x86-linux-nat.c | 24 ++++++++---------------- > 3 files changed, 32 insertions(+), 36 deletions(-) > > diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c > index 1909d61..1bc1a6f 100644 > --- a/gdb/i386-linux-tdep.c > +++ b/gdb/i386-linux-tdep.c > @@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd) > return xcr0; > } > > -/* Get Linux/x86 target description from core dump. */ > - > -static const struct target_desc * > -i386_linux_core_read_description (struct gdbarch *gdbarch, > - struct target_ops *target, > - bfd *abfd) > +const struct target_desc * > +i386_linux_read_description (uint64_t xcr0) This would need a comment /* See i386-linux-tdep.h. */ > { > - /* Linux/i386. */ > - uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd); > - > switch ((xcr0 & X86_XSTATE_ALL_MASK)) > { > case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK: > @@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch > *gdbarch, > break; > } > > + return NULL; > +} I was going to say: I think it would make more sense if i386_linux_read_description always returned something non-NULL, but then I realized you need to handle the special case in i386_linux_core_read_description... > + > +/* Get Linux/x86 target description from core dump. */ > + > +static const struct target_desc * > +i386_linux_core_read_description (struct gdbarch *gdbarch, > + struct target_ops *target, > + bfd *abfd) > +{ > + /* Linux/i386. */ > + uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd); > + const struct target_desc * tdesc = i386_linux_read_description > (xcr0); Extra space after *. The logic seems OK to me. It's not related to your patch, but I find a bit confusing that i386-linux has SSE, and in i386-mmx-linux, the mmx is there to denote the lack of sse. I looks like Mark agrees with me :) https://sourceware.org/ml/gdb-patches/2010-04/msg00300.html Simon