From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id DFCFB3858C50 for ; Thu, 1 Dec 2022 16:16:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DFCFB3858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 5B16B1E0D3; Thu, 1 Dec 2022 11:16:44 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1669911404; bh=1CkAbExisyOQup3v2WLSmSrXhcv6uc3F8hwW++lqN3k=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=uCdoKcDXA2SIMeIIwX8Vu08SxhtK7rTWB4VOxs4W9tlNhcyITEG9YCukw/ZQ5JixF eLday1wxPy7ZhhG+mJ1XVkfigUmJKQATKZGBd8NqLJU20KDeMLWSlH9bUjXOLQIiI1 mIJDsi2MKgKaJUaupQrHHulXyQsBzOfglQqS/6ww= Message-ID: <77a4062a-6a8b-151b-9b7f-6b717eb0a082@simark.ca> Date: Thu, 1 Dec 2022 11:16:43 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH v2 6/6] gdb/aarch64: Detect vector length changes when debugging remotely Content-Language: fr To: Thiago Jung Bauermann Cc: gdb-patches@sourceware.org, Luis Machado References: <20221126020452.1686509-1-thiago.bauermann@linaro.org> <20221126020452.1686509-7-thiago.bauermann@linaro.org> <5d3ca3ed-df52-444e-1652-99b149137707@simark.ca> <87r0xjg6e4.fsf@linaro.org> From: Simon Marchi In-Reply-To: <87r0xjg6e4.fsf@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,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: On 11/30/22 22:16, Thiago Jung Bauermann via Gdb-patches wrote: > > Simon Marchi writes: > >> On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote: >>> If the remote target provides an expedited VG register, use it to update >>> the thread's gdbarch. This allows debugging programs that change their SVE >>> vector length during runtime. >>> >>> This is accomplished by implementing the thread_architecture method in >>> remote_target, which returns the gdbarch corresponding to the expedited >>> registers provided by the last stop reply. >>> >>> To allow changing the architecture based on the expedited registers, add a >>> new gdbarch method to allow arch-specific code to make the adjustment. >> >> I get this when applying, can you address that? >> >> Applying: gdb/aarch64: Detect vector length changes when debugging remotely >> .git/rebase-apply/patch:164: indent with spaces. >> "gdbarch_dump: update_architecture = <%s>\n", >> .git/rebase-apply/patch:165: indent with spaces. >> host_address_to_string (gdbarch->update_architecture)); >> .git/rebase-apply/patch:186: indent with spaces. >> gdbarch_update_architecture_ftype update_architecture) >> warning: 3 lines add whitespace errors. > > This is because gdbarch.py generates these lines in gdbarch.c with > spaces-only indentation. I will send a separate patch to fix it. Ah, I did not realize it was in a generated file. Well, if you can fix the generator, that's nice, but otherwise not a big deal. >> can't read the vq register using 'g', because to interpret the 'g' >> result, we need to know the full architecture. We wouldn't know for >> sure the offset of vq in the reply. By using expedited registers, we >> just need to make sure vq's register number is stable. I guess that is >> the case? > > The register number is determined by the function > aarch64_create_target_description. If SVE is supported, VG will always > have the register number 85. If not, then AFAIU the register number will > be unused (since the PAuth, MTE and TLS features only have a few > registers). But it could potentially be used by another feature in the > future... > > Do we have to reserve the number 85 for the VG pseudo-register? Given that aarch64_update_architecture looks for register 85 (AARCH64_SVE_VG_REGNUM) in the expedited registers without prior knowledge of whether there is really a VG register, I can imagine bad things happening if another unrelated register is using number 85. At this point, do we have access to the inferior's target description? Can we maybe check if register 85 is indeed VG, and bail out otherwise? > >>> @@ -8068,6 +8078,21 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply, >>> /* Expedited registers. */ >>> if (!stop_reply->regcache.empty ()) >>> { >>> + /* If GDB already knows about this thread, we can give the >>> + architecture-specific code a chance to update the gdbarch based on >>> + the expedited registers. */ >>> + if (find_thread_ptid (this, ptid) != nullptr) >>> + { >>> + stop_reply->arch = gdbarch_update_architecture (stop_reply->arch, >>> + stop_reply->regcache); >>> + >>> + /* Save stop_reply->arch so that it can be returned by the >>> + thread_architecture method. */ >>> + remote_thread_info *remote_thr = get_remote_thread_info (this, >>> + ptid); >>> + remote_thr->expedited_arch = stop_reply->arch; >>> + } >>> + >>> struct regcache *regcache >>> = get_thread_arch_regcache (this, ptid, stop_reply->arch); >> >> >> What happens with threads that are not yet known? Imagine the process >> starts with SVE == X, the main threads spawns a worker thread, the >> worker thread updates its SVE to Y, and the worker thread hits a >> breakpoint. This is the first time we hear about that worker thread. > > I tested this scenario and... it sort of works, but not quite. When the > unknown thread hits the breakpoint, gdbserver sends a T stop reply > packet with the new VG value in the expedited registers. It will be the > first time we hear about that worker thread, but at the same time we > will also learn about the value of its VG pseudo-register. So GDB > reports the breakpoint hit event and gets to the prompt as expected. You > can even print the new value of the VG pseudo-register, or any other > expedited register such as PC or SP. > > It's only when you try to print the value of a non-expedited register > that you get an error indicating that GDB and gdbserver don't agree on > the size of the SVE registers (“Truncated register 51 in remote 'g' > packet”). > >> If we don't do a "gdbarch_update_architecture" for it and save it >> somewhere, we'll never set the gdbarch with SVE == Y, will we? > > We do that in remote_target::process_stop_reply, and I was under the > impression that it happened early enough for GDB to have the correct > gdbarch available when it needs it, but unfortunately that's not the > case. > > I will add a testcase exercising this issue and see if I can find a > solution. Thank you for spotting it. I think that the threads (if new) are added just after that, in the remote_notice_new_inferior call. Maybe you can reorder things to do remote_notice_new_inferior first and your code + the get_thread_arch_regcache call after. Simon