From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2610:1c1:1:606c::19:2]) by sourceware.org (Postfix) with ESMTPS id A1CD13858D20 for ; Tue, 9 Apr 2024 18:00:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A1CD13858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A1CD13858D20 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=2610:1c1:1:606c::19:2 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712685604; cv=pass; b=n4gZ8/DSjZMU/ES/UVtkmQGKkWJD6nBgW/dWhu2EYm+p2oHVhzAA50w5Qh27orPvQFmC7lFZU8LeWXKgMe1d7qjLBQ8F/Sm2f8BIZ87ay3Yup6DaQhoHt/90VA7LHSHOzR8DN2KFs3hstElx0y42/JdFdhh6PUBoKDQCnE9/y7I= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712685604; c=relaxed/simple; bh=PAbMDwKo0eLFm2lySTDOcU0eToX5LD7zTiNoq6rTqdY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=oHe+mFsJMvHtl/IVIgyXr8DpGACp7SVTwMho8Wk/wxMNVG4lEn0FkXLZwG0RvTQ8H4Rm2qn4ZUN7d5Atcm148KdGvH5KNsVCx7zD1Zct6yE0slGURl7xgYJKU/2jk3f3jV+wdckyG+HmR1jps2UMWMgTxa7cx1iZvo9kxFHJX80= ARC-Authentication-Results: i=2; server2.sourceware.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 4VDYdX0ldLz4Tbx; Tue, 9 Apr 2024 18:00:00 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VDYdW6ZGmz4lfm; Tue, 9 Apr 2024 17:59:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1712685599; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MYV+jvVwb4jj1mRPaC9edIJjnRSVmvqT5Hzhn8E5xt4=; b=PLtW7LX7gDbORGpw9zUZarSqDY6LN347rbnzyBUJeUMamL2ABsOx0UVKBOd7YwM4sFr93e r7FWYOzc8vhZoLmLLfUGdtvz9ZHahw+Lh8p1yQ6bPRvIxwjmjWWTwc4vBPov3kssydLWrS JwnCDC4OYw3rLUby8KZvPZrAZCYPB7fxbFww6l22/lN1B6ddDZqFZeMgSd3EgntyyMZNGo CfX/kJYLhICSL4xQzj81tpJZF8ZWa02puA/2cVrT+O3QGwUcEnFKw96BCXDIIdxB8mrUJx vLb4RSBbitdFl71taSGjVu56rksda+5bv+JM1I2E02YR3oxSTF/PzB5ykhEztQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1712685599; a=rsa-sha256; cv=none; b=OkyX9mZF5AeocGkYVsCIyal6TbmGXqFWWLMSAReqEEkXTMu5dRNKwBhGoa3DOlxs4rHbFw F3zm3lTkSLNQ+4ZLbcPE1xhGR6KEYNOag6QcfbXtqiRX5HA9sGngNbmbboPAqVbM0e5IJa /8gl2tM/VNn+KMSjSMKx8+yzMbSICTSN5WuNs5DKZgzmtPMVbZvopE6hcZ9zM/sYJ6msiS rrBO96A0wr1c//FbXb//bfaAdqLlBis9Lh6bw9lRNoJ/snNhUA3jdnZ5nffEDObtTJ5eJj Qw+H1UjZ38Tl+F1ZOCKVOFluAYswAPw21+2mSt/gwp1rmEPda+PiqkVibpPPtQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1712685599; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MYV+jvVwb4jj1mRPaC9edIJjnRSVmvqT5Hzhn8E5xt4=; b=Zcto/fkZYteEYtT7gDMBOtCFx+DCq/KPrtge/5ccBWt7AjuNC9dw4fyzfn3Fb7y/kBvA+Z jP/BR2OZEMb4zfJ5oALl/sLe/DViJzbFwWJovSYLzgGSO888SHdkKbLfkCI8TjnROMKtIl 6UVnNeTzFRbLDOptcOd6UEMoc2qbKAEfApeO0dMKdn1f7WHUKY8XMgVQ8+RWRZRWsNgh79 9gnRtTx4aKoRmm1/9k5dIQJ5PbjsZ/hGUcy0OXBtN4OOpuwJnf2kSHhMyZQO826EOrX4Z2 GdSGkQqyxfKlfpZE6GPMqLwnO6V6pUdGHRrloxnhvM6KqRI32LDZAgfpV6i36Q== Received: from [IPV6:2601:644:937f:4c50:7d01:266e:5339:1e93] (unknown [IPv6:2601:644:937f:4c50:7d01:266e:5339:1e93]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4VDYdW1F8vz1DT8; Tue, 9 Apr 2024 17:59:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <41c55e21-9ad9-432f-a650-5f09274b48a4@FreeBSD.org> Date: Tue, 9 Apr 2024 10:59:57 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] gdb: xtensa: fix registers supply if they not present Content-Language: en-US To: Alexey Lapshin , "gdb-patches@sourceware.org" , "tom@tromey.com" Cc: Alexey Gerenkov , Anton Maklakov , Ivan Grokhotkov , "jcmvbkbc@gmail.com" References: <026589bb5fd46e8811b0909d048307261a8165b7.camel@espressif.com> <87356uhm8v.fsf@tromey.com> <9f7a82d385ccbc4c249ff7ecebff24fa42ebb149.camel@espressif.com> From: John Baldwin In-Reply-To: <9f7a82d385ccbc4c249ff7ecebff24fa42ebb149.camel@espressif.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 5/16/23 12:17 AM, Alexey Lapshin via Gdb-patches wrote: > When parsing a core file on hardware configurations without the > zero-overhead loop option (e.g. ESP32-S2 chip), GDB used to assert > while trying to call 'raw_supply' for lbeg, lend, lcount registers, > even though they were not set. > > This was because: > regnum == -1 was used to indicate "supply all registers" > lbeg_regnum == -1 was used to indicate "lbeg register not present" > regnum == lbeg_regnum check was considered successful > --- > gdb/xtensa-tdep.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c > index a47a2987499..af0f5c86bab 100644 > --- a/gdb/xtensa-tdep.c > +++ b/gdb/xtensa-tdep.c > @@ -803,6 +803,18 @@ xtensa_register_reggroup_p (struct gdbarch *gdbarch, > } > > > +/* Check if register raw supplied > + Note: > + - check_regnum == -1 means register is not present. > + - regnum == -1 means all present registers are supplied. */ > + > +static inline bool > +is_reg_raw_supplied (int check_regnum, int regnum) > +{ > + return check_regnum > 0 && (regnum == check_regnum || regnum == -1); > +} > + > + > /* Supply register REGNUM from the buffer specified by GREGS and LEN > in the general-purpose register set REGSET to register cache > REGCACHE. If REGNUM is -1 do this for all registers in REGSET. */ > @@ -825,22 +837,22 @@ xtensa_supply_gregset (const struct regset *regset, > rc->raw_supply (gdbarch_pc_regnum (gdbarch), (char *) ®s->pc); > if (regnum == gdbarch_ps_regnum (gdbarch) || regnum == -1) > rc->raw_supply (gdbarch_ps_regnum (gdbarch), (char *) ®s->ps); > - if (regnum == tdep->wb_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->wb_regnum, regnum)) > rc->raw_supply (tdep->wb_regnum, > (char *) ®s->windowbase); > - if (regnum == tdep->ws_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->ws_regnum, regnum)) > rc->raw_supply (tdep->ws_regnum, > (char *) ®s->windowstart); > - if (regnum == tdep->lbeg_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->lbeg_regnum, regnum)) > rc->raw_supply (tdep->lbeg_regnum, > (char *) ®s->lbeg); > - if (regnum == tdep->lend_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->lend_regnum, regnum)) > rc->raw_supply (tdep->lend_regnum, > (char *) ®s->lend); > - if (regnum == tdep->lcount_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->lcount_regnum, regnum)) > rc->raw_supply (tdep->lcount_regnum, > (char *) ®s->lcount); > - if (regnum == tdep->sar_regnum || regnum == -1) > + if (is_reg_raw_supplied (tdep->sar_regnum, regnum)) > rc->raw_supply (tdep->sar_regnum, > (char *) ®s->sar); > if (regnum >=tdep->ar_base This looks ok to me, but is a bit ununusal. For other GDB arches we tend to add helper methods to the tdep class to indicate if an optional feature is present and then use those helpers in functions like this. For example, aarch64 has an optional TLS register set with the following class members: /* Target-dependent structure in gdbarch. */ struct aarch64_gdbarch_tdep : gdbarch_tdep_base { ... /* TLS registers. This is -1 if the TLS registers are not available. */ int tls_regnum_base = 0; int tls_register_count = 0; bool has_tls() const { return tls_regnum_base != -1; } ... }; Code that supports the TLS register set uses the has_tls function to instead of comparing tls_regnum_base against -1 explicitly, e.g. in aarch64-fbsd-nat.c: void aarch64_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum) { ... gdbarch *gdbarch = regcache->arch (); aarch64_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); if (tdep->has_tls ()) fetch_regset (regcache, regnum, NT_ARM_TLS, &aarch64_fbsd_tls_regset, tdep->tls_regnum_base); } Looking at the xtensa-tdep bits more though, I don't think there is a clear analog to this for the way xtensa_rmap[] works (it is not clear to me how a xtensa gdbarch can ever have a subset of registers given that the same static xtensa_rmap[] is always passed to the tdep object constructed in xtensa_gdbarch_init and xtensa_derive_tdep is called immediately after the constructor, so it seems like lbeg_regnum should always be set?) -- John Baldwin