From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 61BD73858CD1 for ; Tue, 14 Nov 2023 12:20:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 61BD73858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 61BD73858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699964426; cv=none; b=MDIoYm5mxmvqkEYt8VnxZt9p9dj5/T0XgJHSAPIV18AOGps8ygjN10GAnNvCGcAjo/r3KhNsPkpJrwiZquxCZIQtXA7QzjCoUQKQM+Hv3EZIhQoN4LUnOa0pPC7XcBg4HbQPjDFCZ4SjAm0QfjdxLvWRHUx3OrL0YZd36mC2c1w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699964426; c=relaxed/simple; bh=wv7z4H2aqymzUToN2zRrTuRuDywHYqbMgksI/5L/DcA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Vf5/2GO0CDtQ2Bk4uyZ0UhsAx9qP3wLqchINNaW5eAXR3Dlk+Bl0eK7xXv3iZpjhqLnesi3wiAN840q9mOQGQuaenU1ZHXgPtPpZm71m3xcYAtQ/StT7Y5gqnMZNihFoip6zvhcOa6lduj7LKWwfMNiiCvB716++Q+N0wp4WaAk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699964424; 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: in-reply-to:in-reply-to:references:references; bh=XQWXz6inYAhwMoB68n667X5V2v75wlRQHc9cDINr2fo=; b=e2lTw34TPYFnJOtQr17ZY6KiyYGGqYiP55M0I8/sx48eBUD6pyPoglRzprsq3Uj+B7PB11 4BjPGdhQGFaL0RaxTmWVgp0W82g0qkP6dK0CdQw/jPpiCjR/i8wBD+RWeA7w1/nwXiFZXi R6fTxdaLC0DlnEBaKHPLYTDmYAy6jrc= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-632-_wpQrBflMru5sILA65P_Jg-1; Tue, 14 Nov 2023 07:20:22 -0500 X-MC-Unique: _wpQrBflMru5sILA65P_Jg-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-32da39f7f2bso2367224f8f.0 for ; Tue, 14 Nov 2023 04:20:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699964421; x=1700569221; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XQWXz6inYAhwMoB68n667X5V2v75wlRQHc9cDINr2fo=; b=Mw1fJbD1jdgePAuWu5+okGuQG2vtpbf/SqueLjm/giR0ybmXR/wxleavA2/4HtWHvv DbRhzuptBy91wHcvAQhUyxZFH5zGpq4jhP1N2z5QMG46QSJoW2wN0GmOuS+9fTNGWuF2 Hy+GSUNXwoKT2rk/hdydchGfelTmWPJDugQbt3c7yU+EVo5962BbNPWwIf+V+913Nkd2 oohlk9V8Xz3394rHKGNU1L+2x3DiZUznQsDAwBY7vWwf8u//cnjhHlhK+nxgZipZPBoC UOf799qlhaIVVayojqud1q3p/iBc1KXiD0i2hT7vXBeq+7m4tWIi7Fm713lYrGSdxDtA O1zA== X-Gm-Message-State: AOJu0YzsKW+NQwVJ6L13KfTV3T19//ME9uDi/I0IrCVPZLweNst/hWMK nJmtF6y5GRacTmRy8jtYa238NaSQxMi8lxPUp9dGghFF4YjNt5TN3NUsvRXgc8JE9lxWU1dOvkP WefX896wcSCNmOR3GtlKUvg== X-Received: by 2002:a5d:5089:0:b0:32f:8214:5ba1 with SMTP id a9-20020a5d5089000000b0032f82145ba1mr6338493wrt.45.1699964421529; Tue, 14 Nov 2023 04:20:21 -0800 (PST) X-Google-Smtp-Source: AGHT+IF/GTHyv/tXQIyoJ4dmzfUbPkBMoVQFYztcs7TFFOHmYkMoblzYJ+2MN+bvEzAe0W/FfVXFMQ== X-Received: by 2002:a5d:5089:0:b0:32f:8214:5ba1 with SMTP id a9-20020a5d5089000000b0032f82145ba1mr6338483wrt.45.1699964421095; Tue, 14 Nov 2023 04:20:21 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id w10-20020a5d680a000000b0032d893d8dc8sm7751854wru.2.2023.11.14.04.20.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 04:20:20 -0800 (PST) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame In-Reply-To: <20231108051222.1275306-18-simon.marchi@polymtl.ca> References: <20231108051222.1275306-1-simon.marchi@polymtl.ca> <20231108051222.1275306-18-simon.marchi@polymtl.ca> Date: Tue, 14 Nov 2023 12:20:19 +0000 Message-ID: <87o7fwr0fw.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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: Simon Marchi writes: > From: Simon Marchi > > Add a new variant of gdbarch_pseudo_register_write that takes a > frame_info in order to write raw registers. Use this new method when > available: > > - in put_frame_register, when trying to write a pseudo register to a given frame > - in regcache::cooked_write > > No implementation is migrated to use this new method (that will come in > subsequent patches), so no behavior change is expected here. > > The objective is to fix writing pseudo registers to non-current > frames. See previous commit "gdb: read pseudo register through > frame" for a more detailed explanation. > > Change-Id: Ie7fe364a15a4d86c2ecb09de2b4baa08c45555ac > --- > gdb/frame.c | 9 +++++- > gdb/gdbarch-gen.h | 15 +++++++++- > gdb/gdbarch.c | 32 ++++++++++++++++++++ > gdb/gdbarch_components.py | 19 ++++++++++++ > gdb/regcache.c | 4 +++ > gdb/value.c | 63 +++++++++++++++++++++++++++++++++++++++ > gdb/value.h | 22 ++++++++++++++ > 7 files changed, 162 insertions(+), 2 deletions(-) > > diff --git a/gdb/frame.c b/gdb/frame.c > index 3035f87369ca..81836d6d5357 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -1452,7 +1452,14 @@ put_frame_register (frame_info_ptr next_frame, int regnum, > break; > } > case lval_register: > - get_current_regcache ()->cooked_write (realnum, buf, 1.0f); > + /* Not sure if that's always true... but we have a problem if not. */ > + gdb_assert (size == register_size (gdbarch, realnum)); > + > + if (realnum < gdbarch_num_regs (gdbarch) > + || !gdbarch_pseudo_register_write_p (gdbarch)) > + get_current_regcache ()->cooked_write (realnum, buf, 1.0f); > + else > + gdbarch_pseudo_register_write (gdbarch, next_frame, realnum, buf); > break; > default: > error (_("Attempt to assign to an unmodifiable value.")); > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h > index 3160aa8a9613..9cf58490d0b6 100644 > --- a/gdb/gdbarch-gen.h > +++ b/gdb/gdbarch-gen.h > @@ -200,12 +200,25 @@ typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarc > extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, frame_info_ptr next_frame, int cookednum); > extern void set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value); > > +/* Write bytes in BUF to pseudo register with number PSEUDO_REG_NUM. > + > + Raw registers backing the pseudo register should be written to using > + NEXT_FRAME. */ > + > +extern bool gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch); > + > +typedef void (gdbarch_pseudo_register_write_ftype) (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view buf); > +extern void gdbarch_pseudo_register_write (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view buf); > +extern void set_gdbarch_pseudo_register_write (struct gdbarch *gdbarch, gdbarch_pseudo_register_write_ftype *pseudo_register_write); > + > /* Write bytes to a pseudo register. > > This is marked as deprecated because it gets passed a regcache for > implementations to write raw registers in. This doesn't work for unwound > frames, where the raw registers backing the pseudo registers may have been > - saved elsewhere. */ > + saved elsewhere. > + > + Implementations should be migrated to implement pseudo_register_write instead. */ > > extern bool gdbarch_deprecated_pseudo_register_write_p (struct gdbarch *gdbarch); > > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c > index e198d339f6ba..d584305fefb2 100644 > --- a/gdb/gdbarch.c > +++ b/gdb/gdbarch.c > @@ -74,6 +74,7 @@ struct gdbarch > gdbarch_virtual_frame_pointer_ftype *virtual_frame_pointer = legacy_virtual_frame_pointer; > gdbarch_pseudo_register_read_ftype *pseudo_register_read = nullptr; > gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value = nullptr; > + gdbarch_pseudo_register_write_ftype *pseudo_register_write = nullptr; > gdbarch_deprecated_pseudo_register_write_ftype *deprecated_pseudo_register_write = nullptr; > int num_regs = -1; > int num_pseudo_regs = 0; > @@ -330,6 +331,7 @@ verify_gdbarch (struct gdbarch *gdbarch) > /* Skip verify of virtual_frame_pointer, invalid_p == 0 */ > /* Skip verify of pseudo_register_read, has predicate. */ > /* Skip verify of pseudo_register_read_value, has predicate. */ > + /* Skip verify of pseudo_register_write, has predicate. */ > /* Skip verify of deprecated_pseudo_register_write, has predicate. */ > if (gdbarch->num_regs == -1) > log.puts ("\n\tnum_regs"); > @@ -649,6 +651,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) > gdb_printf (file, > "gdbarch_dump: pseudo_register_read_value = <%s>\n", > host_address_to_string (gdbarch->pseudo_register_read_value)); > + gdb_printf (file, > + "gdbarch_dump: gdbarch_pseudo_register_write_p() = %d\n", > + gdbarch_pseudo_register_write_p (gdbarch)); > + gdb_printf (file, > + "gdbarch_dump: pseudo_register_write = <%s>\n", > + host_address_to_string (gdbarch->pseudo_register_write)); > gdb_printf (file, > "gdbarch_dump: gdbarch_deprecated_pseudo_register_write_p() = %d\n", > gdbarch_deprecated_pseudo_register_write_p (gdbarch)); > @@ -1902,6 +1910,30 @@ set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, > gdbarch->pseudo_register_read_value = pseudo_register_read_value; > } > > +bool > +gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch) > +{ > + gdb_assert (gdbarch != NULL); > + return gdbarch->pseudo_register_write != NULL; > +} > + > +void > +gdbarch_pseudo_register_write (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view buf) > +{ > + gdb_assert (gdbarch != NULL); > + gdb_assert (gdbarch->pseudo_register_write != NULL); > + if (gdbarch_debug >= 2) > + gdb_printf (gdb_stdlog, "gdbarch_pseudo_register_write called\n"); > + gdbarch->pseudo_register_write (gdbarch, next_frame, pseudo_reg_num, buf); > +} > + > +void > +set_gdbarch_pseudo_register_write (struct gdbarch *gdbarch, > + gdbarch_pseudo_register_write_ftype pseudo_register_write) > +{ > + gdbarch->pseudo_register_write = pseudo_register_write; > +} > + > bool > gdbarch_deprecated_pseudo_register_write_p (struct gdbarch *gdbarch) > { > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py > index 1100da160550..ce8169b19a57 100644 > --- a/gdb/gdbarch_components.py > +++ b/gdb/gdbarch_components.py > @@ -418,6 +418,23 @@ never be called. > predicate=True, > ) > > +Method( > + comment=""" > +Write bytes in BUF to pseudo register with number PSEUDO_REG_NUM. > + > +Raw registers backing the pseudo register should be written to using > +NEXT_FRAME. > +""", > + type="void", > + name="pseudo_register_write", > + params=[ > + ("frame_info_ptr", "next_frame"), > + ("int", "pseudo_reg_num"), > + ("gdb::array_view", "buf"), > + ], > + predicate=True, > +) > + > Method( > comment=""" > Write bytes to a pseudo register. > @@ -426,6 +443,8 @@ This is marked as deprecated because it gets passed a regcache for > implementations to write raw registers in. This doesn't work for unwound > frames, where the raw registers backing the pseudo registers may have been > saved elsewhere. > + > +Implementations should be migrated to implement pseudo_register_write instead. > """, > type="void", > name="deprecated_pseudo_register_write", > diff --git a/gdb/regcache.c b/gdb/regcache.c > index dadc949434ee..16b117e767be 100644 > --- a/gdb/regcache.c > +++ b/gdb/regcache.c > @@ -929,6 +929,10 @@ regcache::cooked_write (int regnum, gdb::array_view src, float) > > if (regnum < num_raw_registers ()) > raw_write (regnum, src, 1.0f); > + else if (gdbarch_pseudo_register_write_p (m_descr->gdbarch)) > + gdbarch_pseudo_register_write > + (m_descr->gdbarch, get_next_frame_sentinel_okay (get_current_frame ()), > + regnum, src); > else > gdbarch_deprecated_pseudo_register_write (m_descr->gdbarch, this, regnum, > src.data ()); > diff --git a/gdb/value.c b/gdb/value.c > index 27a79dc62fc1..51dca972a587 100644 > --- a/gdb/value.c > +++ b/gdb/value.c > @@ -4057,6 +4057,22 @@ pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num, > > /* See value.h. */ > > +void > +pseudo_to_raw_part (frame_info_ptr next_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_num, int raw_offset) > +{ > + int raw_reg_size = register_size (get_frame_arch (next_frame), raw_reg_num); > + > + /* When overflowing a register, put_frame_register_bytes writes to the > + subsequent registers. We don't want that behavior here, so make sure > + the write is wholly within register RAW_REG_NUM. */ > + gdb_assert (raw_offset + pseudo_buf.size () <= raw_reg_size); > + put_frame_register_bytes (next_frame, raw_reg_num, raw_offset, pseudo_buf); > +} > + > +/* See value.h. */ > + > value * > pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num, > int raw_reg_1_num, int raw_reg_2_num) > @@ -4080,6 +4096,27 @@ pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num, > return pseudo_reg_val; > } > > +void > +pseudo_to_concat_raw (frame_info_ptr next_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_1_num, int raw_reg_2_num) This function, and its overload are missing a /* See value.h. */ comment. > +{ > + int src_offset = 0; > + gdbarch *arch = frame_unwind_arch (next_frame); > + > + int raw_reg_1_size = register_size (arch, raw_reg_1_num); > + put_frame_register_bytes (next_frame, raw_reg_1_num, 0, > + pseudo_buf.slice (src_offset, raw_reg_1_size)); > + src_offset += raw_reg_1_size; > + > + int raw_reg_2_size = register_size (arch, raw_reg_2_num); > + put_frame_register_bytes (next_frame, raw_reg_2_num, 0, > + pseudo_buf.slice (src_offset, raw_reg_2_size)); > + src_offset += raw_reg_2_size; > + > + gdb_assert (src_offset == pseudo_buf.size ()); > +} > + > /* See value.h. */ > > value * > @@ -4111,6 +4148,32 @@ pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num, > return pseudo_reg_val; > } > > +void > +pseudo_to_concat_raw (frame_info_ptr next_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_1_num, int raw_reg_2_num, int raw_reg_3_num) > +{ > + int src_offset = 0; > + gdbarch *arch = frame_unwind_arch (next_frame); > + > + int raw_reg_1_size = register_size (arch, raw_reg_1_num); > + put_frame_register_bytes (next_frame, raw_reg_1_num, 0, > + pseudo_buf.slice (src_offset, raw_reg_1_size)); > + src_offset += raw_reg_1_size; > + > + int raw_reg_2_size = register_size (arch, raw_reg_2_num); > + put_frame_register_bytes (next_frame, raw_reg_2_num, 0, > + pseudo_buf.slice (src_offset, raw_reg_2_size)); > + src_offset += raw_reg_2_size; > + > + int raw_reg_3_size = register_size (arch, raw_reg_3_num); > + put_frame_register_bytes (next_frame, raw_reg_3_num, 0, > + pseudo_buf.slice (src_offset, raw_reg_3_size)); > + src_offset += raw_reg_3_size; > + > + gdb_assert (src_offset == pseudo_buf.size ()); > +} > + > /* Implementation of the convenience function $_isvoid. */ > > static struct value * > diff --git a/gdb/value.h b/gdb/value.h > index b5c097ad58bf..6a74d4e2c2ee 100644 > --- a/gdb/value.h > +++ b/gdb/value.h > @@ -1661,6 +1661,13 @@ struct scoped_array_length_limiting > value *pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num, > int raw_reg_num, int raw_offset); > > +/* Write PSEUDO_BUF, the contents of a pseudo register, to part of raw register > + RAW_REG_NUM starting at RAW_OFFSET. */ > + > +void pseudo_to_raw_part (frame_info_ptr this_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_num, int raw_offset); > + > /* Create a value for pseudo register PSEUDO_REG_NUM by concatenating raw > registers RAW_REG_1_NUM and RAW_REG_2_NUM. > > @@ -1670,10 +1677,25 @@ value *pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num, > value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num, > int raw_reg_1_num, int raw_reg_2_num); > > +/* Write PSEUDO_BUF, the contents of a pseudo register, to the two raw registers > + RAW_REG_1_NUM and RAW_REG_2_NUM. */ > + > +void pseudo_to_concat_raw (frame_info_ptr this_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_1_num, int raw_reg_2_num); > + > /* Same as the above, but with three raw registers. */ > > value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num, > int raw_reg_1_num, int raw_reg_2_num, > int raw_reg_3_num); > > +/* Write PSEUDO_BUF, the contents of a pseudo register, to the tthreewo raw > + registers RAW_REG_1_NUM, RAW_REG_2_NUM and RAW_REG_3_NUM. */ typo s/tthreewo/three/. Thanks, Andrew > + > +void pseudo_to_concat_raw (frame_info_ptr this_frame, > + gdb::array_view pseudo_buf, > + int raw_reg_1_num, int raw_reg_2_num, > + int raw_reg_3_num); > + > #endif /* !defined (VALUE_H) */ > -- > 2.42.1