| Message ID | 20231108051222.1275306-1-simon.marchi@polymtl.ca |
|---|---|
| Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 412093858428 for <patchwork@sourceware.org>; Wed, 8 Nov 2023 05:12:49 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 229573858D1E for <gdb-patches@sourceware.org>; Wed, 8 Nov 2023 05:12:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 229573858D1E 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 229573858D1E 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=1699420354; cv=none; b=KZkJTxz8OMiwx72+DuO29sZH83u5rOkV0grGpacdtPcC176emSwFIa4cKeQWkF3TUoCkJc9AOP34AjcFbWaIoeHi2un7Gcjvdfj8fe42xGPCDB3xGzL7EGhZhsltQk6gT02+ZHaqMRe1GyZJeUXrBLZMRmc/6rVKPcOL8G+BfdM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699420354; c=relaxed/simple; bh=EmTsSFBpHYB9QvToloK97pMTB1Iigwhw/eNys9TkNos=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=IF+jLdoNSmZ9RKTCM3azZ8vWTSVbcKREHQsRghHi+0AkE9d9fqkWi91QlH2nmjhvhL6ca1ivpsGLQtL1Rjn8M7oogzQYiAxlgs6dSRN87G9ZtLqCTd/1+b5i1DtxcoPFPd//dCkhs/mr0d3oLsJ/qP1WANNGJq+vewFKonir63Y= 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 3A85CQ8p015312 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 8 Nov 2023 00:12:31 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 3A85CQ8p015312 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1699420351; bh=bMh5ItM7Ke+Pt/9NjfiYZN+Zngv6MVNwMKvx/Fgxxdc=; h=From:To:Cc:Subject:Date:From; b=V3GklvEmegBx6coU77UNj7aEFDhrERUT4K0srFIW4KfVzbaJ+LqgAGZ5nBsiYPuRS ZfCBX2/pIWU0djKlxy3F+wI2TugHTcOkpmbBMmbT8Nype2XBjZ+4j/BAdzE46H27IE M+Cl+pwoffgT/NO3F5icM04GloQHbTixMvDdeJC4= Received: from simark.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7A3BC1E098; Wed, 8 Nov 2023 00:12:26 -0500 (EST) From: Simon Marchi <simon.marchi@polymtl.ca> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Date: Wed, 8 Nov 2023 00:00:44 -0500 Message-ID: <20231108051222.1275306-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.42.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 8 Nov 2023 05:12:26 +0000 X-Spam-Status: No, score=-3182.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org |
| Series |
Fix reading and writing pseudo registers in non-current frames
|
|
Message
Simon Marchi
Nov. 8, 2023, 5 a.m. UTC
This series fixes reading/writing pseudo registers from/to non-current
frames (that is, frames other than frame 0). Currently, we get this:
(gdb) frame 0
#0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:38
38 pop %rbx
(gdb) p/x $rbx
$1 = 0x2021222324252627
(gdb) p/x $ebx
$2 = 0x24252627
(gdb) frame 1
#1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:58
58 call callee
(gdb) p/x $rbx
$3 = 0x1011121314151617
(gdb) p/x $ebx
$4 = 0x24252627
This is a bit surprising, we would expect the last value to be
0x14151617, the bottom half of the rbx value from frame 1 (the currently
selected frame at that point). Instead, we got the bottom half of the
rbx value from frame 0. This is because pseudo registers are always
read/written from/to the current thread's regcache.
This series fixes this (as well as writing to pseudo registers) by
making it so pseudo registers are read/written using a frame.
Simon Marchi (24):
gdb: don't handle i386 k registers as pseudo registers
gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h
gdb: make store_integer take an array_view
gdb: simplify conditions in
regcache::{read,write,raw_collect,raw_supply}_part
gdb: change regcache interface to use array_view
gdb: fix bugs in {get,put}_frame_register_bytes
gdb: make put_frame_register take an array_view
gdb: change value_of_register and value_of_register_lazy to take the
next frame
gdb: remove frame_register
gdb: make put_frame_register take the next frame
gdb: make put_frame_register_bytes take the next frame
gdb: make get_frame_register_bytes take the next frame
gdb: add value::allocate_register
gdb: read pseudo register through frame
gdb: change parameter name in frame_unwind_register_unsigned
declaration
gdb: rename gdbarch_pseudo_register_write to
gdbarch_deprecated_pseudo_register_write
gdb: add gdbarch_pseudo_register_write that takes a frame
gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write
gdb: make aarch64_za_offsets_from_regnum return za_offsets
gdb: add missing raw register read in
aarch64_sme_pseudo_register_write
gdb: migrate aarch64 to new gdbarch_pseudo_register_write
gdb: migrate arm to gdbarch_pseudo_register_read_value
gdb: migrate arm to new gdbarch_pseudo_register_write
gdb/testsuite: add tests for unwinding of pseudo registers
gdb/aarch64-tdep.c | 297 +++++-----
gdb/alpha-tdep.c | 11 +-
gdb/amd64-tdep.c | 82 +--
gdb/arch/arm-get-next-pcs.c | 6 +-
gdb/arch/arm-get-next-pcs.h | 5 +-
gdb/arch/arm.c | 2 +-
gdb/arch/arm.h | 4 +-
gdb/arm-linux-tdep.c | 11 +-
gdb/arm-tdep.c | 145 +++--
gdb/avr-tdep.c | 3 +-
gdb/bfin-tdep.c | 3 +-
gdb/csky-tdep.c | 4 +-
gdb/defs.h | 39 +-
gdb/dwarf2/expr.c | 22 +-
gdb/dwarf2/frame.c | 5 +-
gdb/eval.c | 3 +-
gdb/findvar.c | 50 +-
gdb/frame-unwind.c | 3 +-
gdb/frame.c | 174 +++---
gdb/frame.h | 28 +-
gdb/frv-tdep.c | 3 +-
gdb/gdbarch-gen.h | 28 +-
gdb/gdbarch.c | 40 +-
gdb/gdbarch_components.py | 29 +-
gdb/guile/scm-frame.c | 3 +-
gdb/h8300-tdep.c | 3 +-
gdb/i386-tdep.c | 380 ++++--------
gdb/i386-tdep.h | 15 +-
gdb/i387-tdep.c | 16 +-
gdb/ia64-tdep.c | 18 +-
gdb/infcmd.c | 6 +-
gdb/loongarch-tdep.c | 3 +-
gdb/m32c-tdep.c | 3 +-
gdb/m68hc11-tdep.c | 3 +-
gdb/m68k-tdep.c | 17 +-
gdb/mep-tdep.c | 3 +-
gdb/mi/mi-main.c | 3 +-
gdb/mips-tdep.c | 29 +-
gdb/msp430-tdep.c | 3 +-
gdb/nat/aarch64-hw-point.c | 3 +-
gdb/nat/aarch64-scalable-linux-ptrace.c | 20 +-
gdb/nat/linux-btrace.c | 3 +-
gdb/nds32-tdep.c | 8 +-
gdb/python/py-frame.c | 3 +-
gdb/python/py-unwind.c | 4 +-
gdb/regcache.c | 548 +++++++++++-------
gdb/regcache.h | 113 +++-
gdb/riscv-tdep.c | 13 +-
gdb/rl78-tdep.c | 3 +-
gdb/rs6000-tdep.c | 21 +-
gdb/s12z-tdep.c | 2 +-
gdb/s390-tdep.c | 3 +-
gdb/sh-tdep.c | 9 +-
gdb/sparc-tdep.c | 3 +-
gdb/sparc64-tdep.c | 3 +-
gdb/std-regs.c | 11 +-
.../gdb.arch/aarch64-pseudo-unwind-asm.S | 82 +++
.../gdb.arch/aarch64-pseudo-unwind.c | 33 ++
.../gdb.arch/aarch64-pseudo-unwind.exp | 90 +++
.../gdb.arch/amd64-pseudo-unwind-asm.S | 66 +++
gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c | 33 ++
.../gdb.arch/amd64-pseudo-unwind.exp | 91 +++
.../gdb.arch/arm-pseudo-unwind-asm.S | 81 +++
.../gdb.arch/arm-pseudo-unwind-legacy-asm.S | 84 +++
.../gdb.arch/arm-pseudo-unwind-legacy.c | 33 ++
.../gdb.arch/arm-pseudo-unwind-legacy.exp | 86 +++
gdb/testsuite/gdb.arch/arm-pseudo-unwind.c | 33 ++
gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp | 88 +++
gdb/valops.c | 31 +-
gdb/value.c | 149 +++++
gdb/value.h | 64 +-
gdb/xtensa-tdep.c | 3 +-
gdbserver/linux-arm-low.cc | 4 +-
gdbserver/regcache.cc | 69 ++-
gdbserver/regcache.h | 6 +-
gdbsupport/common-regcache.cc | 2 +-
gdbsupport/common-regcache.h | 58 +-
gdbsupport/rsp-low.cc | 8 +
gdbsupport/rsp-low.h | 2 +
79 files changed, 2324 insertions(+), 1144 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S
create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c
create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp
create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c
create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp
base-commit: 1185b5b79a12ba67eb60bee3f75babf7a222fde0
Comments
On 2023-11-08 00:00, Simon Marchi wrote: > This series fixes reading/writing pseudo registers from/to non-current > frames (that is, frames other than frame 0). Currently, we get this: > > (gdb) frame 0 > #0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:38 > 38 pop %rbx > (gdb) p/x $rbx > $1 = 0x2021222324252627 > (gdb) p/x $ebx > $2 = 0x24252627 > (gdb) frame 1 > #1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:58 > 58 call callee > (gdb) p/x $rbx > $3 = 0x1011121314151617 > (gdb) p/x $ebx > $4 = 0x24252627 > > This is a bit surprising, we would expect the last value to be > 0x14151617, the bottom half of the rbx value from frame 1 (the currently > selected frame at that point). Instead, we got the bottom half of the > rbx value from frame 0. This is because pseudo registers are always > read/written from/to the current thread's regcache. > > This series fixes this (as well as writing to pseudo registers) by > making it so pseudo registers are read/written using a frame. Ah, I forgot because it's been so long, but this can be considered a v3 of this series here... https://inbox.sourceware.org/gdb-patches/20181024014333.14143-1-simon.marchi@polymtl.ca/ Simon
Hi Simon, On 11/8/23 05:00, Simon Marchi wrote: > This series fixes reading/writing pseudo registers from/to non-current > frames (that is, frames other than frame 0). Currently, we get this: > > (gdb) frame 0 > #0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:38 > 38 pop %rbx > (gdb) p/x $rbx > $1 = 0x2021222324252627 > (gdb) p/x $ebx > $2 = 0x24252627 > (gdb) frame 1 > #1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:58 > 58 call callee > (gdb) p/x $rbx > $3 = 0x1011121314151617 > (gdb) p/x $ebx > $4 = 0x24252627 > > This is a bit surprising, we would expect the last value to be > 0x14151617, the bottom half of the rbx value from frame 1 (the currently > selected frame at that point). Instead, we got the bottom half of the > rbx value from frame 0. This is because pseudo registers are always > read/written from/to the current thread's regcache. > > This series fixes this (as well as writing to pseudo registers) by > making it so pseudo registers are read/written using a frame. > > Simon Marchi (24): > gdb: don't handle i386 k registers as pseudo registers > gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h > gdb: make store_integer take an array_view > gdb: simplify conditions in > regcache::{read,write,raw_collect,raw_supply}_part > gdb: change regcache interface to use array_view > gdb: fix bugs in {get,put}_frame_register_bytes > gdb: make put_frame_register take an array_view > gdb: change value_of_register and value_of_register_lazy to take the > next frame > gdb: remove frame_register > gdb: make put_frame_register take the next frame > gdb: make put_frame_register_bytes take the next frame > gdb: make get_frame_register_bytes take the next frame > gdb: add value::allocate_register > gdb: read pseudo register through frame > gdb: change parameter name in frame_unwind_register_unsigned > declaration > gdb: rename gdbarch_pseudo_register_write to > gdbarch_deprecated_pseudo_register_write > gdb: add gdbarch_pseudo_register_write that takes a frame > gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write > gdb: make aarch64_za_offsets_from_regnum return za_offsets > gdb: add missing raw register read in > aarch64_sme_pseudo_register_write > gdb: migrate aarch64 to new gdbarch_pseudo_register_write > gdb: migrate arm to gdbarch_pseudo_register_read_value > gdb: migrate arm to new gdbarch_pseudo_register_write > gdb/testsuite: add tests for unwinding of pseudo registers > > gdb/aarch64-tdep.c | 297 +++++----- > gdb/alpha-tdep.c | 11 +- > gdb/amd64-tdep.c | 82 +-- > gdb/arch/arm-get-next-pcs.c | 6 +- > gdb/arch/arm-get-next-pcs.h | 5 +- > gdb/arch/arm.c | 2 +- > gdb/arch/arm.h | 4 +- > gdb/arm-linux-tdep.c | 11 +- > gdb/arm-tdep.c | 145 +++-- > gdb/avr-tdep.c | 3 +- > gdb/bfin-tdep.c | 3 +- > gdb/csky-tdep.c | 4 +- > gdb/defs.h | 39 +- > gdb/dwarf2/expr.c | 22 +- > gdb/dwarf2/frame.c | 5 +- > gdb/eval.c | 3 +- > gdb/findvar.c | 50 +- > gdb/frame-unwind.c | 3 +- > gdb/frame.c | 174 +++--- > gdb/frame.h | 28 +- > gdb/frv-tdep.c | 3 +- > gdb/gdbarch-gen.h | 28 +- > gdb/gdbarch.c | 40 +- > gdb/gdbarch_components.py | 29 +- > gdb/guile/scm-frame.c | 3 +- > gdb/h8300-tdep.c | 3 +- > gdb/i386-tdep.c | 380 ++++-------- > gdb/i386-tdep.h | 15 +- > gdb/i387-tdep.c | 16 +- > gdb/ia64-tdep.c | 18 +- > gdb/infcmd.c | 6 +- > gdb/loongarch-tdep.c | 3 +- > gdb/m32c-tdep.c | 3 +- > gdb/m68hc11-tdep.c | 3 +- > gdb/m68k-tdep.c | 17 +- > gdb/mep-tdep.c | 3 +- > gdb/mi/mi-main.c | 3 +- > gdb/mips-tdep.c | 29 +- > gdb/msp430-tdep.c | 3 +- > gdb/nat/aarch64-hw-point.c | 3 +- > gdb/nat/aarch64-scalable-linux-ptrace.c | 20 +- > gdb/nat/linux-btrace.c | 3 +- > gdb/nds32-tdep.c | 8 +- > gdb/python/py-frame.c | 3 +- > gdb/python/py-unwind.c | 4 +- > gdb/regcache.c | 548 +++++++++++------- > gdb/regcache.h | 113 +++- > gdb/riscv-tdep.c | 13 +- > gdb/rl78-tdep.c | 3 +- > gdb/rs6000-tdep.c | 21 +- > gdb/s12z-tdep.c | 2 +- > gdb/s390-tdep.c | 3 +- > gdb/sh-tdep.c | 9 +- > gdb/sparc-tdep.c | 3 +- > gdb/sparc64-tdep.c | 3 +- > gdb/std-regs.c | 11 +- > .../gdb.arch/aarch64-pseudo-unwind-asm.S | 82 +++ > .../gdb.arch/aarch64-pseudo-unwind.c | 33 ++ > .../gdb.arch/aarch64-pseudo-unwind.exp | 90 +++ > .../gdb.arch/amd64-pseudo-unwind-asm.S | 66 +++ > gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c | 33 ++ > .../gdb.arch/amd64-pseudo-unwind.exp | 91 +++ > .../gdb.arch/arm-pseudo-unwind-asm.S | 81 +++ > .../gdb.arch/arm-pseudo-unwind-legacy-asm.S | 84 +++ > .../gdb.arch/arm-pseudo-unwind-legacy.c | 33 ++ > .../gdb.arch/arm-pseudo-unwind-legacy.exp | 86 +++ > gdb/testsuite/gdb.arch/arm-pseudo-unwind.c | 33 ++ > gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp | 88 +++ > gdb/valops.c | 31 +- > gdb/value.c | 149 +++++ > gdb/value.h | 64 +- > gdb/xtensa-tdep.c | 3 +- > gdbserver/linux-arm-low.cc | 4 +- > gdbserver/regcache.cc | 69 ++- > gdbserver/regcache.h | 6 +- > gdbsupport/common-regcache.cc | 2 +- > gdbsupport/common-regcache.h | 58 +- > gdbsupport/rsp-low.cc | 8 + > gdbsupport/rsp-low.h | 2 + > 79 files changed, 2324 insertions(+), 1144 deletions(-) > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp > > > base-commit: 1185b5b79a12ba67eb60bee3f75babf7a222fde0 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: 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 0xaaaaabce0913 _Z29gdbarch_pseudo_register_writeP7gdbarch14frame_info_ptriN3gdb10array_viewIKhEE^M ../../../repos/binutils-gdb/gdb/gdbarch.c:1927^M 0xaaaaabec892f _Z18put_frame_register14frame_info_ptriN3gdb10array_viewIKhEE^M ../../../repos/binutils-gdb/gdb/frame.c:1462^M 0xaaaaabed0fa7 _Z24put_frame_register_bytes14frame_info_ptrimN3gdb10array_viewIKhEE^M ../../../repos/binutils-gdb/gdb/frame.c:1592^M 0xaaaaac33092b _Z12value_assignP5valueS0_^M ../../../repos/binutils-gdb/gdb/valops.c:1260^M 0xaaaaabe8fcdf _ZN10expression8evaluateEP4type6noside^M ../../../repos/binutils-gdb/gdb/eval.c:111^M 0xaaaaac0cba9f set_command^M ../../../repos/binutils-gdb/gdb/printcmd.c:1542^M 0xaaaaabd9d91b _Z8cmd_funcP16cmd_list_elementPKci^M ../../../repos/binutils-gdb/gdb/cli/cli-decode.c:2735^M 0xaaaaac2af583 _Z15execute_commandPKci^M ../../../repos/binutils-gdb/gdb/top.c:575^M 0xaaaaabe97413 _Z15command_handlerPKc^M ../../../repos/binutils-gdb/gdb/event-top.c:552^M 0xaaaaabe9869f _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE^M ../../../repos/binutils-gdb/gdb/event-top.c:788^M 0xaaaaabe97f17 gdb_rl_callback_handler^M ../../../repos/binutils-gdb/gdb/event-top.c:259^M 0xaaaaac3ce363 rl_callback_read_char^M ../../../../repos/binutils-gdb/readline/readline/callback.c:290^M 0xaaaaabe96e93 gdb_rl_callback_read_char_wrapper_noexcept^M ../../../repos/binutils-gdb/gdb/event-top.c:195^M 0xaaaaabe97d7b gdb_rl_callback_read_char_wrapper^M ../../../repos/binutils-gdb/gdb/event-top.c:234^M 0xaaaaac2ea1f7 stdin_event_handler^M ../../../repos/binutils-gdb/gdb/ui.c:155^M 0xaaaaac860ecf gdb_wait_for_event^M ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:694^M 0xaaaaac8618af gdb_wait_for_event^M ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:593^M 0xaaaaac8618af _Z16gdb_do_one_eventi^M ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:264^M 0xaaaaabffe02f start_event_loop^M ../../../repos/binutils-gdb/gdb/main.c:407^M 0xaaaaabffe02f captured_command_loop^M ../../../repos/binutils-gdb/gdb/main.c:471^M 0xaaaaabfffc73 captured_main^M ../../../repos/binutils-gdb/gdb/main.c:1324^M 0xaaaaabfffc73 _Z8gdb_mainP18captured_main_args^M ../../../repos/binutils-gdb/gdb/main.c:1343^M 0xaaaaabc3b457 main^M ../../../repos/binutils-gdb/gdb/gdb.c:39^M 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; } Please let me know if you want me to narrow this down or provide more info.
> 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 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? Simon diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 1815d78dec40..87e6f9e10ae2 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<const gdb_byte> 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; - gdb_byte raw_buf[raw_reg_size] {}; + int raw_reg_size = register_size (gdbarch, raw_regnum); + gdb::byte_vector raw_buf (raw_reg_size); gdb::array_view<gdb_byte> raw_view (raw_buf); 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
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 > > Simon > > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 1815d78dec40..87e6f9e10ae2 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<const gdb_byte> 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; > - gdb_byte raw_buf[raw_reg_size] {}; > + int raw_reg_size = register_size (gdbarch, raw_regnum); > + gdb::byte_vector raw_buf (raw_reg_size); > gdb::array_view<gdb_byte> raw_view (raw_buf); > 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 >
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<const gdb_byte> 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<gdb_byte> raw_view (raw_buf); + gdb::array_view<gdb_byte> 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
On 2023-11-08 00:16, Simon Marchi wrote: > > > On 2023-11-08 00:00, Simon Marchi wrote: >> This series fixes reading/writing pseudo registers from/to non-current >> frames (that is, frames other than frame 0). Currently, we get this: >> >> (gdb) frame 0 >> #0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:38 >> 38 pop %rbx >> (gdb) p/x $rbx >> $1 = 0x2021222324252627 >> (gdb) p/x $ebx >> $2 = 0x24252627 >> (gdb) frame 1 >> #1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:58 >> 58 call callee >> (gdb) p/x $rbx >> $3 = 0x1011121314151617 >> (gdb) p/x $ebx >> $4 = 0x24252627 >> >> This is a bit surprising, we would expect the last value to be >> 0x14151617, the bottom half of the rbx value from frame 1 (the currently >> selected frame at that point). Instead, we got the bottom half of the >> rbx value from frame 0. This is because pseudo registers are always >> read/written from/to the current thread's regcache. >> >> This series fixes this (as well as writing to pseudo registers) by >> making it so pseudo registers are read/written using a frame. > > Ah, I forgot because it's been so long, but this can be considered a v3 > of this series here... > > https://inbox.sourceware.org/gdb-patches/20181024014333.14143-1-simon.marchi@polymtl.ca/ > > Simon A bit more information that I realized I left out: this series fixes the behavior for AArch64, ARM and AMD64/i386, because those are the three architectures I had tests already written for. It shouldn't be too hard to update other architectures, the slightly more difficult part is to write a test, since it involves writing assembly and CFI directives, not something most people do everyday. Simon
On 11/8/23 14:34, Simon Marchi wrote: > 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<const gdb_byte> 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<gdb_byte> raw_view (raw_buf); > + gdb::array_view<gdb_byte> 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 I managed to run a Debian AArch64 image in qemu, with SVE support, so I was able to reproduce the failures you mentioned. In the end, here's a version of aarch64_pseudo_write_1 that works for me (written as to minimize the number of unnecessary changes, since that seems to introduce unexpected bugs...). static void aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame, int regnum_offset, gdb::array_view<const gdb_byte> buf) { unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset; /* Enough space for a full vector register. */ int raw_reg_size = register_size (gdbarch, raw_regnum); gdb_byte raw_buf[raw_reg_size]; gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); /* Ensure the register buffer is zero, we want gdb writes of the various 'scalar' pseudo registers to behavior like architectural writes, register width bytes are written the remainder are set to zero. */ memset (raw_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM)); gdb::array_view<gdb_byte> raw_view (raw_buf, raw_reg_size); copy (buf, raw_view.slice (0, buf.size ())); put_frame_register (next_frame, raw_regnum, raw_view); } Simon
On 11/7/23 9:00 PM, Simon Marchi wrote: > This series fixes reading/writing pseudo registers from/to non-current > frames (that is, frames other than frame 0). Currently, we get this: > > (gdb) frame 0 > #0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:38 > 38 pop %rbx > (gdb) p/x $rbx > $1 = 0x2021222324252627 > (gdb) p/x $ebx > $2 = 0x24252627 > (gdb) frame 1 > #1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:58 > 58 call callee > (gdb) p/x $rbx > $3 = 0x1011121314151617 > (gdb) p/x $ebx > $4 = 0x24252627 > > This is a bit surprising, we would expect the last value to be > 0x14151617, the bottom half of the rbx value from frame 1 (the currently > selected frame at that point). Instead, we got the bottom half of the > rbx value from frame 0. This is because pseudo registers are always > read/written from/to the current thread's regcache. > > This series fixes this (as well as writing to pseudo registers) by > making it so pseudo registers are read/written using a frame. > > Simon Marchi (24): > gdb: don't handle i386 k registers as pseudo registers > gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h > gdb: make store_integer take an array_view > gdb: simplify conditions in > regcache::{read,write,raw_collect,raw_supply}_part > gdb: change regcache interface to use array_view > gdb: fix bugs in {get,put}_frame_register_bytes > gdb: make put_frame_register take an array_view > gdb: change value_of_register and value_of_register_lazy to take the > next frame > gdb: remove frame_register > gdb: make put_frame_register take the next frame > gdb: make put_frame_register_bytes take the next frame > gdb: make get_frame_register_bytes take the next frame > gdb: add value::allocate_register > gdb: read pseudo register through frame > gdb: change parameter name in frame_unwind_register_unsigned > declaration > gdb: rename gdbarch_pseudo_register_write to > gdbarch_deprecated_pseudo_register_write > gdb: add gdbarch_pseudo_register_write that takes a frame > gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write > gdb: make aarch64_za_offsets_from_regnum return za_offsets > gdb: add missing raw register read in > aarch64_sme_pseudo_register_write > gdb: migrate aarch64 to new gdbarch_pseudo_register_write > gdb: migrate arm to gdbarch_pseudo_register_read_value > gdb: migrate arm to new gdbarch_pseudo_register_write > gdb/testsuite: add tests for unwinding of pseudo registers > > gdb/aarch64-tdep.c | 297 +++++----- > gdb/alpha-tdep.c | 11 +- > gdb/amd64-tdep.c | 82 +-- > gdb/arch/arm-get-next-pcs.c | 6 +- > gdb/arch/arm-get-next-pcs.h | 5 +- > gdb/arch/arm.c | 2 +- > gdb/arch/arm.h | 4 +- > gdb/arm-linux-tdep.c | 11 +- > gdb/arm-tdep.c | 145 +++-- > gdb/avr-tdep.c | 3 +- > gdb/bfin-tdep.c | 3 +- > gdb/csky-tdep.c | 4 +- > gdb/defs.h | 39 +- > gdb/dwarf2/expr.c | 22 +- > gdb/dwarf2/frame.c | 5 +- > gdb/eval.c | 3 +- > gdb/findvar.c | 50 +- > gdb/frame-unwind.c | 3 +- > gdb/frame.c | 174 +++--- > gdb/frame.h | 28 +- > gdb/frv-tdep.c | 3 +- > gdb/gdbarch-gen.h | 28 +- > gdb/gdbarch.c | 40 +- > gdb/gdbarch_components.py | 29 +- > gdb/guile/scm-frame.c | 3 +- > gdb/h8300-tdep.c | 3 +- > gdb/i386-tdep.c | 380 ++++-------- > gdb/i386-tdep.h | 15 +- > gdb/i387-tdep.c | 16 +- > gdb/ia64-tdep.c | 18 +- > gdb/infcmd.c | 6 +- > gdb/loongarch-tdep.c | 3 +- > gdb/m32c-tdep.c | 3 +- > gdb/m68hc11-tdep.c | 3 +- > gdb/m68k-tdep.c | 17 +- > gdb/mep-tdep.c | 3 +- > gdb/mi/mi-main.c | 3 +- > gdb/mips-tdep.c | 29 +- > gdb/msp430-tdep.c | 3 +- > gdb/nat/aarch64-hw-point.c | 3 +- > gdb/nat/aarch64-scalable-linux-ptrace.c | 20 +- > gdb/nat/linux-btrace.c | 3 +- > gdb/nds32-tdep.c | 8 +- > gdb/python/py-frame.c | 3 +- > gdb/python/py-unwind.c | 4 +- > gdb/regcache.c | 548 +++++++++++------- > gdb/regcache.h | 113 +++- > gdb/riscv-tdep.c | 13 +- > gdb/rl78-tdep.c | 3 +- > gdb/rs6000-tdep.c | 21 +- > gdb/s12z-tdep.c | 2 +- > gdb/s390-tdep.c | 3 +- > gdb/sh-tdep.c | 9 +- > gdb/sparc-tdep.c | 3 +- > gdb/sparc64-tdep.c | 3 +- > gdb/std-regs.c | 11 +- > .../gdb.arch/aarch64-pseudo-unwind-asm.S | 82 +++ > .../gdb.arch/aarch64-pseudo-unwind.c | 33 ++ > .../gdb.arch/aarch64-pseudo-unwind.exp | 90 +++ > .../gdb.arch/amd64-pseudo-unwind-asm.S | 66 +++ > gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c | 33 ++ > .../gdb.arch/amd64-pseudo-unwind.exp | 91 +++ > .../gdb.arch/arm-pseudo-unwind-asm.S | 81 +++ > .../gdb.arch/arm-pseudo-unwind-legacy-asm.S | 84 +++ > .../gdb.arch/arm-pseudo-unwind-legacy.c | 33 ++ > .../gdb.arch/arm-pseudo-unwind-legacy.exp | 86 +++ > gdb/testsuite/gdb.arch/arm-pseudo-unwind.c | 33 ++ > gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp | 88 +++ > gdb/valops.c | 31 +- > gdb/value.c | 149 +++++ > gdb/value.h | 64 +- > gdb/xtensa-tdep.c | 3 +- > gdbserver/linux-arm-low.cc | 4 +- > gdbserver/regcache.cc | 69 ++- > gdbserver/regcache.h | 6 +- > gdbsupport/common-regcache.cc | 2 +- > gdbsupport/common-regcache.h | 58 +- > gdbsupport/rsp-low.cc | 8 + > gdbsupport/rsp-low.h | 2 + > 79 files changed, 2324 insertions(+), 1144 deletions(-) > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c > create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp > > > base-commit: 1185b5b79a12ba67eb60bee3f75babf7a222fde0 I did not review the aarch64/arm changes very thoroughly (patches 19-23), but the rest all look fine to me aside from the one comment I had on patch 18. (So to be clear, you can add my Reviewed-by on all of 1-18.) Looks like Luis is helping to validate the arm changes. I certainly have a use case for this for CHERI support where GPRs are also extended (so ideally I'd like to treat the 64-bit GPRs as pseudos of the 129-bit capability registers), and also for Morello in particular where the stack pointer is banked in userland and can vary by stack frame depending on a permission in the PC as to which real register it maps on to.
On 11/11/23 15:26, John Baldwin wrote: > I did not review the aarch64/arm changes very thoroughly (patches 19-23), > but the rest all look fine to me aside from the one comment I had on > patch 18. > > (So to be clear, you can add my Reviewed-by on all of 1-18.) Looks like > Luis is helping to validate the arm changes. Thanks for reviewing, will add. Simon
Simon, On 11/9/23 19:04, Simon Marchi wrote: > On 11/8/23 14:34, Simon Marchi wrote: >> 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<const gdb_byte> 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<gdb_byte> raw_view (raw_buf); >> + gdb::array_view<gdb_byte> 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 > > I managed to run a Debian AArch64 image in qemu, with SVE support, so I > was able to reproduce the failures you mentioned. In the end, here's a > version of aarch64_pseudo_write_1 that works for me (written as to > minimize the number of unnecessary changes, since that seems to > introduce unexpected bugs...). > > static void > aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame, > int regnum_offset, > gdb::array_view<const gdb_byte> buf) > { > unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset; > > /* Enough space for a full vector register. */ > int raw_reg_size = register_size (gdbarch, raw_regnum); > gdb_byte raw_buf[raw_reg_size]; > gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); > > /* Ensure the register buffer is zero, we want gdb writes of the > various 'scalar' pseudo registers to behavior like architectural > writes, register width bytes are written the remainder are set to > zero. */ > memset (raw_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM)); > > gdb::array_view<gdb_byte> raw_view (raw_buf, raw_reg_size); > copy (buf, raw_view.slice (0, buf.size ())); > put_frame_register (next_frame, raw_regnum, raw_view); > } > > Simon Sorry for the late reply. I was out a couple days last week. The above seems to make gdb.arch/aarch64-fp.exp happy, but gdb.arch/aarch64-pseudo-unwind.exp is still slightly unhappy: 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: continue to breakpoint: continue to break_here_c FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value This was tested on hardware (AWS's Graviton 3). Let me play with it a bit to understand what's up. I also see a SIGSEGV in the test that shouldn't be there. I'm guessing some sort of raw register corruption, as it leads to pc == 0x0.
On 11/13/23 13:10, Luis Machado wrote: > Simon, > > On 11/9/23 19:04, Simon Marchi wrote: >> On 11/8/23 14:34, Simon Marchi wrote: >>> 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<const gdb_byte> 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<gdb_byte> raw_view (raw_buf); >>> + gdb::array_view<gdb_byte> 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 >> >> I managed to run a Debian AArch64 image in qemu, with SVE support, so I >> was able to reproduce the failures you mentioned. In the end, here's a >> version of aarch64_pseudo_write_1 that works for me (written as to >> minimize the number of unnecessary changes, since that seems to >> introduce unexpected bugs...). >> >> static void >> aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame, >> int regnum_offset, >> gdb::array_view<const gdb_byte> buf) >> { >> unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset; >> >> /* Enough space for a full vector register. */ >> int raw_reg_size = register_size (gdbarch, raw_regnum); >> gdb_byte raw_buf[raw_reg_size]; >> gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); >> >> /* Ensure the register buffer is zero, we want gdb writes of the >> various 'scalar' pseudo registers to behavior like architectural >> writes, register width bytes are written the remainder are set to >> zero. */ >> memset (raw_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM)); >> >> gdb::array_view<gdb_byte> raw_view (raw_buf, raw_reg_size); >> copy (buf, raw_view.slice (0, buf.size ())); >> put_frame_register (next_frame, raw_regnum, raw_view); >> } >> >> Simon > > Sorry for the late reply. I was out a couple days last week. > > The above seems to make gdb.arch/aarch64-fp.exp happy, but gdb.arch/aarch64-pseudo-unwind.exp is still slightly unhappy: > > 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: continue to breakpoint: continue to break_here_c > FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value > > This was tested on hardware (AWS's Graviton 3). Let me play with it a bit to understand what's up. I also see a > SIGSEGV in the test that shouldn't be there. I'm guessing some sort of raw register corruption, as it leads to > pc == 0x0. I think I understand what's going on here. In the callee, we have CFI stating we're saving the following registers: .cfi_offset 29, -16 // x29 (SP) .cfi_offset 30, -8 // x30 (LR) .cfi_offset 72, -32 // v8 -> 128-bit in size So we're reserving a slot of 16 bytes to save v8, which is fine. But later, when we go up a frame and try to put a v8 pseudo-register value to the frame there are two distinct scenarios. The first one is on a sytem that doesn't support SVE or that does support SVE but the vector length is 128-bit, which means z8 (the raw register) is the same size as v8 (the pseudo-register). This works fine, because we use the size of the raw register to put the pseudo-register value to the frame. Now, if we have SVE support and the vector length is bigger than 128-bit (in my case it is 256-bit), then using the size of the raw register (z8) will put 32 bytes into a slot of 16 bytes, corrupting (IIUC) both SP and LR, and leading to a segfault. Technically we would be fine with using the correct size of the slot when putting a pseudo-register to a frame, but I think there are further complications. Take, for instance, the following text from the aadwarf64 (https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst), note 5 from DWARF registers 64-95: "In a similar manner to the general register file the size of an FP/Advanced SIMD register is taken from some external context to the register number. If no context is available then only the least significant 64 bits of the register are referenced. In particular this means that the most significant part of a SIMD register is unrecoverable by frame unwinding." So this makes me think we might have situations where we don't know exactly what the size of the slot is. We may have saved all 128 bits of a v register, but it may also be the case we only saved 64 bits. How we distinguish what size to use for putting a v pseudo-register to a frame isn't completely clear to me at this point. I'm wondering if we should instead use x/w registers for testing this functionality for aarch64.