From patchwork Mon Mar 25 16:51:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 31975 Received: (qmail 49449 invoked by alias); 25 Mar 2019 16:51:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 48434 invoked by uid 89); 25 Mar 2019 16:51:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=if X-HELO: EUR02-AM5-obe.outbound.protection.outlook.com Received: from mail-eopbgr00047.outbound.protection.outlook.com (HELO EUR02-AM5-obe.outbound.protection.outlook.com) (40.107.0.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 25 Mar 2019 16:51:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hRUBLAMarrN12AyZpb5rorFSlXTfsrG8joW2lthxTwI=; b=IbNg843JrvhWk/cGGroLctlq389yAVhrbxgo2K57EnbnYxlZQ1UkLSgUVK1OJIoNkzYgfG/L0DFEPzJPLC+tLlAg62uWli+Z/tW0NL/AdEXO8ekDQmutK8V4b9mC5N2I9xq/M69MH4BPMBGnFcHclTLJhgGomG4z57kZCSTt4bY= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.227.22) by DB6PR0802MB2440.eurprd08.prod.outlook.com (10.172.251.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.18; Mon, 25 Mar 2019 16:51:04 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::2083:2d62:84fa:a547]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::2083:2d62:84fa:a547%3]) with mapi id 15.20.1730.019; Mon, 25 Mar 2019 16:51:04 +0000 From: Alan Hayward To: Simon Marchi CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH 1/2] Add linux_get_hwcap Date: Mon, 25 Mar 2019 16:51:04 +0000 Message-ID: <52EFD498-1551-41A6-BB95-81A7BB5A3928@arm.com> References: <20190325120542.92123-1-alan.hayward@arm.com> <0cea3f8f-e66e-574c-0c6e-aa2c2bbead4f@simark.ca> In-Reply-To: <0cea3f8f-e66e-574c-0c6e-aa2c2bbead4f@simark.ca> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 Content-ID: <1792682C1C377E40870603D3B9118A8F@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-IsSubscribed: yes > On 25 Mar 2019, at 15:18, Simon Marchi wrote: > > On 2019-03-25 8:05 a.m., Alan Hayward wrote: >> Tidy up calls to read HWCAP (and HWCAP2) by adding common functions, >> removing the PPC and AArch64 specific versions. >> The only function difference is in aarch64_linux_core_read_description - if >> the hwcap read fails it now return a valid description instead of nullptr. > > ARM also seems to have changed behavior, both the native target and core target. It's not necessarily, a problem, as long as it's a conscious change. I should have mentioned it really. In the original it returned immediately on error. In the new version it continues, but all the “if”s will fail and it’ll return at the end of the function with the same result as the original early return. > >> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h >> index 824ba3afaf..ee9c2bcc90 100644 >> --- a/gdb/linux-tdep.h >> +++ b/gdb/linux-tdep.h >> @@ -61,4 +61,10 @@ extern void linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch); >> extern int linux_is_uclinux (void); >> +/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. */ >> +extern CORE_ADDR linux_get_hwcap (struct target_ops *target); >> + >> +/* Fetch the AT_HWCAP2 entry from the auxv vector for the given TARGET. */ >> +extern CORE_ADDR linux_get_hwcap2 (struct target_ops *target); > > For these two functions, can you mention that 0 is returned if the search in the AUXV vector fails? > Done. > I was a bit surprised you didn't keep your version returning a bool to indicate whether the search succeeded or failed, but if this version is good enough, I am fine with it. If we ever have a target that really needs to differentiate between a lookup failure and a lookup success that returns the value 0, we can change it back or add another overload. > Once I had changed the AArch64 code, there was nothing left that was checking the error. So it seemed better to remove it. > This patch LGTM with the comment above updated. > > Simon Thanks. Pushed code included below. Alan. diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index 86c7e87dd5..6d43eb7070 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -42,6 +42,7 @@ #include #include "gregset.h" +#include "linux-tdep.h" /* Defines ps_err_e, struct ps_prochandle. */ #include "gdb_proc_service.h" @@ -641,11 +642,10 @@ aarch64_linux_nat_target::read_description () if (ret == 0) return tdesc_arm_with_neon; - CORE_ADDR hwcap = 0; - bool pauth_p = aarch64_linux_get_hwcap (this, &hwcap) - && (hwcap & AARCH64_HWCAP_PACA); + CORE_ADDR hwcap = linux_get_hwcap (this); - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p); + return aarch64_read_description (aarch64_sve_get_vq (tid), + hwcap & AARCH64_HWCAP_PACA); } /* Convert a native/host siginfo object, into/from the siginfo in the diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index bcd1961744..7f2193f2fa 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -655,13 +655,10 @@ static const struct target_desc * aarch64_linux_core_read_description (struct gdbarch *gdbarch, struct target_ops *target, bfd *abfd) { - CORE_ADDR aarch64_hwcap = 0; - - if (!aarch64_linux_get_hwcap (target, &aarch64_hwcap)) - return nullptr; + CORE_ADDR hwcap = linux_get_hwcap (target); return aarch64_read_description (aarch64_linux_core_read_vq (gdbarch, abfd), - aarch64_hwcap & AARCH64_HWCAP_PACA); + hwcap & AARCH64_HWCAP_PACA); } /* Implementation of `gdbarch_stap_is_single_operand', as defined in @@ -1439,15 +1436,6 @@ aarch64_linux_gcc_target_options (struct gdbarch *gdbarch) return NULL; } -/* See aarch64-linux-tdep.h. */ - -bool -aarch64_linux_get_hwcap (struct target_ops *target, CORE_ADDR *hwcap) -{ - *hwcap = 0; - return target_auxv_search (target, AT_HWCAP, hwcap) == 1; -} - static void aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) { diff --git a/gdb/aarch64-linux-tdep.h b/gdb/aarch64-linux-tdep.h index ec494bfc53..f075f0acd9 100644 --- a/gdb/aarch64-linux-tdep.h +++ b/gdb/aarch64-linux-tdep.h @@ -42,7 +42,4 @@ extern const struct regset aarch64_linux_fpregset; /* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h. */ #define AARCH64_HWCAP_PACA (1 << 30) -/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. */ -bool aarch64_linux_get_hwcap (struct target_ops *target, CORE_ADDR *hwcap); - #endif /* AARCH64_LINUX_TDEP_H */ diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c index 11e353e61c..b54bd5afe9 100644 --- a/gdb/arm-linux-nat.c +++ b/gdb/arm-linux-nat.c @@ -533,7 +533,7 @@ ps_get_thread_area (struct ps_prochandle *ph, const struct target_desc * arm_linux_nat_target::read_description () { - CORE_ADDR arm_hwcap = 0; + CORE_ADDR arm_hwcap = linux_get_hwcap (this); if (have_ptrace_getregset == TRIBOOL_UNKNOWN) { @@ -551,11 +551,6 @@ arm_linux_nat_target::read_description () have_ptrace_getregset = TRIBOOL_TRUE; } - if (target_auxv_search (this, AT_HWCAP, &arm_hwcap) != 1) - { - return this->beneath ()->read_description (); - } - if (arm_hwcap & HWCAP_IWMMXT) return tdesc_arm_with_iwmmxt; diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c index 11db9bae32..a5ad06434c 100644 --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -730,10 +730,7 @@ arm_linux_core_read_description (struct gdbarch *gdbarch, struct target_ops *target, bfd *abfd) { - CORE_ADDR arm_hwcap = 0; - - if (target_auxv_search (target, AT_HWCAP, &arm_hwcap) != 1) - return NULL; + CORE_ADDR arm_hwcap = linux_get_hwcap (target); if (arm_hwcap & HWCAP_VFP) { diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 77bc714286..f6c4f7b208 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -2462,6 +2462,28 @@ linux_displaced_step_location (struct gdbarch *gdbarch) return addr; } +/* See linux-tdep.h. */ + +CORE_ADDR +linux_get_hwcap (struct target_ops *target) +{ + CORE_ADDR field; + if (target_auxv_search (target, AT_HWCAP, &field) != 1) + return 0; + return field; +} + +/* See linux-tdep.h. */ + +CORE_ADDR +linux_get_hwcap2 (struct target_ops *target) +{ + CORE_ADDR field; + if (target_auxv_search (target, AT_HWCAP2, &field) != 1) + return 0; + return field; +} + /* Display whether the gcore command is using the /proc/PID/coredump_filter file. */ diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h index 824ba3afaf..913045a954 100644 --- a/gdb/linux-tdep.h +++ b/gdb/linux-tdep.h @@ -61,4 +61,12 @@ extern void linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch); extern int linux_is_uclinux (void); +/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. On + error, 0 is returned. */ +extern CORE_ADDR linux_get_hwcap (struct target_ops *target); + +/* Fetch the AT_HWCAP2 entry from the auxv vector for the given TARGET. On + error, 0 is returned. */ +extern CORE_ADDR linux_get_hwcap2 (struct target_ops *target); + #endif /* linux-tdep.h */ diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c index 11394ef36d..3a6bbf4163 100644 --- a/gdb/ppc-linux-nat.c +++ b/gdb/ppc-linux-nat.c @@ -1554,31 +1554,6 @@ store_ppc_registers (const struct regcache *regcache, int tid) function to fail most of the time, so we ignore them. */ } -/* Fetch the AT_HWCAP entry from the aux vector. */ -static CORE_ADDR -ppc_linux_get_hwcap (void) -{ - CORE_ADDR field; - - if (target_auxv_search (current_top_target (), AT_HWCAP, &field) != 1) - return 0; - - return field; -} - -/* Fetch the AT_HWCAP2 entry from the aux vector. */ - -static CORE_ADDR -ppc_linux_get_hwcap2 (void) -{ - CORE_ADDR field; - - if (target_auxv_search (current_top_target (), AT_HWCAP2, &field) != 1) - return 0; - - return field; -} - /* The cached DABR value, to install in new threads. This variable is used when the PowerPC HWDEBUG ptrace interface is not available. */ @@ -1735,7 +1710,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) takes two hardware watchpoints though. */ if (len > 1 && hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE - && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) + && linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE) return 2; /* Check if the processor provides DAWR interface. */ if (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR) @@ -1755,7 +1730,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) ptrace interface, DAC-based processors (i.e., embedded processors) will use addresses aligned to 4-bytes due to the way the read/write flags are passed in the old ptrace interface. */ - else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) + else if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE) && (addr + len) > (addr & ~3) + 4) || (addr + len) > (addr & ~7) + 8) return 0; @@ -2294,7 +2269,7 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len, long dabr_value; long read_mode, write_mode; - if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) + if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE) { /* PowerPC 440 requires only the read/write flags to be passed to the kernel. */ @@ -2497,9 +2472,9 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr, int mask; if (have_ptrace_hwdebug_interface () - && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) + && linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE) return start <= addr && start + length >= addr; - else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) + else if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE) mask = 3; else mask = 7; @@ -2637,8 +2612,8 @@ ppc_linux_nat_target::read_description () features.wordsize = ppc_linux_target_wordsize (tid); - CORE_ADDR hwcap = ppc_linux_get_hwcap (); - CORE_ADDR hwcap2 = ppc_linux_get_hwcap2 (); + CORE_ADDR hwcap = linux_get_hwcap (current_top_target ()); + CORE_ADDR hwcap2 = linux_get_hwcap2 (current_top_target ()); if (have_ptrace_getsetvsxregs && (hwcap & PPC_FEATURE_HAS_VSX)) diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c index df44ad826e..19435602b5 100644 --- a/gdb/ppc-linux-tdep.c +++ b/gdb/ppc-linux-tdep.c @@ -1601,10 +1601,7 @@ ppc_linux_core_read_description (struct gdbarch *gdbarch, if (vsx) features.vsx = true; - CORE_ADDR hwcap; - - if (target_auxv_search (target, AT_HWCAP, &hwcap) != 1) - hwcap = 0; + CORE_ADDR hwcap = linux_get_hwcap (target); features.isa205 = ppc_linux_has_isa205 (hwcap); diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c index 5f6c5be8e0..3a3afae7a6 100644 --- a/gdb/s390-linux-nat.c +++ b/gdb/s390-linux-nat.c @@ -1016,9 +1016,8 @@ s390_linux_nat_target::read_description () that mode, report s390 architecture with 64-bit GPRs. */ #ifdef __s390x__ { - CORE_ADDR hwcap = 0; + CORE_ADDR hwcap = linux_get_hwcap (current_top_target ()); - target_auxv_search (current_top_target (), AT_HWCAP, &hwcap); have_regset_tdb = (hwcap & HWCAP_S390_TE) && check_regset (tid, NT_S390_TDB, s390_sizeof_tdbregset); diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c index 64fe7abd80..02ae28b4ea 100644 --- a/gdb/s390-linux-tdep.c +++ b/gdb/s390-linux-tdep.c @@ -332,10 +332,9 @@ s390_core_read_description (struct gdbarch *gdbarch, struct target_ops *target, bfd *abfd) { asection *section = bfd_get_section_by_name (abfd, ".reg"); - CORE_ADDR hwcap = 0; + CORE_ADDR hwcap = linux_get_hwcap (target); bool high_gprs, v1, v2, te, vx, gs; - target_auxv_search (target, AT_HWCAP, &hwcap); if (!section) return NULL;