From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Sb8xBaV1jGC6cAAAWB0awg (envelope-from ) for ; Fri, 30 Apr 2021 17:24:53 -0400 Received: by simark.ca (Postfix, from userid 112) id 061971F11C; Fri, 30 Apr 2021 17:24:53 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id ED3241E813 for ; Fri, 30 Apr 2021 17:24:51 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4C3473857828; Fri, 30 Apr 2021 21:24:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4C3473857828 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619817891; bh=0Aozh1Bxs/OAwUewiiInPgijC5sCcs6WZAyYp4+yzUY=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=lu3GXK1/EyhRNHqdEhbw/RIfAxu3bG7f3sy7Xj0RkqZxo6zgmHaPgwLmFmwFAbSJQ ksXfWXlZjA2FXlO/rch+VeptLnEdqVKpSoCAxU7IO1O0YPNG+NdCPhMVbAcHXMgTGq CpBJvP7pB0Mrk42PCwA+e81991BmIaC3aQ04hjOU= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id CC70B3857828 for ; Fri, 30 Apr 2021 21:24:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CC70B3857828 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 13ULOfPW022026 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 30 Apr 2021 17:24:46 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 13ULOfPW022026 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 54C2D1E813; Fri, 30 Apr 2021 17:24:41 -0400 (EDT) Subject: Re: [PATCH 19/43] Add new memory access interface to expr.c To: Zoran Zaric , gdb-patches@sourceware.org References: <20210301144620.103016-1-Zoran.Zaric@amd.com> <20210301144620.103016-20-Zoran.Zaric@amd.com> Message-ID: <764283de-8366-64da-d5d9-859cdef5c4f5@polymtl.ca> Date: Fri, 30 Apr 2021 17:24:41 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210301144620.103016-20-Zoran.Zaric@amd.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 30 Apr 2021 21:24:41 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2021-03-01 9:45 a.m., Zoran Zaric via Gdb-patches wrote: > DWARF expression evaluator is currently using a few different > interfaces for memory access: write_memory_with_notification, > read_value_memory, read_memory. > > They all seem incosistent, while some of them even need a struct > value typed argument to be present. > > This patch is simplifying that interface by replacing it with two new > low level functions: read_from_memory and write_to_memory. > > The advantage of this new interface is that it behaves in the same way > as the register access interface from the previous patch. Both of these > have the same error returning policy, which will be usefull for the > following patches. > > * dwarf2/expr.c (xfer_memory): New function. > (read_from_memory): New function. > (write_to_memory): New function. > (rw_pieced_value): Now calls the read_from_memory and > write_to_memory functions. > --- > gdb/dwarf2/expr.c | 187 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 144 insertions(+), 43 deletions(-) > > diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c > index 5a1fd5b941f..1abece7d173 100644 > --- a/gdb/dwarf2/expr.c > +++ b/gdb/dwarf2/expr.c > @@ -32,6 +32,8 @@ > #include "frame.h" > #include "gdbsupport/underlying.h" > #include "gdbarch.h" > +#include "inferior.h" > +#include "observable.h" > > /* Cookie for gdbarch data. */ > > @@ -196,6 +198,85 @@ write_to_register (struct frame_info *frame, int regnum, > return; > } > > +/* Helper for read_from_memory and write_to_memory. */ > + > +static void > +xfer_memory (CORE_ADDR address, gdb_byte *readbuf, > + const gdb_byte *writebuf, > + size_t length, bool stack, int *unavailable) unavailable should probably be bool (end-to-end). > +{ > + *unavailable = 0; > + > + enum target_object object > + = stack ? TARGET_OBJECT_STACK_MEMORY : TARGET_OBJECT_MEMORY; > + > + ULONGEST xfered_total = 0; > + > + while (xfered_total < length) > + { > + ULONGEST xfered_partial; > + > + enum target_xfer_status status > + = target_xfer_partial (current_top_target (), object, NULL, > + (readbuf != nullptr > + ? readbuf + xfered_total > + : nullptr), > + (writebuf != nullptr > + ? writebuf + xfered_total > + : nullptr), > + address + xfered_total, length - xfered_total, > + &xfered_partial); > + > + if (status == TARGET_XFER_OK) > + { > + xfered_total += xfered_partial; > + QUIT; > + } > + else if (status == TARGET_XFER_UNAVAILABLE) > + { > + *unavailable = 1; > + return; > + } > + else if (status == TARGET_XFER_EOF) > + memory_error (TARGET_XFER_E_IO, address + xfered_total); > + else > + memory_error (status, address + xfered_total); > + } > +} > + > +/* Read LENGTH bytes of memory contents starting at ADDRESS. > + > + The data read is copied to a caller-managed buffer BUF. STACK BUF -> BUFFER. > + indicates whether the memory range specified belongs to a stack > + memory region. > + > + If the memory is unavailable, the UNAVAILABLE output is set. */ > + > +static void > +read_from_memory (CORE_ADDR address, gdb_byte *buffer, > + size_t length, bool stack, int *unavailable) > +{ > + xfer_memory (address, buffer, nullptr, length, stack, unavailable); > +} > + > +/* Write LENGTH bytes of memory contents starting at ADDRESS. > + > + The data written is copied from a caller-managed buffer buf. STACK buf -> BUFFER > + indicates whether the memory range specified belongs to a stack > + memory region. > + > + If the memory is unavailable, the UNAVAILABLE output is set. */ > + > +static void > +write_to_memory (CORE_ADDR address, const gdb_byte *buffer, > + size_t length, bool stack, int *unavailable) > +{ > + xfer_memory (address, nullptr, buffer, length, stack, unavailable); > + > + gdb::observers::memory_changed.notify (current_inferior (), address, > + length, buffer); > +} > + > struct piece_closure > { > /* Reference count. */ > @@ -381,66 +462,86 @@ rw_pieced_value (struct value *v, struct value *from) > bits_to_skip += p->offset; > > CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8; > + bool in_stack_memory = p->v.mem.in_stack_memory; > + int unavail = 0; > > if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0 > && offset % 8 == 0) > { > /* Everything is byte-aligned; no buffer needed. */ > if (from != NULL) > - write_memory_with_notification (start_addr, > - (from_contents > - + offset / 8), > - this_size_bits / 8); > + write_to_memory (start_addr, (from_contents + offset / 8), > + this_size_bits / 8, in_stack_memory, > + &unavail); > else > - read_value_memory (v, offset, > - p->v.mem.in_stack_memory, > - p->v.mem.addr + bits_to_skip / 8, > - v_contents + offset / 8, > - this_size_bits / 8); > - break; > - } > - > - this_size = bits_to_bytes (bits_to_skip, this_size_bits); > - buffer.resize (this_size); > - > - if (from == NULL) > - { > - /* Read mode. */ > - read_value_memory (v, offset, > - p->v.mem.in_stack_memory, > - p->v.mem.addr + bits_to_skip / 8, > - buffer.data (), this_size); > - copy_bitwise (v_contents, offset, > - buffer.data (), bits_to_skip % 8, > - this_size_bits, bits_big_endian); > + read_from_memory (start_addr, (v_contents + offset / 8), > + this_size_bits / 8, in_stack_memory, > + &unavail); > } > else > { > - /* Write mode. */ > - if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0) > + this_size = bits_to_bytes (bits_to_skip, this_size_bits); > + buffer.resize (this_size); > + > + if (from == NULL) > { > - if (this_size <= 8) > + /* Read mode. */ > + read_from_memory (start_addr, buffer.data (), > + this_size, in_stack_memory, > + &unavail); > + if (!unavail) > + copy_bitwise (v_contents, offset, > + buffer.data (), bits_to_skip % 8, > + this_size_bits, bits_big_endian); > + } > + else > + { > + /* Write mode. */ > + if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0) > { > - /* Perform a single read for small sizes. */ > - read_memory (start_addr, buffer.data (), > - this_size); > + if (this_size <= 8) > + { > + /* Perform a single read for small sizes. */ > + read_from_memory (start_addr, buffer.data (), > + this_size, in_stack_memory, > + &unavail); > + } > + else > + { > + /* Only the first and last bytes can possibly have > + any bits reused. */ > + read_from_memory (start_addr, buffer.data (), > + 1, in_stack_memory, > + &unavail); > + if (!unavail) > + read_from_memory (start_addr + this_size - 1, > + &buffer[this_size - 1], 1, > + in_stack_memory, &unavail); > + } > } > - else > + > + if (!unavail) > { > - /* Only the first and last bytes can possibly have > - any bits reused. */ > - read_memory (start_addr, buffer.data (), 1); > - read_memory (start_addr + this_size - 1, > - &buffer[this_size - 1], 1); > + copy_bitwise (buffer.data (), bits_to_skip % 8, > + from_contents, offset, > + this_size_bits, bits_big_endian); > + write_to_memory (start_addr, buffer.data (), > + this_size, in_stack_memory, > + &unavail); > } > } > + } > > - copy_bitwise (buffer.data (), bits_to_skip % 8, > - from_contents, offset, > - this_size_bits, bits_big_endian); > - write_memory_with_notification (start_addr, > - buffer.data (), > - this_size); > + if (unavail) > + { > + if (from == NULL) > + mark_value_bits_unavailable (v, (offset + bits_to_skip % 8), > + this_size_bits); > + else > + throw_error (NOT_AVAILABLE_ERROR, > + _("Can't do read-modify-write to " > + "update bitfield; containing word " > + "is unavailable")); Did you get inspiration from existing code for this unavailable byte error handling? Were you able to test it? Simon