From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54795 invoked by alias); 25 Mar 2019 15:41:08 -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 54761 invoked by uid 89); 25 Mar 2019 15:41:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-Languages-Length:2773 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, 25 Mar 2019 15:41:06 +0000 Received: from [172.16.0.89] (192-222-157-41.qc.cable.ebox.net [192.222.157.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 810BC1E471; Mon, 25 Mar 2019 11:41:04 -0400 (EDT) Subject: Re: [PATCH 2/2] gdbserver: Add linux_get_hwcap To: Alan Hayward , "gdb-patches@sourceware.org" Cc: nd References: <20190325120542.92123-1-alan.hayward@arm.com> <20190325120542.92123-2-alan.hayward@arm.com> From: Simon Marchi Message-ID: Date: Mon, 25 Mar 2019 15:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <20190325120542.92123-2-alan.hayward@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2019-03/txt/msg00545.txt.bz2 On 2019-03-25 8:05 a.m., Alan Hayward wrote: > In gdbserver, Tidy up calls to read HWCAP (and HWCAP2) by adding common > functions, removing the Arm, AArch64, PPC and S390 specific versions. > > No functionality differences. > > [ I wasn't sure in gdbserver when to use CORE_ADDR and when to use int/long. > I'm assuming CORE_ADDR is fairly recent to gdbserver? ] I don't know if CORE_ADDR is a recent addition to gdbserver. But I suppose CORE_ADDR was chosen as the return type for functions reading arbitrary AUXV values, since some of them may be pointers. With CORE_ADDR, we know those values will fit in the data type. When we return the HWCAP value, we know it won't be a pointer though, so returning a CORE_ADDR is a bit confusing, IMO. Those functions returning the HWCAP value could return something else, an uint64_t maybe. But then I would change it in the gdb version as well to match. > /* Implementation of linux_target_ops method "arch_setup". */ > > static void > @@ -545,8 +521,8 @@ aarch64_arch_setup (void) > if (is_elf64) > { > uint64_t vq = aarch64_sve_get_vq (tid); > - unsigned long hwcap = 0; > - bool pauth_p = aarch64_get_hwcap (&hwcap) && (hwcap & AARCH64_HWCAP_PACA); > + unsigned long hwcap = linux_get_hwcap (8); > + bool pauth_p = hwcap & AARCH64_HWCAP_PACA; Just wondering, can the linux-aarch64-low.c code be used to debug a process > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 6f703f589f..481919c205 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -7423,6 +7423,64 @@ linux_get_pc_64bit (struct regcache *regcache) > return pc; > } > > +/* Extract the auxiliary vector entry with a_type matching MATCH, storing the > + value in VALP and returning true. If no entry was found, return false. */ > + > +static bool > +linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp) I think this function could return the result (CORE_ADDR) directly, returning 0 on failure. If 4 and 8 are the only supported wordsize values, I would suggest adding an assert to verify it. > +{ > + gdb_byte *data = (gdb_byte *) alloca (2 * wordsize); > + int offset = 0; > + > + while ((*the_target->read_auxv) (offset, data, 2 * wordsize) == 2 * wordsize) > + { > + if (wordsize == 4) > + { > + unsigned int *data_p = (unsigned int *)data; > + if (data_p[0] == match) > + { > + *valp = data_p[1]; > + return true; > + } > + } > + else > + { > + unsigned long *data_p = (unsigned long *)data; > + if (data_p[0] == match) > + { > + *valp = data_p[1]; > + return true; > + } > + } I am a bit worried about relying on the size of the "int" and "long" types in architecture-independent code. Could we use uint32_t and uint64_t instead? Simon