From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 635103947429 for ; Tue, 22 Mar 2022 14:19:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 635103947429 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [172.16.0.95] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 272261EDF0; Tue, 22 Mar 2022 10:19:36 -0400 (EDT) Message-ID: Date: Tue, 22 Mar 2022 10:19:36 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] gdb:csky using csky_gdbarch_tdep in csky_gdbarch_init Content-Language: tl To: Jiangshuai Li , gdb-patches@sourceware.org References: <20220322115203.2201-1-jiangshuai_li@c-sky.com> From: Simon Marchi In-Reply-To: <20220322115203.2201-1-jiangshuai_li@c-sky.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3645.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Mar 2022 14:19:39 -0000 On 2022-03-22 07:52, Jiangshuai Li wrote: > add three points: > 1. add vars to csky_gdbarch_tdep > 2. save mach, fpu_abi, vdsp_verion and fpu_hardfp from info.abfd > 3. find a pre-declared gdbarch via point 2 > > fpu_abi and vdsp_verion will be used by gdbarch_return_value > and gdbarch_push_dummy_call later. > > gdb/ > *csky-tdep.h: add vars to csky_gdbarch_tdep > *csky-tdep.c: save msgs from info.abfd and use them > to find a pre-declared gdbarch Don't need a ChangeLog entry for GDB. The subject of the patch: csky using csky_gdbarch_tdep in csky_gdbarch_init is not clear. What behavior change do you want to introduce here? > --- > gdb/csky-tdep.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-- > gdb/csky-tdep.h | 17 +++++++++- > 2 files changed, 100 insertions(+), 4 deletions(-) > > diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c > index cba0065fa53..44c2b56fba4 100644 > --- a/gdb/csky-tdep.c > +++ b/gdb/csky-tdep.c > @@ -2157,16 +2157,97 @@ static struct gdbarch * > csky_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > { > struct gdbarch *gdbarch; > + /* Analysis info.abfd. */ Analysis -> Analyze ? > + unsigned int mach; > + int mach_find = 0; > + unsigned long abfd_e_flags; > + unsigned int fpu_abi = 0; > + unsigned int vdsp_version = 0; > + unsigned int fpu_hardfp = 0; > + static bfd_arch_info_type bfd_csky_arch_t; What's the purpose of this variable being static? > + > + /* When the type of bfd file is srec(or any files are not elf), > + the E_FLAGS will be not credible. */ > + if (info.abfd && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour) info.abfd != nullptr > + { > + /* Get e_flags form abfd. */ > + abfd_e_flags = elf_elfheader (info.abfd)->e_flags; > + if (IS_CSKY_V2 (abfd_e_flags)) > + { > + switch (abfd_e_flags & CSKY_ARCH_MASK) Please correct the indentation to fit the coding style (tabs + spaces). > + { > + case CSKY_ARCH_801: > + case CSKY_ARCH_802: > + case CSKY_ARCH_803: > + case CSKY_ARCH_807: > + case CSKY_ARCH_810: > + case CSKY_ARCH_860: > + mach = abfd_e_flags & CSKY_ARCH_MASK; > + mach_find = 1; It's not clear why you need to do this here. Selecting the right machine type based on flags is normally the kind of things done in BFD. So by the time it reaches GDB, the correct bfd_arch_info should have already been selected by BFD. For example, if the ELF file is a CSKY_ARCH_803, then info.abfd->arch_info should already point to bfd_mach_ck803. > + break; > + default: > + mach = CSKY_ARCH_810; > + break; > + } > + } > + else > + { > + mach = CSKY_ARCH_810; > + } > + > + /* Get FPU, VDSP build options. */ > + fpu_abi = bfd_elf_get_obj_attr_int (info.abfd, > + OBJ_ATTR_PROC, > + Tag_CSKY_FPU_ABI); > + vdsp_version = bfd_elf_get_obj_attr_int (info.abfd, > + OBJ_ATTR_PROC, > + Tag_CSKY_VDSP_VERSION); > + fpu_hardfp = bfd_elf_get_obj_attr_int (info.abfd, > + OBJ_ATTR_PROC, > + Tag_CSKY_FPU_HARDFP); > + } > + else > + { > + mach = CSKY_ARCH_810; > + } > + > + /* Set info.bfd_arch_info. */ > + memcpy (&bfd_csky_arch_t, info.bfd_arch_info, sizeof (bfd_csky_arch_t)); > + if (mach_find) > + bfd_csky_arch_t.mach = abfd_e_flags; > + else > + bfd_csky_arch_t.mach = mach | CSKY_ABI_V2; > + info.bfd_arch_info = &bfd_csky_arch_t; > > /* Find a candidate among the list of pre-declared architectures. */ > - arches = gdbarch_list_lookup_by_info (arches, &info); > - if (arches != NULL) > - return arches->gdbarch; > + for (arches = gdbarch_list_lookup_by_info (arches, &info); > + arches != NULL; > + arches = gdbarch_list_lookup_by_info (arches->next, &info)) > + { > + csky_gdbarch_tdep *tdep > + = (csky_gdbarch_tdep *) gdbarch_tdep (arches->gdbarch); > + if (mach != tdep->mach) > + continue; As explained above, this filtering on mach should not be necessary: two ELF files with different machine values (in their e_flags) should lead to two abfds with different arch infos. And gdbarch_list_lookup_by_info will automatically filters out those with different arch infos. It's not clear what problem you are trying to solve, maybe if try to explain that first it will be easier to understand what you are trying to achieve. Simon