From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id D2CBB3858C41 for ; Wed, 8 Nov 2023 19:34:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D2CBB3858C41 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D2CBB3858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699472098; cv=none; b=c/8xy/9i3GoGDlJtXFcNlSAhlIUFPugTJq6Typ2/PtFjJ+C8V9RBee/ZmEWiTKnMnbuMsqDRlLZj2vKc7qFIIyQNMuKCR1XZipgAXvC7qj6pbom/BvP4NZmsg5QITytTsVcBmutyrYwlaN8noYG9UdAzq0mEgkuoiy3bntS7h74= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699472098; c=relaxed/simple; bh=dEqzXugB3+dh0XzSUY95T2PVkkPmtfwWqh/gIHiiyOY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=L6PRTKCzw8R16eQZ8/ZfbghSguhYim8QUwX5YjICRjw9oySKPO8ELKSyIjeTXZLtP1R89Gw7vqZUihvos+le6Qgp16owl3WQyayNe/VM4UjCB3qYahmfk1F+V5sDUyhDixxpNmw/LxYQGccY6ytF2gB2U2N8Mp70UdCnSrgnvn4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 3A8JYmL1002014 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 8 Nov 2023 14:34:53 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 3A8JYmL1002014 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1699472093; bh=OZwEd7Z/ozU3LaUz3XI0tlitYcMAq+Q/a0wutgNwZY4=; h=Date:Subject:To:References:From:In-Reply-To:From; b=bLdoakSfIkJkl+4ajoxskRxrzXj3laeNae/d65CXnsgtGNbHKLPWbzEkEz+iNGKQ2 6win/hsVlErTy+lomS4kCrQ+4l5C7+fMtZ5a9sV54etJnU8M9S/hknXaw6zra7KTn2 7ZaGb7aiiGfcYHRMz/gKDKkxTsYm6CGiRY9icqvU= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id E5D581E00F; Wed, 8 Nov 2023 14:34:47 -0500 (EST) Message-ID: <25f0e95c-32bb-41d1-acd0-69273ab7c88b@polymtl.ca> Date: Wed, 8 Nov 2023 14:34:47 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Content-Language: fr To: Luis Machado , gdb-patches@sourceware.org References: <20231108051222.1275306-1-simon.marchi@polymtl.ca> <9a693dcc-9213-4300-a4ee-25820bfada78@arm.com> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 8 Nov 2023 19:34:48 +0000 X-Spam-Status: No, score=-3037.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 11/8/23 12:08, Luis Machado wrote: > On 11/8/23 15:47, Simon Marchi wrote: >>> I haven't tracked the particular patch that causes this, but for aarch64 systems supporting SVE >>> (and probably SME as well) I'm seeing internal errors related to one assertion: >> Thanks a lot for testing. >> >>> Running gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp ... >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: continue to breakpoint: continue to callee >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: callee, before change: p/x $v8.q.u >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: callee, before change: p/x $s8.u >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: set $s8.u = 0x34353637 (GDB internal error) >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change: p/x $v8.q.u >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change: p/x $s8.u >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: up >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: caller, before change: p/x $v8.q.u >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: caller, before change: p/x $s8.u >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: set $s8.u = 0x44454647 (GDB internal error) >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $v8.q.u >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $s8.u >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: down >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change in caller: p/x $v8.q.u >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change in caller: p/x $s8.u >>> PASS: gdb.arch/aarch64-pseudo-unwind.exp: continue to breakpoint: continue to break_here_c >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value >>> Running gdb/testsuite/gdb.arch/aarch64-fp.exp ... >>> PASS: gdb.arch/aarch64-fp.exp: set the breakpoint after setting the fp registers >>> PASS: gdb.arch/aarch64-fp.exp: continue until breakpoint >>> PASS: gdb.arch/aarch64-fp.exp: check register q0 value >>> PASS: gdb.arch/aarch64-fp.exp: check register q1 value >>> PASS: gdb.arch/aarch64-fp.exp: check register v0 value >>> PASS: gdb.arch/aarch64-fp.exp: check register v1 value >>> PASS: gdb.arch/aarch64-fp.exp: check register fpsr value >>> PASS: gdb.arch/aarch64-fp.exp: check register fpcr value >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: set h0.bf to 129 (GDB internal error) >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: h0 fields are valid >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: set v0.h.bf[0] to 0 (GDB internal error) >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: v0.h.s[0] is 0 >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: set v0.h.bf[0] to 129 (GDB internal error) >>> FAIL: gdb.arch/aarch64-fp.exp: bfloat16: v0.h.s[0] is 129 >>> >>> The failure mode is: >>> >>> set $s8.u = 0x34353637^M >>> ../../../repos/binutils-gdb/gdb/frame.c:1441: internal-error: put_frame_register: Assertion `buf.size () == size' failed.^M >>> A problem internal to GDB has been detected,^M >>> further debugging may prove unreliable.^M >>> ----- Backtrace -----^M >>> FAIL: gdb.arch/aarch64-pseudo-unwind.exp: set $s8.u = 0x34353637 (GDB internal error) >>> Resyncing due to internal error. >>> 0xaaaaabd6e08f gdb_internal_backtrace_1^M >>> ../../../repos/binutils-gdb/gdb/bt-utils.c:122^M >>> 0xaaaaabd6e08f _Z22gdb_internal_backtracev^M >>> ../../../repos/binutils-gdb/gdb/bt-utils.c:168^M >>> 0xaaaaac315307 internal_vproblem^M >>> ../../../repos/binutils-gdb/gdb/utils.c:396^M >>> 0xaaaaac31556f _Z15internal_verrorPKciS0_St9__va_list^M >>> ../../../repos/binutils-gdb/gdb/utils.c:476^M >>> 0xaaaaac8604e3 _Z18internal_error_locPKciS0_z^M >>> ../../../repos/binutils-gdb/gdbsupport/errors.cc:58^M >>> 0xaaaaabec89c7 _Z18put_frame_register14frame_info_ptriN3gdb10array_viewIKhEE^M >>> ../../../repos/binutils-gdb/gdb/frame.c:1441^M >>> 0xaaaaabc55243 aarch64_pseudo_write_1^M >>> ../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3316^M >>> 0xaaaaabc5c943 aarch64_pseudo_write^M >>> ../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3414^M >> >> Ok, then I presume the regression is introduced with patch "gdb: migrate >> aarch64 to new gdbarch_pseudo_register_write". >> >>> It is also worth noting that the v registers are *not* pseudo-registers when the aarch64-fpu feature is used. If SVE >>> is available though, then that means the v registers are pseudo-registers as well. You can tell them apart by their types. >>> >>> Real register (no SVE support): >>> >>> type = union aarch64v { >>> vnd d; >>> vns s; >>> vnh h; >>> vnb b; >>> vnq q; >>> } >>> >>> Pseudo-register (SVE support): >>> >>> union __gdb_builtin_type_vnv { >>> __gdb_builtin_type_vnd d; >>> __gdb_builtin_type_vns s; >>> __gdb_builtin_type_vnh h; >>> __gdb_builtin_type_vnb b; >>> __gdb_builtin_type_vnq q; >>> } >> >> Ok, thanks for that explanation. If you recall, I asked you on IRC if >> the V registers were always 16 bytes long, and you said yes. That's why >> I hardcoded the size of 16 in aarch64_pseudo_write_1. But then > > Right. The V registers are always 16 bytes long, being real or pseudo-registers. So > their sizes are fixed. > > The other pseudo-registers are also fixed size. What chages is the size of the SVE > register the V/Q/D/S/H/B pseudo-registers map to. > >> according to your explanation, when SVE is available, the raw register >> behind S registers, for instance, is an SVE register whose length is >> unknown at compile time. That's why the existing code calls >> register_size to get the size of the raw register. >> >> If you apply the patch below, does it help? > > It gets rid of the internal error due to the assertion, but I still see FAIL's due to wrong > values being printed. > > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change: p/x $v8.q.u > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $v8.q.u > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $s8.u > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change in caller: p/x $v8.q.u > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: continue to breakpoint: continue to break_here_c > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value > FAIL: gdb.arch/aarch64-fp.exp: bfloat16: h0 fields are valid > > p/x $v8.q.u > $3 = {0xaaab34353637} > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change: p/x $v8.q.u > -- > p/x $v8.q.u > $7 = {0xaaab34353637} > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $v8.q.u > -- > p/x $s8.u > $8 = 0x34353637 > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $s8.u > -- > p/x $v8.q.u > $9 = {0xaaab34353637} > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: callee, after change in caller: p/x $v8.q.u > -- > Program received signal SIGSEGV, Segmentation fault. > 0x0000aaab0fad6e60 in ?? () > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: continue to breakpoint: continue to break_here_c > p/x value > No symbol "value" in current context. > (gdb) FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value > testcase binutils-gdb/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp completed in 1 seconds > Running binutils-gdb/gdb/testsuite/gdb.arch/aarch64-fp.exp ... > -- > p $h0 > $7 = {bf = 1.136e-28, f = 0.00061798, u = 4368, s = 4368} > (gdb) FAIL: gdb.arch/aarch64-fp.exp: bfloat16: h0 fields are valid Ah, damn, probably because I switched to byte_vector, which doesn't do the zero-initialization we want to do. Here's a new patch (that applies on the series directly) that doesn't use byte_vector. diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 1815d78dec4..200e740e013 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -3300,7 +3300,7 @@ aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame, int regnum_offset, gdb::array_view buf) { - unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset; + unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset; gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); /* Enough space for a full vector register. @@ -3309,11 +3309,11 @@ aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame, various 'scalar' pseudo registers to behavior like architectural writes, register width bytes are written the remainder are set to zero. */ - constexpr int raw_reg_size = 16; + int raw_reg_size = register_size (gdbarch, raw_regnum); gdb_byte raw_buf[raw_reg_size] {}; - gdb::array_view raw_view (raw_buf); + gdb::array_view raw_view (raw_buf, raw_reg_size); copy (buf, raw_view.slice (0, buf.size ())); - put_frame_register (next_frame, v_regnum, raw_view); + put_frame_register (next_frame, raw_regnum, raw_view); } /* Given REGNUM, a SME pseudo-register number, store the bytes from DATA to the Simon