From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UM2LLgy3+WVvyQ0AWB0awg (envelope-from ) for ; Tue, 19 Mar 2024 12:02:20 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; secure) header.d=freebsd.org header.i=@freebsd.org header.a=rsa-sha256 header.s=dkim header.b=mMbsHe/l; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B92611E0BB; Tue, 19 Mar 2024 12:02:20 -0400 (EDT) Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 85C8E1E08C for ; Tue, 19 Mar 2024 12:02:18 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 45E433858CDB for ; Tue, 19 Mar 2024 16:02:18 +0000 (GMT) Received: from mx2.freebsd.org (mx2.freebsd.org [96.47.72.81]) by sourceware.org (Postfix) with ESMTPS id 2BE113858CD1 for ; Tue, 19 Mar 2024 16:01:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2BE113858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2BE113858CD1 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=96.47.72.81 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1710864113; cv=pass; b=TxBeK/HpWe6+dARfgkresd+SsEk7YwnMtM3W/xsSOAe2EwqWaINyO33GT1dDbU5TOyAwUDZwG+nZULvMRZW/OZvfWZJ6gKQGFP9iGGmnu76zipEyRGOO4t8DvwRPOMQsilxeHJzjz8O6fhw7j9EIgz1NK9o4WlHcfJ0axkpfvAM= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1710864113; c=relaxed/simple; bh=oAKfosmIeYE7vc6DU1fNeqncXC9PEAMmEtesYA7M7lI=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=DIPyD1IBo5m4qc7JLNSCFneqTykbealsTghcXEjcwZmgFK4GiG+HSSPZ1ueVI970pRNGjeUyEuFmmhYn+0/O4np6cXCkQr2TUB2fRNoECmnBmrP3EAHHFF2kFLZH5QC3Q9Lo7bn58LEqW3WLk21jFF13/0QQ4Q+MeWhLNw0j9rs= ARC-Authentication-Results: i=2; server2.sourceware.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 4Tzc0t6kCCz3wRh; Tue, 19 Mar 2024 16:01:50 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Tzc0t5mtPz4QQG; Tue, 19 Mar 2024 16:01:50 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710864110; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i2qgubJZW4a28Xxayc5rdVbmQSjqHzHS/tme8Rvjai8=; b=mMbsHe/lNwWq6iJ+qc7hadRD06l0vYFC4/UcOFZ/npZiLu1092MyiP8Hgz7HflQbh9vqgl xRUCqPDFehw5WR25jgYbOUSlG16TXXbVp73jqciiGI6bxYPRnBcALVYnHYhDf/HRwpLfjV c978GFsfNPXYjnmH81JxKB1PznNjKBeAHA0TZwmc7q1gWE+fYdCASbEFyb3+Z0fKXWRDXA dx4VwgEvVAq3Z4F+Exvk5T4iyb+HyMJJRAvf/wvC8RCyqCGXmhngHtbolhowrJKrf3OIUU JLj+2Z89tsm/Jcq/ubKny332hIxUKzBZDatfnX8MuDJwPzRQXhc8imzMxBASiA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1710864110; a=rsa-sha256; cv=none; b=UATPtZpoh8vBgNDTQK9nNGc25eNEIKmMmvi7CEU+JvXMn4ftBlSgtWvO5ptIq+Q47DXEqO iyuStOVmR2CLOaXvrKGB+nbV2fOH55qKS1rbh/AJ8iXc4oaN9fmnyif1WTtxvgOoI2Jfsk Bzpn6/cKiAF8Yyvf09PsSaegPpqYjay/mS0TtKqMEjQkefSfz4sF2K6czv5BOd1F/zWZt+ f0wzmAnPfHNis1IWKBQVhI0KkqnV9Gu6gm50OHYy2b9GVSwA87OrAXKB0/m5viZK0/db8l Z1hRpM5Ppuee6sm1A3h+1ryqR8kP1CDqrnY/uekHUm/tLmEBCVUi6ojibIaCHQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710864110; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i2qgubJZW4a28Xxayc5rdVbmQSjqHzHS/tme8Rvjai8=; b=oITh+fgLIK1B7IzpaCqtFPiozAnlWV9OcPl40utZTckTH7Kw4TE1KJZiFlUasxGbGRr9dX yCl3GBNtuG4BWdb/+M0q4NzFdmOJhX4UtoNjl0htgxWvI7LTmQ8GiU6QKwCskhbW0wefFi qvAbSiXFBhNNo3bh4IOvqGHSFkOf3H8V1iTWV4m5w4+Cfmvi9VfonPVf0368mDm7m2uIQm FwZFKoNonTUYOaDhsl4rdQ+9NSaDTBX1kLbNncd/rrN6ILipbua2gTyjhVbKNT3udEJS+W g2NB5+3OCgyEaY77jAUIH4Zh5clbSBDlgEGPSAKuHuoGKqT9ugz4eRxyXvqQVw== Received: from [IPV6:2601:644:937f:4c50:8511:5903:3902:6538] (unknown [IPv6:2601:644:937f:4c50:8511:5903:3902:6538]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Tzc0t3dQXz13wl; Tue, 19 Mar 2024 16:01:50 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Tue, 19 Mar 2024 09:01:48 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCHv2 6/7] gdbserver: update target description creation for x86/linux Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org References: From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org On 3/5/24 9:00 AM, Andrew Burgess wrote: > This commit is part of a series which aims to share more of the target > description creation between GDB and gdbserver for x86/Linux. > > After some refactoring, the previous commit actually started to share > some code, we added the shared x86_linux_tdesc_for_tid function into > nat/x86-linux-tdesc.c. However, this function still relies on > amd64_linux_read_description and i386_linux_read_description which are > implemented separately for both gdbserver and GDB. Given that at > their core, all these functions to is: > > 1. take an xcr0 value as input, > 2. mask out some feature bits, > 3. look for a cached pre-generated target description and return it > if found, > 4. if no cached target description is found then call either > amd64_create_target_description or > i386_create_target_description to create a new target > description, which is then added to the cache. Return the newly > created target description. > > The inner functions amd64_create_target_description and > i386_create_target_description are already shared between GDB and > gdbserver (in the gdb/arch/ directory), so the only thing that > the *_read_description functions really do is add the caching layer, > and it feels like this really could be shared. > > However, we have a small problem. > > On the GDB side we create target descriptions using a different set of > cpu features than on the gdbserver side! This means that for the > exact same target, we might get a different target description when > using native GDB vs using gdbserver. This surely feels like a > mistake, I would expect to get the same target description on each. > > The table below shows the number of possible different target > descriptions that we can create on the GDB side vs on the gdbserver > side for each target type: > > | GDB | gdbserver > ------|-----|---------- > i386 | 64 | 7 > amd64 | 32 | 7 > x32 | 16 | 7 > > So in theory, all I want to do is move the GDB version > of *_read_description into the nat/ directory and have gdbserver use > that, then both GDB and gdbserver would be able to create any of the > possible target descriptions. > > Unfortunately it's a little more complex than that due to the in > process agent (IPA). > > When the IPA is in use, gdbserver sends a target description index to > the IPA, and the IPA uses this to find the correct target description > to use. > > ** START OF AN ASIDE ** > > Back in the day I suspect this approach made perfect sense. However > since this commit: > > commit a8806230241d201f808d856eaae4d44088117b0c > Date: Thu Dec 7 17:07:01 2017 +0000 > > Initialize target description early in IPA > > I think passing the index is now more trouble than its worth. > > We used to pass the index, and then use that index to lookup which > target description to instantiate and use. However, the above commit > fixed an issue where we can't call malloc() within (certain parts of) > the IPA (apparently), so instead we now pre-compute _every_ possible > target description within the IPA. The index is now only used to > lookup which of the (many) pre-computed target descriptions to use. > > It would (I think) have been easier all around if the IPA just > self-inspected, figured out its own xcr0 value, and used that to > create the one target description that is required. So long as the > xcr0 to target description code is shared (at compile time) with > gdbserver, then we can be sure that the IPA will derive the same > target description as gdbserver, and we would avoid all this index > passing business, which has made this commit so very, very painful. > > ** END OF AN ASIDE ** > > Currently then for x86/linux, gdbserver sends a number between 0 and 7 > to the IPA, and the IPA uses this to create a target description. > > However, I am proposing that gdbserver should now create one of (up > to) 64 different target descriptions for i386, so this 0 to 7 index > isn't going to be good enough any more (amd64 and x32 have slightly > fewer possible target descriptions, but still more than 8, so the > problem is the same). > > For a while I wondered if I was going to have to try and find some > backward compatible solution for this mess. But after seeing how > lightly the IPA is actually documented, I wonder if it is not the case > that there is a tight coupling between a version of gdbserver and a > version of the IPA? At least I'm hoping so. > > In this commit I have thrown out the old IPA target description index > numbering scheme, and switched to a completely new numbering scheme. > Instead of the index that is passed being arbitrary, the index is > instead calculated from the set of cpu features that are present on > the target. Within the IPA we can then reverse this logic to recreate > the xcr0 value based on the index, and from the xcr0 value we can > create the correct target description. > > With the gdbserver to IPA numbering scheme issue resolved I have then > update the gdbserver versions of amd64_linux_read_description and > i386_linux_read_description so that they create target descriptions > using the same set of cpu features as GDB itself. > > After this gdbserver should now always come up with the same target > description as GDB does on any x86/Linux target. > > This commit does not introduce any new code sharing between GDB and > gdbserver as previous commits in this series does. Instead this > commit is all about bringing GDB and gdbserver into alignment > functionally so that the next commit can merge the GDB and gdbserver > versions of these functions. It does seem like the IPA case should just compute the tdesc it needs if I understand you correctly. BTW, on other arches like aarch64 and I believe risc-v, a flat array is not used for the tdesc cache. Now the cache is a std::unordered_map<> and there is a 'struct foo_features' that has an operator== and std::hash<> specialization. It really would be nice if x86 was the same, but that would require abandoning a notion of a portable "index" shared between gdbserver and the IPA. -- John Baldwin