From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2610:1c1:1:606c::19:2]) by sourceware.org (Postfix) with ESMTPS id 23DD23858D28 for ; Tue, 11 Apr 2023 21:37:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 23DD23858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (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 4PwzhC6TQTz4DmX; Tue, 11 Apr 2023 21:37:15 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 4PwzhC5YP6z3n42; Tue, 11 Apr 2023 21:37:15 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681249035; 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=AN0F7aCq0gv7VRPsPYzFPOtEPlcH6JCFiLYTbuoG8m0=; b=a1ztkIMLzt3nLk38wqv0dtDN6Rd98tF9JxXruN6zFXdyODoVsv9963ZVHWF6nYhNa+gQvO 0ej6nQCRHsHlNif4JNN/UHd1J0L9Jh3CPw1c/R1+X7zTcQQJ217wFP43pxzciISvnasyir ISrP1EjiJGtWtscp0nymVJwzMMin2C1a3PmerTqXq2ZnZXIJYpuIugj0yaVM2gik9ehAZU OmucUH4cUumZ/uTJy6dmGdGfj0aVKbmFWmePWJ+B9szSi2A8djTruaYKNbqp7vtDZ24n72 lQvhmTAXR6Kx1zKc8Q0CtNi5WrsA42qSdLuW98VG7gUjBp/mJZTQqBoxNv9AeA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681249035; 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=AN0F7aCq0gv7VRPsPYzFPOtEPlcH6JCFiLYTbuoG8m0=; b=ESb637wcEHb3onM8E+TM2T8O3ZvCxzMS1DylR3+3ZNU4iDT7rYHEx94OK8i0vlzZJ7Qjxi +7iaFC0r7fPUwAIBNY6MsrrZjRh9k2Bi/55PEQtSE5+wYwy3/R7kg9rHwNgjUaUIInub/2 Rq795+MeHfo425wX4v0TBTV2rm6PBQKL5YDu2h2bodtCk2qw90qxexNjIqyMHCtV+sDV1C zEEa1W8WLC0eC8thUBf/2Rk9N+uCHAFehwiJKLTeacK2nN5+J8Ev6JZDI7+0ePBqNzBV4y s/03U69iRjPf1VjaSownoc3szaEGYmGtJPJ7nZVaNnxDfjU0fiow4mQwTMt4YA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1681249035; a=rsa-sha256; cv=none; b=nfOLrY+2/a3q4aQ3L/+TIhJx+Co3IRfMIYzLVDNpS2Xkc13/RugLxEJPJTvMHA0aEnIPRu FStf4eWd6pFen6/w5Jws8t9q45m1wNB5CmSMS+3hlYCCYyCJxLL/1JoS7V1ta79mvz2Srm 1ulaGu005PoV/vWfmI7I/rTZlX9IGADgjRLq+4Wx35QXN9BWOcLLSGfWsySVu/5L0tkj5z xVWo9iJfSH5h3bgkAlotyczyeUSWiYW6z5UBqe2weiwxrJOd4xid/HxrR9ROqFataRpgxT maHI8cFmrOskcxkNy4PzS+7gXAQPLds0Pd0EDXgEAMN2Lop8ilwzt9FpDQIR9Q== Received: from [IPV6:2601:648:8680:16b0:f4ff:5acc:9dc0:ebcf] (unknown [IPv6:2601:648:8680:16b0:f4ff:5acc:9dc0:ebcf]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4PwzhC2ZJxz19Lk; Tue, 11 Apr 2023 21:37:14 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Tue, 11 Apr 2023 14:37:14 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org References: <20230318010905.14294-1-jhb@FreeBSD.org> <20230318010905.14294-7-jhb@FreeBSD.org> <448537fa-fff2-4bf9-e01b-1c01e5cb48dd@simark.ca> <198191af-c026-9e6a-767c-3418e8174959@simark.ca> <02d796f2-de80-b3c4-8279-38c750286446@FreeBSD.org> From: John Baldwin Subject: Re: [PATCH v4 06/13] gdb: Support XSAVE layouts for the current host in the FreeBSD x86 targets. In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_NONE,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 4/11/23 9:46 AM, Simon Marchi wrote: > On 4/11/23 12:19, John Baldwin wrote: >> On 4/10/23 7:23 PM, Simon Marchi wrote: >>> On 4/10/23 17:27, John Baldwin wrote: >>>> On 4/6/23 1:18 PM, Simon Marchi wrote: >>>>>> @@ -43,3 +46,51 @@ x86_fbsd_nat_target::low_new_fork (ptid_t parent, pid_t child) >>>>>> child_state = x86_debug_reg_state (child); >>>>>> *child_state = *parent_state; >>>>>> } >>>>>> + >>>>>> +#ifdef PT_GETXSTATE_INFO >>>>>> +/* Implement the "xfer_partial" target_ops method. */ >>>>>> + >>>>>> +enum target_xfer_status >>>>>> +x86_fbsd_nat_target::xfer_partial (enum target_object object, >>>>>> + const char *annex, gdb_byte *readbuf, >>>>>> + const gdb_byte *writebuf, >>>>>> + ULONGEST offset, ULONGEST len, >>>>>> + ULONGEST *xfered_len) >>>>>> +{ >>>>>> + switch (object) >>>>>> + { >>>>>> + case TARGET_OBJECT_X86_XSAVE_LAYOUT: >>>>>> + if (xsave_layout.sizeof_xsave == 0) >>>>>> + return TARGET_XFER_E_IO; >>>>> >>>>> I'm wondering why you don't call probe_xsave_layout before using >>>>> xsave_layout here. I guess because you assume that the child class will >>>>> have called probe_xsave_layout at some point before? >>>> >>>> Yes. Note that the default value of xsave_layout (all zeroes) will result >>>> in this failing to read the object if it hasn't been called (rather than >>>> undefined behavior). >>>> >>>>> To make that more robust, could we access xsave_layout through a method >>>>> like: >>>>> >>>>> const x86_xsave_layout &xsave_layout (); >>>>> >>>>> which would take care of the caching? Callers wouldn't have to worry >>>>> about calling probe_xsave_layout before. There could be another method >>>>> to access xsave_info. >>>> >>>> Well, part of the trick here is that probe_xsave_layout takes a pid >>>> argument that it passes to ptrace () to fetch the layout, but at the >>>> point of xfer_partial I don't necessarily have a pid. It's true that >>>> the only callers (the target ::read_description methods) all use >>>> inferior_ptid's pid, so I could perhaps use inferior_ptid directly, >>>> but that seems somewhat dubious. >>> >>> Using inferior_ptid would probably be fine... I think? It's how targets >>> know which thread's memory to read from or write to, for the >>> TARGET_OBJECT_MEMORY object. The question is, will all calls to >>> xfer_partial with the TARGET_OBJECT_X86_XSAVE_LAYOUT object be done with >>> inferior_ptid set to a stopped thread that you can used to do the ptrace >>> query? >>> >>> Ok, that made me think about an unlikely (but still possible >>> scenario) where the read_description target method can be called with >>> inferior_ptid == null_ptid: >>> >>> (gdb) target native # push the native target >>> (gdb) unset tdesc filename >>> >>> I think the code as it is in this patch would try to call ptrace with >>> pid 0. >>> >>> The approach of fetching the xsave layout using ptrace has this >>> particularity that you need a stopped thread to access it. I'm >>> wondering, is there an advantage in fetching the xsave layout using >>> ptrace instead of the cpuid method? The latter has the advantage of >>> working at any time. >> >> Well, you need the value of xcr0 as well as the total size, though it >> is possible to execute XGETBV in userspace I believe. I had wanted to >> ensure we were using the same mask ptrace is going to export vs assuming >> that the mask of the running GDB process will match the mask of all >> target processes. I guess in practice we are assuming that though via >> the caching. > > Who sets xcr0, the kernel or the userspace? In theory could we have > xcr0 values that differ per process? Could the xcr0 value for a process > change over time? To be clear, I'm just asking to understand better, I > don't suggest to support these cases if they don't happen in reality. Only the kernel sets it because the kernel is the one that allocates space for the XSAVE area for each thread to save/restore state during context switches. Also, the relevant instruction (XSETBV) is kernel only. Possibly a kernel could choose to alter it between processes, but if that were the case that would be an argument to not do any caching and always check it in read_description() for the relevant inferior. Neither Linux nor FreeBSD do this though, both determine a value to use at boot and then never change it. (Well, not in a way relevant to GDB, FreeBSD can choose to use a smaller mask for guest VMs while the guest is running, but that value of XCR0 is never visible to GDB, and even if GDB is debugging the userspace portion of the hypervisor it always sees the "host" values of XSAVE registers, not guest values.) >> Note that the read_description methods for both Linux and >> FreeBSD already use ptrace for other things. Both use ptrace to decide >> if the inferior is using a 64-bit vs 32-bit tdesc. For 32-bit both use >> ptrace to determine if there is support for a custom ptrace op to fetch >> SSE/XMM register state (PTRACE_GETFPXREGS on Linux, PT_GETXMMREGS on >> FreeBSD), so this case is already quite broken. > > Heh, you're right, I ran those commands on x86 Linux and didn't even > notice the error message! > > (gdb) target native > Done. Use the "run" command to start a process. > (gdb) unset tdesc filename > Couldn't get CS register: No such process. > > So yeah, perhaps make sure this corner case is relatively correctly > handled. That probably means skipping over the ptrace check if > inferior_ptid is null_ptid, and make sure you don't cache that value (so > we actually probe it next time around). I think the right answer for theses cases is that the read_description methods need to just return a specific base-line description if inferior_ptid == null_ptid before doing any of the ptrace, etc. checks. Probably for 64-bit it means a simple 64-bit tdesc with SSE for example. -- John Baldwin