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 F1B123857C53 for ; Mon, 27 Nov 2023 19:30:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F1B123857C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org F1B123857C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701113437; cv=none; b=tDZXje0VL1RjYUP3hzFgMQSYoXLgW0/lVH5mCsW/iPt4xaiOnKV5NJjDpE12R5o3tnsiSdi8fdqelMtxyeGihM53JoZoRa9nPK0gjaaDMZ0eh1NmHw6yi86Vpr7ky9lH+Vw3kUYrcN75I4QdWUATqm2NFiEDoZNzY5MMhawjIAw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701113437; c=relaxed/simple; bh=HTmkv/NvQIBaPeFLYhe4X6kkr1tWJ/Uv3gRfsVPchoE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=F34LrrWzOkXVhZ4O4wstijxu37Fe26CNB1g/tgEExxavTLxmPcfxn6+ug6fch8mHqijct03uXIPWCLq2LVhluCy+AnTAfo4+aMj/IqgC6eWckQRyr8EGeO5eGtFtZN2uHSn93h5qkIJqNt8NzdOCPg/IkTOYvnVAurJGsYlAahw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1701113435; bh=HTmkv/NvQIBaPeFLYhe4X6kkr1tWJ/Uv3gRfsVPchoE=; h=Date:Subject:To:References:From:In-Reply-To:From; b=GoNMBanWSyS2u9qHavu7oUuvDjWXLoNxyJstzV8ROy7hKFGCLSIx4EZwJWyHP8mPW J5GmrOb4S/cmmyO4a2J6nQxuihfU0q32l5MnlyWIms8iSMb4pQlS1OZreV+B4FL15B 8KyjgToI+HSZo10i7slO+CTFOAGMnDq4sgX/zVKU= Received: from [172.16.0.146] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 166A51E091; Mon, 27 Nov 2023 14:30:34 -0500 (EST) Message-ID: <011cfdf6-2c58-4cbd-a1e9-f3102345232b@simark.ca> Date: Mon, 27 Nov 2023 14:30:34 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] i386: Use a fallback XSAVE layout for remote targets Content-Language: fr To: John Baldwin , gdb-patches@sourceware.org References: <20231121220931.48497-1-jhb@FreeBSD.org> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT 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/21/23 17:22, John Baldwin wrote: > On 11/21/23 2:09 PM, John Baldwin wrote: >> If a target provides a target description including registers from the >> XSAVE extended region, but does not provide an XSAVE layout, use a >> fallback XSAVE layout based on the included registers. This fallback >> layout matches GDB's behavior in earlier releases which assumes the >> layout from Intel CPUs. >> >> This fallback layout is currently only used for remote targets since >> native targets which support XSAVE provide an explicit layout derived >> from CPUID. >> >> PR gdb/30912 >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30912 > > This fixes the regression for me locally. It would perhaps be cleaner > to isolate the regression to the remote target by moving the new logic > in i386_gdbarch_init into remote::fetch_x86_xsave_layout. The one > caveat of that is getting access to the right tdesc. I could perhaps > change target::fetch_x86_xsave_layout to pass down the tdesc pointer > (or maybe the gdbarch_info?). Alternatively it might be sufficient to > just use target_current_description() as the tdesc in remote.c? It > wasn't clear to me if the relevant tdesc in this case was always the > same as target_current_description() (though I suspect it probably is). My understanding is that the fallback is used for targets that can debug x86, where xcr0 may claim that the inferior has extended state registers, but where the target is unable to provide an xsave layout. There probably won't ever be another target than the remote target in this situation, so if you can make the fix local to the remote target that would be nice. But really I would also be fine with what you have. I think that using target_current_description in remote_target::fetch_x86_xsave_layout would be fine. When connecting to a remote target, fetch_x86_xsave_layout would be called here: (top-gdb) bt #0 0x0000555562d27c3a in target_ops::fetch_x86_xsave_layout (this=0x61b000044480) at /home/smarchi/src/binutils-gdb/gdb/target-delegates.c:4568 #1 0x0000555562c86416 in target_fetch_x86_xsave_layout () at /home/smarchi/src/binutils-gdb/gdb/target.c:803 #2 0x0000555561a672d2 in i386_gdbarch_init (info=..., arches=0x602000019bf0) at /home/smarchi/src/binutils-gdb/gdb/i386-tdep.c:8575 #3 0x0000555560749019 in gdbarch_find_by_info (info=...) at /home/smarchi/src/binutils-gdb/gdb/arch-utils.c:1417 #4 0x000055556070de0b in gdbarch_update_p (info=...) at /home/smarchi/src/binutils-gdb/gdb/arch-utils.c:599 #5 0x0000555562bb7e25 in target_find_description () at /home/smarchi/src/binutils-gdb/gdb/target-descriptions.c:504 #6 0x00005555626a6d0a in remote_target::start_remote_1 (this=0x61b000044480, from_tty=1, extended_p=0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:5077 #7 0x00005555626ab0be in remote_target::start_remote (this=0x61b000044480, from_tty=1, extended_p=0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:5316 #8 0x00005555626b329b in remote_target::open_1 (name=0x60200001a398 ":1234", from_tty=1, extended_p=0) at /home/smarchi/src/binutils-gdb/gdb/remote.c:6175 #9 0x00005555626ab3f5 in remote_target::open (name=0x60200001a398 ":1234", from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/remote.c:5338 #10 0x0000555562c86879 in open_target (args=0x60200001a398 ":1234", from_tty=1, command=0x61200004f3c0) at /home/smarchi/src/binutils-gdb/gdb/target.c:824 #11 0x0000555560d50343 in cmd_func (cmd=0x61200004f3c0, args=0x60200001a398 ":1234", from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735 #12 0x0000555562dd9f5c in execute_command (p=0x60200001a39c "4", from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/top.c:575 #13 0x000055556174be63 in command_handler (command=0x60200001a390 "") at /home/smarchi/src/binutils-gdb/gdb/event-top.c:566 #14 0x000055556174d212 in command_line_handler (rl=...) at /home/smarchi/src/binutils-gdb/gdb/event-top.c:802 #15 0x0000555562f3c3e1 in tui_command_line_handler (rl=...) at /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104 #16 0x0000555561749de2 in gdb_rl_callback_handler (rl=0x60200001a330 "tar rem :1234") at /home/smarchi/src/binutils-gdb/gdb/event-top.c:259 #17 0x00007ffff7f7b96f in rl_callback_read_char () from /usr/lib/libreadline.so.8 #18 0x000055556174978f in gdb_rl_callback_read_char_wrapper_noexcept () at /home/smarchi/src/binutils-gdb/gdb/event-top.c:195 #19 0x00005555617499e7 in gdb_rl_callback_read_char_wrapper (client_data=0x6110000007c0) at /home/smarchi/src/binutils-gdb/gdb/event-top.c:234 #20 0x0000555563092d1c in stdin_event_handler (error=0, client_data=0x6110000007c0) at /home/smarchi/src/binutils-gdb/gdb/ui.c:155 #21 0x000055556378a1e4 in handle_file_event (file_ptr=0x60700000bc40, ready_mask=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573 #22 0x000055556378ab7a in gdb_wait_for_event (block=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694 #23 0x00005555637887f3 in gdb_do_one_event (mstimeout=-1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264 #24 0x0000555561eeb6b3 in start_event_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:407 #25 0x0000555561eebeba in captured_command_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:471 #26 0x0000555561ef1535 in captured_main (data=0x7ffff35135a0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1324 #27 0x0000555561ef1672 in gdb_main (args=0x7ffff35135a0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1343 #28 0x00005555603daedf in main (argc=5, argv=0x7fffffffdd18) at /home/smarchi/src/binutils-gdb/gdb/gdb.c:39 The target description (if any) has been saved in inferior::tdesc_info earlier in target_find_description (frame 5). As to what to pass down to fetch_x86_xsave_layout (if you wanted to avoid using target_find_description), I think it depends on what the semantic of target_ops::fetch_x86_xsave_layout is: - Does the core of GDB see the xsave layout as a target-specific thing (meaning there is always a single xsave layout for a whole target)? If so I wouldn't pass anything down. The remote target could then find an inferior it owns and use this inferior's tdesc to generate a fallback xsave layout. - Does the core of GDB see the xsave layout as an inferior-specific thing (meaning that different inferiors of a given target can have different xsave layouts)? If so I would pass the inferior down. The current targets that support returning an xsave layout return a per-target value. So the question is: when we add support for fetching the xsave layout from a remote, do we make it global for the whole remote, or do we make it per-process? Also, my understanding is that even when we'll have an RSP packet to fetch the xsave layout from the remote side, we'll still need this fallback for when connecting to remotes that don't support the packet. Simon