From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 9wHDIjjxHGBPGAAAWB0awg (envelope-from ) for ; Fri, 05 Feb 2021 02:18:16 -0500 Received: by simark.ca (Postfix, from userid 112) id 722DE1EFCB; Fri, 5 Feb 2021 02:18:16 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.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 ECEA51E590 for ; Fri, 5 Feb 2021 02:18:13 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 17CD439E4C21; Fri, 5 Feb 2021 07:18:13 +0000 (GMT) Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id 65A59394443E for ; Fri, 5 Feb 2021 07:18:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 65A59394443E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=palmer@dabbelt.com Received: by mail-pl1-x633.google.com with SMTP id e12so3112109pls.4 for ; Thu, 04 Feb 2021 23:18:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20150623.gappssmtp.com; s=20150623; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=pw/OYG5XeZjmFWeL+RDv0OLfT5gKofhu/u9FxtXcoZg=; b=F+W+qqc3/aNXI52NsL75n+r4YNoGVG+U4XRESANPCb5AMTZH3+pUpsytCAxpSk2Bz6 hyhp6DzvFwZDxiSpgkwqDWCozSW2scDAYmkZqB8z7NSoesaz9Yauqsc2UyeBGv5b3hHn FnIla8AXi4zmwKVUbe06QwG/rM+z92+TDp8sqgmOtqfAkOSLWejCY3IyqYj/488OxCXp EFqkZMJvUm/Kjo0E60DI46Nd9Lv+r/vxe5l6Exk7LpKCh6coHcHz/If50kFjq73CHeTe L+ZoaRzAryO5sUnR8IovCDQZqQivuhSSqEpQlcf5QI6KIlT+yjC88A2CyFy7SyzIW9rI qbtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=pw/OYG5XeZjmFWeL+RDv0OLfT5gKofhu/u9FxtXcoZg=; b=DrI3jqo/D5fdnAtDx5+i/DSqYAdRtIUxGwp4mQ+RhfBnZb/RAR7Tc/+CJ2xRs+iRxN NQsh66oDZ2qiJ56ZQ5Gn5yIVlhUTXRihuOnBwy13045iOwA2qovkXrN/d+ms+tu5Kw5H bMfW+offnDo7X2cD1ocoIza0ecP8BNSVobdbMklzFnxiT49aEvGm2r2fByN9uy8kl8mi yR3i6DW2DoXBBDHt8I0sWGMf8MJhrUrhJMMJZsBQH4fe2eytuNwtBp1dcwg1RreDig6l wzMzthAGjWQljUWtCOjUqxXUq+X53E8t/P6hX6KsOH1g3XhDfSalN/b7u2zQzpQVmpvQ szfg== X-Gm-Message-State: AOAM532GyoOOapkF0hY1cY/fAzcggPhJuu5eZLEo9AGPb5/y0iCCCO3E H6y6gwprwrrfbgzMDc2hPmv4h4UWDqdHnr9n X-Google-Smtp-Source: ABdhPJyoeze23uNVXjQo54h2zGUSKKwFy6ytPF28MGLvaCWuPwqiYkroxqHzyuZzrGR7YrIwsymG6g== X-Received: by 2002:a17:90a:5303:: with SMTP id x3mr2914582pjh.54.1612509488986; Thu, 04 Feb 2021 23:18:08 -0800 (PST) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id h5sm9074940pgl.86.2021.02.04.23.18.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Feb 2021 23:18:08 -0800 (PST) Date: Thu, 04 Feb 2021 23:18:08 -0800 (PST) X-Google-Original-Date: Thu, 04 Feb 2021 23:12:30 PST (-0800) Subject: Re: [PATCH] gdb/riscv: select rv32 target by default when requested In-Reply-To: <20210204204426.608948-1-andrew.burgess@embecosm.com> From: Palmer Dabbelt To: andrew.burgess@embecosm.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 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: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On Thu, 04 Feb 2021 12:44:26 PST (-0800), andrew.burgess@embecosm.com wrote: > GDB for RISC-V always uses target descriptions. When the target > doesn't provide a target description then a default is selected. > Usually this default is selected based on the properties of the > executable being debugged. However, when there is no executable being > debugged we currently fallback to the riscv:rv64 target description as > the default. This leads to strange behaviour like this: > > $ gdb > (gdb) set architecture riscv:rv32 > (gdb) p sizeof ($pc) > $1 = 8 > > Despite the users specifically setting the architecture to riscv:rv32 > GDB still thinks that the target has riscv:rv64 register sizes. > > The above is a bit of a contrived situation. I actually ran into this I don't think it's that contrived, it used to happen to me every day ;). I just didn't know how to fix it. Thanks! > situation while trying to connect to a running riscv:rv32 target > without supplying an executable (the target didn't provide a target > description). When I tried to set a register on the target I ran into > errors because GDB was passing 8 bytes to the target rather than the > expected 4. Even when I manually specified the architecture (as > above) I couldn't convince GDB to only send 4 bytes. > > This patch fixes this issue. Now, when we selected a default target > description we will make use of the user selected architecture to > guide our choice. In the above example we now get: > > $ gdb > (gdb) set architecture riscv:rv32 > (gdb) p sizeof ($pc) > $1 = 4 > > And my real world example of connecting to a remote without an > executable works fine. > > I've used the fact that we can ask GDB about $pc even when no > executable is loaded as the basis for a test to cover this situation. > > gdb/ChangeLog: > > * riscv-tdep.c (riscv_features_from_gdbarch_info): Rename to... > (riscv_features_from_bfd): ...this. Change parameter type to > 'bfd*', and update as required. > (riscv_find_default_target_description): Update call to > riscv_features_from_bfd. Select a default xlen based on > info.bfd_arch_info. > (riscv_gdbarch_init): Update call to riscv_features_from_bfd. > > gdb/testsuite/ChangeLog: > > * gdb.arch/riscv-default-tdesc.exp: New file. > --- > gdb/ChangeLog | 10 ++++ > gdb/riscv-tdep.c | 28 ++++----- > gdb/testsuite/ChangeLog | 4 ++ > .../gdb.arch/riscv-default-tdesc.exp | 59 +++++++++++++++++++ > 4 files changed, 86 insertions(+), 15 deletions(-) > create mode 100644 gdb/testsuite/gdb.arch/riscv-default-tdesc.exp > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 460746a9bfe..2a7b792893d 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -3215,13 +3215,11 @@ static const struct frame_unwind riscv_frame_unwind = > /*.prev_arch =*/ NULL, > }; > > -/* Extract a set of required target features out of INFO, specifically the > - bfd being executed is examined to see what target features it requires. > - IF there is no current bfd, or the bfd doesn't indicate any useful > - features then a RISCV_GDBARCH_FEATURES is returned in its default state. */ > +/* Extract a set of required target features out of ABFD. If ABFD is > + nullptr then a RISCV_GDBARCH_FEATURES is returned in its default state. */ > > static struct riscv_gdbarch_features > -riscv_features_from_gdbarch_info (const struct gdbarch_info info) > +riscv_features_from_bfd (const bfd *abfd) > { > struct riscv_gdbarch_features features; > > @@ -3231,11 +3229,10 @@ riscv_features_from_gdbarch_info (const struct gdbarch_info info) > only used at all if the target hasn't given us a description, so this > is really a last ditched effort to do something sane before giving > up. */ > - if (info.abfd != NULL > - && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour) > + if (abfd != nullptr && bfd_get_flavour (abfd) == bfd_target_elf_flavour) > { > - unsigned char eclass = elf_elfheader (info.abfd)->e_ident[EI_CLASS]; > - int e_flags = elf_elfheader (info.abfd)->e_flags; > + unsigned char eclass = elf_elfheader (abfd)->e_ident[EI_CLASS]; > + int e_flags = elf_elfheader (abfd)->e_flags; > > if (eclass == ELFCLASS32) > features.xlen = 4; > @@ -3273,13 +3270,14 @@ riscv_find_default_target_description (const struct gdbarch_info info) > { > /* Extract desired feature set from INFO. */ > struct riscv_gdbarch_features features > - = riscv_features_from_gdbarch_info (info); > + = riscv_features_from_bfd (info.abfd); > > - /* If the XLEN field is still 0 then we got nothing useful from INFO. In > - this case we fall back to a minimal useful target, 8-byte x-registers, > - with no floating point. */ > + /* If the XLEN field is still 0 then we got nothing useful from INFO.BFD, > + maybe there was no bfd object. In this case we fall back to a minimal > + useful target with no floating point, the x-register size is selected > + based on the architecture from INFO. */ > if (features.xlen == 0) > - features.xlen = 8; > + features.xlen = info.bfd_arch_info->bits_per_word == 32 ? 4 : 8; > > /* Now build a target description based on the feature set. */ > return riscv_lookup_target_description (features); > @@ -3497,7 +3495,7 @@ riscv_gdbarch_init (struct gdbarch_info info, > target, then check that this matches with what the target is > providing. */ > struct riscv_gdbarch_features abi_features > - = riscv_features_from_gdbarch_info (info); > + = riscv_features_from_bfd (info.abfd); > > /* If the ABI_FEATURES xlen is 0 then this indicates we got no useful abi > features from the INFO object. In this case we just treat the > diff --git a/gdb/testsuite/gdb.arch/riscv-default-tdesc.exp b/gdb/testsuite/gdb.arch/riscv-default-tdesc.exp > new file mode 100644 > index 00000000000..2b7e8fa63e8 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/riscv-default-tdesc.exp > @@ -0,0 +1,59 @@ > +# Copyright 2021 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Check the size of the $pc register as the user changes the selected > +# architecture. As no executable is provided then the size of the $pc > +# register is defined by the default target description selected by > +# GDB. > +# > +# This test ensures that GDB is selecting an RV32 default if the user > +# has forced the current architecture to be riscv:rv32. > +# > +# In all other cases the default will be RV64. > + > +if {![istarget "riscv*-*-*"]} { > + verbose "Skipping ${gdb_test_file_name}." > + return > +} > + > +# Start GDB with no executable. > +gdb_start > + > +# The tests are defined by a list of architecture names to switch too > +# and the expected size of $pc. The first list entry is special and > +# has an empty architecture string, this checks GDB's default value on > +# startup. > +foreach data {{"" 8} {"riscv:rv32" 4} {"riscv:rv64" 8} {"riscv" 8} \ > + {"auto" 8}} { > + set arch [lindex $data 0] > + set size [lindex $data 1] > + > + # Switch architecture. > + if { $arch != "" && $arch != "auto" } { > + gdb_test "set architecture $arch" \ > + "The target architecture is set to \"$arch\"\\." > + } elseif { $arch == "auto" } { > + gdb_test "set architecture $arch" \ > + "The target architecture is set to \"auto\" \\(currently \"riscv\"\\)\\." > + } else { > + set arch "default architecture" > + } > + > + # Check the size of $pc. > + with_test_prefix "$arch" { > + gdb_test "p sizeof (\$pc)" " = $size" \ > + "size of \$pc register is $size" > + } > +}