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 506AD3858D28 for ; Mon, 11 Sep 2023 10:52:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 506AD3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694429532; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=1lFfc/IJ0bPJKZ9aAhzvIdxcj/i66jSmxqCMbhLlCnc=; b=f0y+RAt8X5Qqq7ItfVygdHDEaJJix24S92SZEs2uMXLVJobkWi/GWN9hLuyVkZrRa1FC3Y MjNpNyZVpP4exWx2ayYxB2yMnDH8zR2o/dvpCvQtmfUpB3t1sJ0IKhLbbs/0ScR6U7iSU8 cWNIhemFsic/whUdyeg+DIBPGDg3F1M= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-34-8Fa3AtOyNcya1_EYNuF-nQ-1; Mon, 11 Sep 2023 06:52:10 -0400 X-MC-Unique: 8Fa3AtOyNcya1_EYNuF-nQ-1 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-3f5df65f9f4so32062615e9.2 for ; Mon, 11 Sep 2023 03:52:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694429530; x=1695034330; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1lFfc/IJ0bPJKZ9aAhzvIdxcj/i66jSmxqCMbhLlCnc=; b=nKQT1J5unLLUVa87dM/5q3U+/HgaVJE1xiFACknradIWD83qgzvlZzt+rwwKkVBTSs uhkz9bP+dErt9eE1Ba+eDAt4O3VF9LHLHYx3klL2QE6wzRl/zKVZ+IwskHaXtreOTgjv XvGJQ5Frs+t0rPCVpXX1ENQgbCAY+91wUrJqVY1zNij7bBCZ0N4ZoK37lYrBRAr2rUVu Ss/8GZKu1cnXQaif3N+OGpnnGXNXC8kzn2GE6SYRO5AgcmaHCRtQjYh8+p182DYyGiqP 1Bo7nt8LoA3MKwCzaO1zkAXk+hI7elr1Vs1Uu4rZXtvxQUIOtOYLJZPKd9YZvLwyij4a LKwA== X-Gm-Message-State: AOJu0YyYXT5e60kYy9eeWHQIXYJUbNxVZfxPl/uUUcT0LDJpWXAc2Lyx JgwqJZ+3LNMmFYuswo5SppSDkQQtyssq7sopMXwKlO33osxWhQcsxhFHipD7XoZTKCOKoRYvbPy mIc98HfiSx7mdsLWZqso0xw== X-Received: by 2002:adf:f28e:0:b0:317:5d3d:c9df with SMTP id k14-20020adff28e000000b003175d3dc9dfmr7607264wro.18.1694429529811; Mon, 11 Sep 2023 03:52:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHKuYST0y6pGuBpb2a9FGc6B5ssebufswbA0mVbfzkSA/LF+mmq+Okx7YPi0og3mwvD4Noynw== X-Received: by 2002:adf:f28e:0:b0:317:5d3d:c9df with SMTP id k14-20020adff28e000000b003175d3dc9dfmr7607254wro.18.1694429529409; Mon, 11 Sep 2023 03:52:09 -0700 (PDT) Received: from localhost (197.126.90.146.dyn.plus.net. [146.90.126.197]) by smtp.gmail.com with ESMTPSA id s17-20020adff811000000b0030ada01ca78sm9662364wrp.10.2023.09.11.03.52.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Sep 2023 03:52:08 -0700 (PDT) From: Andrew Burgess To: Ciaran Woodward , gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/tdesc: Don't assign custom-group tdesc registers to 'general' In-Reply-To: <20230907170752.28639-1-ciaranwoodward@xmos.com> References: <20230907170752.28639-1-ciaranwoodward@xmos.com> Date: Mon, 11 Sep 2023 11:52:08 +0100 Message-ID: <87fs3lkmnb.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.6 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 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: Ciaran Woodward writes: > The 'Target Description' mechanism in GDB enables the target to > supply the full set of registers available on the system to gdb > in an XML format. > > This format enables setting the 'group' of each register, such > that they can be queried using the 'info registers ' > mechanism. > > However prior to this change, even if a register was explicitly > assigned to a group, it would still show up in the > 'info registers general' report. This is unexpected, and also > disagrees with the comment above the tdesc_register_in_reggroup_p > function, which says that '-1' should be returned if the register > group is not-known, not the register group is known, but differs. > > There was a previous change that did address this issue in > aa66aac47b4dd38f9524ddb5546c08cc09930d37 > but it also caused registers with *no* group in the target > description to be removed from 'general', so it was reverted in > 440cf44eb0f70830b8d8ac35289f84129c7a35c1 > as that behaviour was used by some targets. > > The change in this commit enhances the usefulness of the tdesc > 'group' attribute for adding system configuration registers, > of which there may be hundreds - very inconvenient to request > and print on every 'info registers' call. > --- > > Email archive link to discussion of previously referenced patch: > https://inbox.sourceware.org/gdb-patches/20200120155315.30333-1-shahab.vahedi@gmail.com/T/#m0ce2983153b2f482b66461ed6b97c4b287b09a89 > > gdb/target-descriptions.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index cdedf88c793..c1bb03c3adf 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -954,14 +954,13 @@ tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno, > { > struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno); > > - if (reg != NULL && !reg->group.empty () > - && (reg->group == reggroup->name ())) > - return 1; > - > if (reg != NULL > && (reggroup == save_reggroup || reggroup == restore_reggroup)) > return reg->save_restore; > > + if (reg != NULL && !reg->group.empty ()) > + return (reg->group == reggroup->name ()); > + > return -1; > } I haven't tested this, but looking at the series you referenced, I don't think the above change can be correct. If reggroup is all_reggroup, and a register has a group set, then this is going to return 0, right? If we then look at tdesc_register_reggroup_p, which calls the above function, we can see that it relies on the function you changed to return -1 in this case, at which point we fall through to default_register_reggroup_p, where all_reggroup is handled. Of course, this all depends on which target you're using. Some targets implement their own set_gdbarch_register_reggroup_p callback, and handle all_reggroup before calling tdesc_register_in_reggroup_p, but others (e.g. MIPS) don't do it this way, so I think your change will break them. Thanks, Andrew