From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id m7WfEmU/a2YIHDQAWB0awg (envelope-from ) for ; Thu, 13 Jun 2024 14:50:13 -0400 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=SyomjGrT; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 39AAB1E0C1; Thu, 13 Jun 2024 14:50:13 -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 00E631E092 for ; Thu, 13 Jun 2024 14:50:10 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 656633882100 for ; Thu, 13 Jun 2024 18:50:10 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4E4203882053 for ; Thu, 13 Jun 2024 18:49:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4E4203882053 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4E4203882053 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718304580; cv=none; b=ALf4Ic7K1QuNA9egmPXIgVuzlyO1hkeMm6slj7zYQHKKnvZAYkBHCBgJwNCu2T/2KYXoz5J01tR7ppNaZ+2zsxsA87Il0WEQpvHkUt9Oq8WjNs5+vUTywztpHJ0BrRdZlzixH1IvdE46ZQIZwzY8dTACvXtCMBQZJwuv/xvgn/E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718304580; c=relaxed/simple; bh=Ac0gJ415yy20oEptC3JTn6n/cmxPmsMPqw9TA6bepU0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=gS2epyTZz5FUdwzglNZZHIn3Tmi/t7XGhz0UA07XbPaZS0y8D7n9/gfKgtoExFem7qsXS9fDcs/rSG7EHI6i79opOmAHPB6baskOyyCrJR+VK/72vn1crfh0rIHHXe7g+mJMzRPdizi2z1+YWi6GvWyQChwmiOytk27AWhJHhl8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718304573; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=WgF/wlE+tH5V2Mvc2uvUgMz/tqvpiNW7r2JuiJlQy9w=; b=SyomjGrTeGlwStJbGqR3R4g4ni76j6o5Q5b12vxx7xgf7ZJAzN+5LEfWMeOCMBOAY7gdGd OZ5Orxgn/4yJCPDix23/2boC5ndtDmZTrgmccFRWegs72PPsA/gN0OqDOGG8CIekbm8KT2 BxpVBDvMISYVD8GrYbFZM1gVOUz8OfY= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-191-EqmzmHzxNJO7z4NvmNrUyA-1; Thu, 13 Jun 2024 14:49:30 -0400 X-MC-Unique: EqmzmHzxNJO7z4NvmNrUyA-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-a6f0da6cd62so119237966b.1 for ; Thu, 13 Jun 2024 11:49:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718304569; x=1718909369; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KMwctI34FBarelwomgtepvdiWkT/JBUEBr8rUIxkWhk=; b=COFqx2tl7pclX6P3uSFR/nIXyWRq7oQWVqNyBi7Sn4We4d3pndeZu2NJDfB2P+ayaA pz6nDe1JMb8EyzIGMPL+Ns9xKBtSffYoz3W3bj7ju0FmxORYd2/xzzSbHSJtinx2koaT E4YInK4bSdI2xNtyJ4splAWOYIeMEMQuTyEsx/qyOrlaNknq8zeUkSI9kzwgJw5872eT gW244ES+IL+kC3VEmYTH6Cn6lfjlwh14eKVHpQlyFPdPJrlIMmhzOXI0bRMemCeh9Q7B 5Tj4+4sMoXl09pN6/F6gG2G08LQdXDJ71cw8aiC80whmLHn5kdpKFSM0mqIpZ9ed1FMH qJFQ== X-Forwarded-Encrypted: i=1; AJvYcCWpCXEHmdhBJIBbwd5EQQzLkfZzp40yoWdA7+5s3KSDUAoBAw2hwX3XosL9vMWfnqyD0gvgTAOM0lU4CcoYNDFfRkXyQsG1Cydd4g== X-Gm-Message-State: AOJu0YxXgGAJf1aKCOXUH5L7j2by7zvsQXhvr0a8EbnM2a68oQ6nluqD 708Bgb8Myo4aU0CmCpnIyG6r/3nRG/YONfI8j64Jklu9PaTXrW4jhqP2NhVxN9i9+G+pw4U5VI2 RHWJW3KwhjgDQkbQskq4poo4Bik8AKj4a4KdjIzj//AMC/lG9qF0Tk8s9AUg= X-Received: by 2002:a17:906:601:b0:a6e:6555:4bcd with SMTP id a640c23a62f3a-a6f608bc410mr44074066b.35.1718304568546; Thu, 13 Jun 2024 11:49:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF5bjNcMCnrU5PvXlddb4QBNiyLtVFmHwk6lDCR9PZ4JcAYTfjGgANE7uueb6nQisMx2OFhOg== X-Received: by 2002:a17:906:601:b0:a6e:6555:4bcd with SMTP id a640c23a62f3a-a6f608bc410mr44071066b.35.1718304567544; Thu, 13 Jun 2024 11:49:27 -0700 (PDT) Received: from localhost (92.40.185.142.threembb.co.uk. [92.40.185.142]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6f56db6b1asm99173266b.88.2024.06.13.11.49.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Jun 2024 11:49:27 -0700 (PDT) From: Andrew Burgess To: "Willgerodt, Felix" , "gdb-patches@sourceware.org" Cc: John Baldwin Subject: RE: [PATCHv8 7/9] gdb/gdbserver: share some code relating to target description creation In-Reply-To: References: <624f143dded15f6be8520faa72f9742758a489bc.1717664371.git.aburgess@redhat.com> Date: Thu, 13 Jun 2024 19:49:25 +0100 Message-ID: <87v82c8yq2.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=-8.3 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, RCVD_IN_SBL_CSS, SPF_HELO_NONE, SPF_NONE, 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 "Willgerodt, Felix" writes: >> diff --git a/gdb/nat/x86-linux-tdesc.h b/gdb/nat/x86-linux-tdesc.h >> new file mode 100644 >> index 00000000000..a36ac1d0ab7 >> --- /dev/null >> +++ b/gdb/nat/x86-linux-tdesc.h >> @@ -0,0 +1,50 @@ >> +/* Target description related code for GNU/Linux x86 (i386 and x86-64). >> + >> + Copyright (C) 2024 Free Software Foundation, Inc. >> + >> + This file is part of GDB. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +#ifndef NAT_X86_LINUX_TDESC_H >> +#define NAT_X86_LINUX_TDESC_H >> + >> +#include "gdbsupport/function-view.h" >> + >> +struct target_desc; >> +struct x86_xsave_layout; >> + >> +/* Return the target description for Linux thread TID. >> + >> + The storage pointed to by XCR0_STORAGE and XSAVE_LAYOUT_STORAGE >> must >> + exist until the program (GDB or gdbserver) terminates, this storage is >> + used to cache the xcr0 and xsave layout values. The values pointed to >> + by these arguments are only updated at most once, the first time this >> + function is called. >> + >> + This function returns a target description based on the extracted xcr0 >> + value along with other characteristics of the thread identified by TID. >> + >> + This function can return nullptr if we encounter a machine configuration >> + for which a target_desc cannot be created. Ideally this would not be >> + the case, we should be able to create a target description for every >> + possible machine configuration. See amd64_linux_read_description and >> + i386_linux_read_description for cases when nullptr might be >> + returned. */ >> + >> +extern const target_desc * >> +x86_linux_tdesc_for_tid (int tid, uint64_t *xcr0_storage, >> + x86_xsave_layout *xsave_layout_storage); >> + >> +#endif /* NAT_X86_LINUX_TDESC_H */ >> diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c >> index ed76c1760bc..52416a5597f 100644 >> --- a/gdb/x86-linux-nat.c >> +++ b/gdb/x86-linux-nat.c >> @@ -41,6 +41,7 @@ >> #include "nat/x86-linux.h" >> #include "nat/x86-linux-dregs.h" >> #include "nat/linux-ptrace.h" >> +#include "nat/x86-linux-tdesc.h" >> >> /* linux_nat_target::low_new_fork implementation. */ >> >> @@ -95,93 +96,14 @@ x86_linux_nat_target::post_startup_inferior (ptid_t >> ptid) >> const struct target_desc * >> x86_linux_nat_target::read_description () >> { >> - int tid; >> - int is_64bit = 0; >> -#ifdef __x86_64__ >> - int is_x32; >> -#endif >> - static uint64_t xcr0; >> - uint64_t xcr0_features_bits; >> + static uint64_t xcr0_storage; > > Does this need to be static for GDB? We always assign to it in > x86_linux_tdesc_for_tid and never check this value again. > Which is different to gdbserver. If you change that you might also want > to update the comment for x86_linux_tdesc_for_tid in the .h file. We only assign to the storage in x86_linux_tdesc_for_tid when the global have_ptrace_getregset is UNKNOWN, after which have_ptrace_getregset is set to either TRUE or FALSE. In all future calls to x86_linux_tdesc_for_tid we use the value directly from the storage. If I was writing this code from scratch today I'm not sure I'd take this approach. I'm sure the ptrace calls used to fetch the xcr0 value are pretty slow, but really we only read the tdesc occasionally, so removing this caching might simplify things a bit. But we'd still need to return the xcr0 value as we need it on the gdbserver side -- that's why the cache doesn't live in x86_linux_tdesc_for_tid, I didn't want to end up storing the xcr0 value twice (even though it's tiny), we only had one copy in gdbserver before this series, and I wanted just one after this series. Similarly, I retained the whole caching and use of have_ptrace_getregset because that's how things were before. >> diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc >> index 2d99a82f566..84aeaefa1a0 100644 >> --- a/gdbserver/linux-x86-low.cc >> +++ b/gdbserver/linux-x86-low.cc >> @@ -29,10 +29,13 @@ >> >> #ifdef __x86_64__ >> #include "nat/amd64-linux-siginfo.h" >> +#include "arch/amd64-linux-tdesc.h" >> #else >> #include "nat/i386-linux.h" >> #endif >> >> +#include "arch/i386-linux-tdesc.h" >> + >> #include "gdb_proc_service.h" >> /* Don't include elf/common.h if linux/elf.h got included by >> gdb_proc_service.h. */ >> @@ -48,6 +51,7 @@ >> #include "nat/x86-linux.h" >> #include "nat/x86-linux-dregs.h" >> #include "linux-x86-tdesc.h" >> +#include "nat/x86-linux-tdesc.h" >> >> #ifdef __x86_64__ >> static target_desc_up tdesc_amd64_linux_no_xml; >> @@ -836,29 +840,9 @@ static int use_xml; >> /* Get Linux/x86 target description from running target. */ >> >> static const struct target_desc * >> -x86_linux_read_description (void) >> +x86_linux_read_description () >> { >> - unsigned int machine; >> - int is_elf64; >> - int xcr0_features; >> - int tid; >> - static uint64_t xcr0; >> - static int xsave_len; >> - struct regset_info *regset; >> - >> - tid = lwpid_of (current_thread); >> - >> - is_elf64 = linux_pid_exe_is_elf_64_file (tid, &machine); >> - >> - if (sizeof (void *) == 4) >> - { >> - if (is_elf64 > 0) >> - error (_("Can't debug 64-bit process with 32-bit GDBserver")); >> -#ifndef __x86_64__ >> - else if (machine == EM_X86_64) >> - error (_("Can't debug x86-64 process with 32-bit GDBserver")); >> -#endif >> - } >> + int tid = lwpid_of (current_thread); >> >> /* If we are not allowed to send an XML target description then we need >> to use the hard-wired target descriptions. This corresponds to GDB's >> @@ -868,103 +852,55 @@ x86_linux_read_description (void) >> generate some alternative target descriptions. */ >> if (!use_xml) >> { >> + x86_linux_arch_size arch_size = x86_linux_ptrace_get_arch_size (tid); >> + bool is_64bit = arch_size.is_64bit (); >> + bool is_x32 = arch_size.is_x32 (); >> + >> + if (sizeof (void *) == 4 && is_64bit && !is_x32) >> + error (_("Can't debug 64-bit process with 32-bit GDBserver")); >> + >> #ifdef __x86_64__ >> - if (machine == EM_X86_64) >> + if (is_64bit && !is_x32) >> return tdesc_amd64_linux_no_xml.get (); >> else >> #endif >> return tdesc_i386_linux_no_xml.get (); >> } >> >> -#if !defined __x86_64__ && defined HAVE_PTRACE_GETFPXREGS >> - if (machine == EM_386 && have_ptrace_getfpxregs == >> TRIBOOL_UNKNOWN) >> - { >> - elf_fpxregset_t fpxregs; >> - >> - if (ptrace (PTRACE_GETFPXREGS, tid, 0, (long) &fpxregs) < 0) >> - { >> - have_ptrace_getfpxregs = TRIBOOL_FALSE; >> - have_ptrace_getregset = TRIBOOL_FALSE; >> - return i386_linux_read_description (X86_XSTATE_X87); >> - } >> - else >> - have_ptrace_getfpxregs = TRIBOOL_TRUE; >> - } >> -#endif >> - >> - if (have_ptrace_getregset == TRIBOOL_UNKNOWN) >> - { >> - uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))]; >> - struct iovec iov; >> - >> - iov.iov_base = xstateregs; >> - iov.iov_len = sizeof (xstateregs); >> - >> - /* Check if PTRACE_GETREGSET works. */ >> - if (ptrace (PTRACE_GETREGSET, tid, >> - (unsigned int) NT_X86_XSTATE, (long) &iov) < 0) >> - have_ptrace_getregset = TRIBOOL_FALSE; >> - else >> - { >> - have_ptrace_getregset = TRIBOOL_TRUE; >> - >> - /* Get XCR0 from XSAVE extended state. */ >> - xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET >> - / sizeof (uint64_t))]; >> - >> - /* No MPX on x32. */ >> - if (machine == EM_X86_64 && !is_elf64) >> - xcr0 &= ~X86_XSTATE_MPX; >> - >> - xsave_len = x86_xsave_length (); >> - >> - /* Use PTRACE_GETREGSET if it is available. */ >> - for (regset = x86_regsets; >> - regset->fill_function != NULL; regset++) >> - if (regset->get_request == PTRACE_GETREGSET) >> - regset->size = xsave_len; >> - else if (regset->type != GENERAL_REGS) >> - regset->size = 0; >> - } >> - } >> + /* If have_ptrace_getregset is changed to true by calling >> + x86_linux_tdesc_for_tid then we will perform some additional >> + initialisation. */ >> + bool have_ptrace_getregset_is_unknown >> + = have_ptrace_getregset == TRIBOOL_UNKNOWN; >> >> - /* Check the native XCR0 only if PTRACE_GETREGSET is available. */ >> - xcr0_features = (have_ptrace_getregset == TRIBOOL_TRUE >> - && (xcr0 & X86_XSTATE_ALL_MASK)); >> + /* Get pointers to where we should store the xcr0 and xsave_layout >> + values. These will be filled in by x86_linux_tdesc_for_tid the first >> + time that the function is called. Subsequent calls will not modify >> + the stored values. */ >> + std::pair storage >> + = i387_get_xsave_storage (); >> >> - if (xcr0_features) >> - i387_set_xsave_mask (xcr0, xsave_len); >> + const target_desc *tdesc >> + = x86_linux_tdesc_for_tid (tid, storage.first, storage.second); >> >> - if (machine == EM_X86_64) >> + if (have_ptrace_getregset_is_unknown >> + && have_ptrace_getregset == TRIBOOL_TRUE) > > Nit-picking: When you only read this and have missed the > lines just above you start to scratch your head. I wonder what you think > of naming it "have_ptrace_getregset_initially_unknown" or > "have_ptrace_getregset_was_unknown". But not sure how much > better that actually is. I renamed to have_ptrace_getregset_was_unknown. I agree this is a little clearer. >> { >> -#ifdef __x86_64__ >> - const target_desc *tdesc = NULL; >> + int xsave_len = x86_xsave_length (); >> >> - if (xcr0_features) >> + /* Use PTRACE_GETREGSET if it is available. */ >> + for (regset_info *regset = x86_regsets; >> + regset->fill_function != nullptr; >> + regset++) >> { >> - tdesc = amd64_linux_read_description (xcr0 & >> X86_XSTATE_ALL_MASK, >> - !is_elf64); >> + if (regset->get_request == PTRACE_GETREGSET) >> + regset->size = xsave_len; >> + else if (regset->type != GENERAL_REGS) >> + regset->size = 0; >> } > > This is more of an aside, not necessary to address in this series: > I know you just copied this, but I always found this loop very odd. > Checking "PTRACE_GETREGSET" works for now, but it also is a bad > check to find the XSTATE regset. Why not check regset_type? > I also always wonder if supporting fpxregset and fpregset is still necessary. > But I don't know enough about them and their use cases and didn't > investigate. > > > Thanks for doing this again! From my POV the whole series is basically > ready to merge after the latest nits are addressed. > I also ran some tests on the series and couldn't find anything. > Though it is pretty hard to cover all things that this changes. > I am also in favour of merging this early and get some testing in until > the next release. Thanks. I've included the updated patch below, but as the revisions are minor, and you seemed otherwise happy, if I don't hear back from you within a week I'll just go ahead and merge this series (just let me know if you want more time for further review though). Thanks, Andrew --- commit 70bf34db561980f80c44f69e21476732455323b3 Author: Andrew Burgess Date: Thu Jan 25 14:25:57 2024 +0000 gdb/gdbserver: share some code relating to target description creation This commit is part of a series to share more of the x86 target description creation code between GDB and gdbserver. Unlike previous commits which were mostly refactoring, this commit is the first that makes a real change, though that change should mostly be for gdbserver; I've largely adopted the "GDB" way of doing things for gdbserver, and this fixes a real gdbserver bug. On a x86-64 Linux target, running the test: gdb.server/connect-with-no-symbol-file.exp results in two core files being created. Both of these core files are from the inferior process, created after gdbserver has detached. In this test a gdbserver process is started and then, after gdbserver has started, but before GDB attaches, we either delete the inferior executable, or change its permissions so it can't be read. Only after doing this do we attempt to connect with GDB. As GDB connects to gdbserver, gdbserver attempts to figure out the target description so that it can send the description to GDB, this involves a call to x86_linux_read_description. In x86_linux_read_description one of the first things we do is try to figure out if the process is 32-bit or 64-bit. To do this we look up the executable via the thread-id, and then attempt to read the architecture size from the executable. This isn't going to work if the executable has been deleted, or is no longer readable. And so, as we can't read the executable, we default to an i386 target and use an i386 target description. A consequence of using an i386 target description is that addresses are assumed to be 32-bits. Here's an example session that shows the problems this causes. This is run on an x86-64 machine, and the test binary (xx.x) is a standard 64-bit x86-64 binary: shell_1$ gdbserver --once localhost :54321 /tmp/xx.x shell_2$ gdb -q (gdb) set sysroot (gdb) shell chmod 000 /tmp/xx.x (gdb) target remote :54321 Remote debugging using :54321 warning: /tmp/xx.x: Permission denied. 0xf7fd3110 in ?? () (gdb) show architecture The target architecture is set to "auto" (currently "i386"). (gdb) p/x $pc $1 = 0xf7fd3110 (gdb) info proc mappings process 2412639 Mapped address spaces: Start Addr End Addr Size Offset Perms objfile 0x400000 0x401000 0x1000 0x0 r--p /tmp/xx.x 0x401000 0x402000 0x1000 0x1000 r-xp /tmp/xx.x 0x402000 0x403000 0x1000 0x2000 r--p /tmp/xx.x 0x403000 0x405000 0x2000 0x2000 rw-p /tmp/xx.x 0xf7fcb000 0xf7fcf000 0x4000 0x0 r--p [vvar] 0xf7fcf000 0xf7fd1000 0x2000 0x0 r-xp [vdso] 0xf7fd1000 0xf7fd3000 0x2000 0x0 r--p /usr/lib64/ld-2.30.so 0xf7fd3000 0xf7ff3000 0x20000 0x2000 r-xp /usr/lib64/ld-2.30.so 0xf7ff3000 0xf7ffb000 0x8000 0x22000 r--p /usr/lib64/ld-2.30.so 0xf7ffc000 0xf7ffe000 0x2000 0x2a000 rw-p /usr/lib64/ld-2.30.so 0xf7ffe000 0xf7fff000 0x1000 0x0 rw-p 0xfffda000 0xfffff000 0x25000 0x0 rw-p [stack] 0xff600000 0xff601000 0x1000 0x0 r-xp [vsyscall] (gdb) info inferiors Num Description Connection Executable * 1 process 2412639 1 (remote :54321) (gdb) shell cat /proc/2412639/maps 00400000-00401000 r--p 00000000 fd:03 45907133 /tmp/xx.x 00401000-00402000 r-xp 00001000 fd:03 45907133 /tmp/xx.x 00402000-00403000 r--p 00002000 fd:03 45907133 /tmp/xx.x 00403000-00405000 rw-p 00002000 fd:03 45907133 /tmp/xx.x 7ffff7fcb000-7ffff7fcf000 r--p 00000000 00:00 0 [vvar] 7ffff7fcf000-7ffff7fd1000 r-xp 00000000 00:00 0 [vdso] 7ffff7fd1000-7ffff7fd3000 r--p 00000000 fd:00 143904 /usr/lib64/ld-2.30.so 7ffff7fd3000-7ffff7ff3000 r-xp 00002000 fd:00 143904 /usr/lib64/ld-2.30.so 7ffff7ff3000-7ffff7ffb000 r--p 00022000 fd:00 143904 /usr/lib64/ld-2.30.so 7ffff7ffc000-7ffff7ffe000 rw-p 0002a000 fd:00 143904 /usr/lib64/ld-2.30.so 7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0 7ffffffda000-7ffffffff000 rw-p 00000000 00:00 0 [stack] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] (gdb) Notice the difference between the mappings reported via GDB and those reported directly from the kernel via /proc/PID/maps, the addresses of every mapping is clamped to 32-bits for GDB, while the kernel reports real 64-bit addresses. Notice also that the $pc value is a 32-bit value. It appears to be within one of the mappings reported by GDB, but is outside any of the mappings reported from the kernel. And this is where the problem arises. When gdbserver detaches from the inferior we pass the inferior the address from which it should resume. Due to the 32/64 bit confusion we tell the inferior to resume from the 32-bit $pc value, which is not within any valid mapping, and so, as soon as the inferior resumes, it segfaults. If we look at how GDB (not gdbserver) figures out its target description then we see an interesting difference. GDB doesn't try to read the executable. Instead GDB uses ptrace to query the thread's state, and uses this to figure out the if the thread is 32 or 64 bit. If we update gdbserver to do it the "GDB" way then the above problem is resolved, gdbserver now sees the process as 64-bit, and when we detach from the inferior we give it the correct 64-bit address, and the inferior no longer segfaults. Now, I could just update the gdbserver code, but better, I think, to share one copy of the code between GDB and gdbserver in gdb/nat/. That is what this commit does. The cores of x86_linux_read_description from gdbserver and x86_linux_nat_target::read_description from GDB are moved into a new file gdb/nat/x86-linux-tdesc.c and combined into a single function x86_linux_tdesc_for_tid which is called from each location. This new function does things mostly the GDB way, some changes are needed to allow for the sharing; we now take some pointers for where the shared code can cache the xcr0 and xsave layout values. Another thing to note about this commit is how the functions i386_linux_read_description and amd64_linux_read_description are handled. For now I've left these function as implemented separately in GDB and gdbserver. I've moved the declarations of these functions into gdb/arch/{i386,amd64}-linux-tdesc.h, but the implementations are left where they are. A later commit in this series will make these functions shared too, but doing this is not trivial, so I've left that for a separate commit. Merging the declarations as I've done here ensures that everyone implements the function to the same API, and once these functions are shared (in a later commit) we'll want a shared declaration anyway. Reviewed-By: Felix Willgerodt Acked-By: John Baldwin diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 277f878371c..949f1042ce7 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1550,8 +1550,10 @@ HFILES_NO_SRCDIR = \ arch/aarch64-insn.h \ arch/aarch64-mte-linux.h \ arch/aarch64-scalable-linux.h \ + arch/amd64-linux-tdesc.h \ arch/arc.h \ arch/arm.h \ + arch/i386-linux-tdesc.h \ arch/i386.h \ arch/loongarch.h \ arch/ppc-linux-common.h \ @@ -1608,6 +1610,7 @@ HFILES_NO_SRCDIR = \ nat/x86-gcc-cpuid.h \ nat/x86-linux.h \ nat/x86-linux-dregs.h \ + nat/x86-linux-tdesc.h \ python/py-event.h \ python/py-events.h \ python/py-stopevent.h \ diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index c52b0436872..f9647dce1e6 100644 --- a/gdb/amd64-linux-tdep.c +++ b/gdb/amd64-linux-tdep.c @@ -42,6 +42,7 @@ #include "arch/amd64.h" #include "target-descriptions.h" #include "expop.h" +#include "arch/amd64-linux-tdesc.h" /* The syscall's XML filename for i386. */ #define XML_SYSCALL_FILENAME_AMD64 "syscalls/amd64-linux.xml" diff --git a/gdb/amd64-linux-tdep.h b/gdb/amd64-linux-tdep.h index 66170256639..6f5b55a01a5 100644 --- a/gdb/amd64-linux-tdep.h +++ b/gdb/amd64-linux-tdep.h @@ -31,12 +31,6 @@ /* Total number of registers for GNU/Linux. */ #define AMD64_LINUX_NUM_REGS (AMD64_LINUX_ORIG_RAX_REGNUM + 1) -/* Return the right amd64-linux target descriptions according to - XCR0_FEATURES_BIT and IS_X32. */ - -const target_desc *amd64_linux_read_description (uint64_t xcr0_features_bit, - bool is_x32); - /* Enum that defines the syscall identifiers for amd64 linux. Used for process record/replay, these will be translated into a gdb-canonical set of syscall ids in linux-record.c. */ diff --git a/gdb/arch/amd64-linux-tdesc.h b/gdb/arch/amd64-linux-tdesc.h new file mode 100644 index 00000000000..db425b60df6 --- /dev/null +++ b/gdb/arch/amd64-linux-tdesc.h @@ -0,0 +1,30 @@ +/* Target description related code for GNU/Linux x86-64. + + Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef ARCH_AMD64_LINUX_TDESC_H +#define ARCH_AMD64_LINUX_TDESC_H + +struct target_desc; + +/* Return the AMD64 target descriptions corresponding to XCR0 and IS_X32. */ + +extern const target_desc *amd64_linux_read_description (uint64_t xcr0, + bool is_x32); + +#endif /* ARCH_AMD64_LINUX_TDESC_H */ diff --git a/gdb/arch/i386-linux-tdesc.h b/gdb/arch/i386-linux-tdesc.h new file mode 100644 index 00000000000..0b736337a75 --- /dev/null +++ b/gdb/arch/i386-linux-tdesc.h @@ -0,0 +1,29 @@ +/* Target description related code for GNU/Linux i386. + + Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef ARCH_I386_LINUX_TDESC_H +#define ARCH_I386_LINUX_TDESC_H + +struct target_desc; + +/* Return the i386 target description corresponding to XCR0. */ + +extern const struct target_desc *i386_linux_read_description (uint64_t xcr0); + +#endif /* ARCH_I386_LINUX_TDESC_H */ diff --git a/gdb/configure.nat b/gdb/configure.nat index 25268bb268b..a30b07ef8c5 100644 --- a/gdb/configure.nat +++ b/gdb/configure.nat @@ -256,7 +256,8 @@ case ${gdb_host} in NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \ nat/x86-xstate.o \ i386-linux-nat.o x86-linux-nat.o nat/linux-btrace.o \ - nat/x86-linux.o nat/x86-linux-dregs.o nat/i386-linux.o" + nat/x86-linux.o nat/x86-linux-dregs.o nat/i386-linux.o \ + nat/x86-linux-tdesc.o" ;; ia64) # Host: Intel IA-64 running GNU/Linux @@ -322,7 +323,7 @@ case ${gdb_host} in NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \ nat/x86-xstate.o amd64-nat.o amd64-linux-nat.o x86-linux-nat.o \ nat/linux-btrace.o \ - nat/x86-linux.o nat/x86-linux-dregs.o \ + nat/x86-linux.o nat/x86-linux-dregs.o nat/x86-linux-tdesc.o \ nat/amd64-linux-siginfo.o" ;; sparc) diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 511e43f3b6f..6a7490680c1 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -40,6 +40,7 @@ #include "i387-tdep.h" #include "gdbsupport/x86-xstate.h" +#include "arch/i386-linux-tdesc.h" /* The syscall's XML filename for i386. */ #define XML_SYSCALL_FILENAME_I386 "syscalls/i386-linux.xml" diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h index 07593c6a8ec..e8691cd778e 100644 --- a/gdb/i386-linux-tdep.h +++ b/gdb/i386-linux-tdep.h @@ -55,9 +55,6 @@ extern void i386_linux_report_signal_info (struct gdbarch *gdbarch, struct ui_out *uiout, enum gdb_signal siggnal); -/* Return the target description according to XCR0. */ -extern const struct target_desc *i386_linux_read_description (uint64_t xcr0); - extern int i386_linux_gregset_reg_offset[]; /* Return x86 siginfo type. */ diff --git a/gdb/nat/x86-linux-tdesc.c b/gdb/nat/x86-linux-tdesc.c new file mode 100644 index 00000000000..05aa75d1b80 --- /dev/null +++ b/gdb/nat/x86-linux-tdesc.c @@ -0,0 +1,133 @@ +/* Target description related code for GNU/Linux x86 (i386 and x86-64). + + Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include "nat/x86-linux-tdesc.h" +#ifdef __x86_64__ +#include "arch/amd64.h" +#include "arch/amd64-linux-tdesc.h" +#endif +#include "arch/i386.h" +#include "arch/i386-linux-tdesc.h" + +#include "nat/x86-linux.h" +#include "nat/gdb_ptrace.h" +#include "nat/x86-xstate.h" +#include "gdbsupport/x86-xstate.h" + +#ifndef __x86_64__ +#include +#include "nat/i386-linux.h" +#endif + +#include +#include + +#ifndef IN_PROCESS_AGENT + +/* See nat/x86-linux-tdesc.h. */ + +const target_desc * +x86_linux_tdesc_for_tid (int tid, uint64_t *xcr0_storage, + x86_xsave_layout *xsave_layout_storage) +{ +#ifdef __x86_64__ + x86_linux_arch_size arch_size = x86_linux_ptrace_get_arch_size (tid); + bool is_64bit = arch_size.is_64bit (); + bool is_x32 = arch_size.is_x32 (); + + if (sizeof (void *) == 4 && is_64bit && !is_x32) + { +#ifdef GDBSERVER + error (_("Can't debug 64-bit process with 32-bit GDBserver")); +#else + error (_("Can't debug 64-bit process with 32-bit GDB")); +#endif + } + +#elif HAVE_PTRACE_GETFPXREGS + if (have_ptrace_getfpxregs == TRIBOOL_UNKNOWN) + { + elf_fpxregset_t fpxregs; + + if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0) + { + have_ptrace_getfpxregs = TRIBOOL_FALSE; + have_ptrace_getregset = TRIBOOL_FALSE; + } + else + have_ptrace_getfpxregs = TRIBOOL_TRUE; + } + + if (have_ptrace_getfpxregs == TRIBOOL_FALSE) + return i386_linux_read_description (X86_XSTATE_X87_MASK); +#endif + + if (have_ptrace_getregset == TRIBOOL_UNKNOWN) + { + uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))]; + struct iovec iov; + + iov.iov_base = xstateregs; + iov.iov_len = sizeof (xstateregs); + + /* Check if PTRACE_GETREGSET works. */ + if (ptrace (PTRACE_GETREGSET, tid, + (unsigned int) NT_X86_XSTATE, &iov) < 0) + { + have_ptrace_getregset = TRIBOOL_FALSE; + *xcr0_storage = 0; + } + else + { + have_ptrace_getregset = TRIBOOL_TRUE; + + /* Get XCR0 from XSAVE extended state. */ + *xcr0_storage = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET + / sizeof (uint64_t))]; + +#ifdef __x86_64__ + /* No MPX on x32. */ + if (is_64bit && is_x32) + *xcr0_storage &= ~X86_XSTATE_MPX; +#endif /* __x86_64__ */ + + *xsave_layout_storage + = x86_fetch_xsave_layout (*xcr0_storage, x86_xsave_length ()); + } + } + + /* Check the native XCR0 only if PTRACE_GETREGSET is available. If + PTRACE_GETREGSET is not available then set xcr0_features_bits to + zero so that the "no-features" descriptions are returned by the + code below. */ + uint64_t xcr0_features_bits; + if (have_ptrace_getregset == TRIBOOL_TRUE) + xcr0_features_bits = *xcr0_storage & X86_XSTATE_ALL_MASK; + else + xcr0_features_bits = 0; + +#ifdef __x86_64__ + if (is_64bit) + return amd64_linux_read_description (xcr0_features_bits, is_x32); + else +#endif + return i386_linux_read_description (xcr0_features_bits); +} + +#endif /* !IN_PROCESS_AGENT */ diff --git a/gdb/nat/x86-linux-tdesc.h b/gdb/nat/x86-linux-tdesc.h new file mode 100644 index 00000000000..b7a4649ed93 --- /dev/null +++ b/gdb/nat/x86-linux-tdesc.h @@ -0,0 +1,51 @@ +/* Target description related code for GNU/Linux x86 (i386 and x86-64). + + Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef NAT_X86_LINUX_TDESC_H +#define NAT_X86_LINUX_TDESC_H + +#include "gdbsupport/function-view.h" + +struct target_desc; +struct x86_xsave_layout; + +/* Return the target description for Linux thread TID. + + The storage pointed to by XCR0_STORAGE and XSAVE_LAYOUT_STORAGE must + exist until the program (GDB or gdbserver) terminates, this storage is + used to cache the xcr0 and xsave layout values. The values pointed to + by these arguments are only updated at most once, the first time this + function is called if the have_ptrace_getregset global is set to + TRIBOOL_UNKNOWN. + + This function returns a target description based on the extracted xcr0 + value along with other characteristics of the thread identified by TID. + + This function can return nullptr if we encounter a machine configuration + for which a target_desc cannot be created. Ideally this would not be + the case, we should be able to create a target description for every + possible machine configuration. See amd64_linux_read_description and + i386_linux_read_description for cases when nullptr might be + returned. */ + +extern const target_desc * +x86_linux_tdesc_for_tid (int tid, uint64_t *xcr0_storage, + x86_xsave_layout *xsave_layout_storage); + +#endif /* NAT_X86_LINUX_TDESC_H */ diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c index ed76c1760bc..2afa04f6288 100644 --- a/gdb/x86-linux-nat.c +++ b/gdb/x86-linux-nat.c @@ -41,6 +41,7 @@ #include "nat/x86-linux.h" #include "nat/x86-linux-dregs.h" #include "nat/linux-ptrace.h" +#include "nat/x86-linux-tdesc.h" /* linux_nat_target::low_new_fork implementation. */ @@ -95,93 +96,16 @@ x86_linux_nat_target::post_startup_inferior (ptid_t ptid) const struct target_desc * x86_linux_nat_target::read_description () { - int tid; - int is_64bit = 0; -#ifdef __x86_64__ - int is_x32; -#endif - static uint64_t xcr0; - uint64_t xcr0_features_bits; + /* The x86_linux_tdesc_for_tid call only reads xcr0 the first time it is + called, the xcr0 value is stored here and reused on subsequent calls. */ + static uint64_t xcr0_storage; if (inferior_ptid == null_ptid) return this->beneath ()->read_description (); - tid = inferior_ptid.pid (); - -#ifdef __x86_64__ - - x86_linux_arch_size arch_size = x86_linux_ptrace_get_arch_size (tid); - is_64bit = arch_size.is_64bit (); - is_x32 = arch_size.is_x32 (); - - if (sizeof (void *) == 4 && is_64bit && !is_x32) - error (_("Can't debug 64-bit process with 32-bit GDB")); - -#elif HAVE_PTRACE_GETFPXREGS - if (have_ptrace_getfpxregs == TRIBOOL_UNKNOWN) - { - elf_fpxregset_t fpxregs; - - if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0) - { - have_ptrace_getfpxregs = TRIBOOL_FALSE; - have_ptrace_getregset = TRIBOOL_FALSE; - return i386_linux_read_description (X86_XSTATE_X87_MASK); - } - } -#endif - - if (have_ptrace_getregset == TRIBOOL_UNKNOWN) - { - uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))]; - struct iovec iov; - - iov.iov_base = xstateregs; - iov.iov_len = sizeof (xstateregs); - - /* Check if PTRACE_GETREGSET works. */ - if (ptrace (PTRACE_GETREGSET, tid, - (unsigned int) NT_X86_XSTATE, &iov) < 0) - have_ptrace_getregset = TRIBOOL_FALSE; - else - { - have_ptrace_getregset = TRIBOOL_TRUE; - - /* Get XCR0 from XSAVE extended state. */ - xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET - / sizeof (uint64_t))]; - - m_xsave_layout = x86_fetch_xsave_layout (xcr0, x86_xsave_length ()); - } - } - - /* Check the native XCR0 only if PTRACE_GETREGSET is available. If - PTRACE_GETREGSET is not available then set xcr0_features_bits to - zero so that the "no-features" descriptions are returned by the - switches below. */ - if (have_ptrace_getregset == TRIBOOL_TRUE) - xcr0_features_bits = xcr0 & X86_XSTATE_ALL_MASK; - else - xcr0_features_bits = 0; - - if (is_64bit) - { -#ifdef __x86_64__ - return amd64_linux_read_description (xcr0_features_bits, is_x32); -#endif - } - else - { - const struct target_desc * tdesc - = i386_linux_read_description (xcr0_features_bits); - - if (tdesc == NULL) - tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK); - - return tdesc; - } + int tid = inferior_ptid.pid (); - gdb_assert_not_reached ("failed to return tdesc"); + return x86_linux_tdesc_for_tid (tid, &xcr0_storage, &this->m_xsave_layout); } diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv index 8e882d2159b..538845b96d5 100644 --- a/gdbserver/configure.srv +++ b/gdbserver/configure.srv @@ -110,6 +110,7 @@ case "${gdbserver_host}" in srv_tgtobj="${srv_tgtobj} nat/x86-linux.o" srv_tgtobj="${srv_tgtobj} nat/x86-linux-dregs.o" srv_tgtobj="${srv_tgtobj} nat/i386-linux.o" + srv_tgtobj="${srv_tgtobj} nat/x86-linux-tdesc.o" srv_linux_usrregs=yes srv_linux_regsets=yes srv_linux_thread_db=yes @@ -372,6 +373,7 @@ case "${gdbserver_host}" in srv_tgtobj="${srv_tgtobj} nat/linux-btrace.o" srv_tgtobj="${srv_tgtobj} nat/x86-linux.o" srv_tgtobj="${srv_tgtobj} nat/x86-linux-dregs.o" + srv_tgtobj="${srv_tgtobj} nat/x86-linux-tdesc.o" srv_tgtobj="${srv_tgtobj} nat/amd64-linux-siginfo.o" srv_linux_usrregs=yes # This is for i386 progs. srv_linux_regsets=yes diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc index 62cafd87204..4d8bcb5edfa 100644 --- a/gdbserver/i387-fp.cc +++ b/gdbserver/i387-fp.cc @@ -21,7 +21,7 @@ #include "nat/x86-xstate.h" /* Default to SSE. */ -static unsigned long long x86_xcr0 = X86_XSTATE_SSE_MASK; +static uint64_t x86_xcr0 = X86_XSTATE_SSE_MASK; static const int num_mpx_bnd_registers = 4; static const int num_mpx_cfg_registers = 2; @@ -944,9 +944,8 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf) /* See i387-fp.h. */ -void -i387_set_xsave_mask (uint64_t xcr0, int len) +std::pair +i387_get_xsave_storage () { - x86_xcr0 = xcr0; - xsave_layout = x86_fetch_xsave_layout (xcr0, len); + return { &x86_xcr0, &xsave_layout }; } diff --git a/gdbserver/i387-fp.h b/gdbserver/i387-fp.h index 450466efb75..4ee21da8461 100644 --- a/gdbserver/i387-fp.h +++ b/gdbserver/i387-fp.h @@ -19,6 +19,8 @@ #ifndef GDBSERVER_I387_FP_H #define GDBSERVER_I387_FP_H +struct x86_xsave_layout; + void i387_cache_to_fsave (struct regcache *regcache, void *buf); void i387_fsave_to_cache (struct regcache *regcache, const void *buf); @@ -30,6 +32,6 @@ void i387_xsave_to_cache (struct regcache *regcache, const void *buf); /* Set the XSAVE mask and fetch the XSAVE layout via CPUID. */ -void i387_set_xsave_mask (uint64_t xcr0, int len); +std::pair i387_get_xsave_storage (); #endif /* GDBSERVER_I387_FP_H */ diff --git a/gdbserver/linux-amd64-ipa.cc b/gdbserver/linux-amd64-ipa.cc index a6346750f49..df5e6aca081 100644 --- a/gdbserver/linux-amd64-ipa.cc +++ b/gdbserver/linux-amd64-ipa.cc @@ -22,6 +22,7 @@ #include "tracepoint.h" #include "linux-x86-tdesc.h" #include "gdbsupport/x86-xstate.h" +#include "arch/amd64-linux-tdesc.h" /* fast tracepoints collect registers. */ diff --git a/gdbserver/linux-i386-ipa.cc b/gdbserver/linux-i386-ipa.cc index 8f14e0937d4..aa346fc9bc3 100644 --- a/gdbserver/linux-i386-ipa.cc +++ b/gdbserver/linux-i386-ipa.cc @@ -22,6 +22,7 @@ #include "tracepoint.h" #include "linux-x86-tdesc.h" #include "gdbsupport/x86-xstate.h" +#include "arch/i386-linux-tdesc.h" /* GDB register numbers. */ diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc index 2d99a82f566..efb67979310 100644 --- a/gdbserver/linux-x86-low.cc +++ b/gdbserver/linux-x86-low.cc @@ -29,10 +29,13 @@ #ifdef __x86_64__ #include "nat/amd64-linux-siginfo.h" +#include "arch/amd64-linux-tdesc.h" #else #include "nat/i386-linux.h" #endif +#include "arch/i386-linux-tdesc.h" + #include "gdb_proc_service.h" /* Don't include elf/common.h if linux/elf.h got included by gdb_proc_service.h. */ @@ -48,6 +51,7 @@ #include "nat/x86-linux.h" #include "nat/x86-linux-dregs.h" #include "linux-x86-tdesc.h" +#include "nat/x86-linux-tdesc.h" #ifdef __x86_64__ static target_desc_up tdesc_amd64_linux_no_xml; @@ -836,29 +840,9 @@ static int use_xml; /* Get Linux/x86 target description from running target. */ static const struct target_desc * -x86_linux_read_description (void) +x86_linux_read_description () { - unsigned int machine; - int is_elf64; - int xcr0_features; - int tid; - static uint64_t xcr0; - static int xsave_len; - struct regset_info *regset; - - tid = lwpid_of (current_thread); - - is_elf64 = linux_pid_exe_is_elf_64_file (tid, &machine); - - if (sizeof (void *) == 4) - { - if (is_elf64 > 0) - error (_("Can't debug 64-bit process with 32-bit GDBserver")); -#ifndef __x86_64__ - else if (machine == EM_X86_64) - error (_("Can't debug x86-64 process with 32-bit GDBserver")); -#endif - } + int tid = lwpid_of (current_thread); /* If we are not allowed to send an XML target description then we need to use the hard-wired target descriptions. This corresponds to GDB's @@ -868,103 +852,55 @@ x86_linux_read_description (void) generate some alternative target descriptions. */ if (!use_xml) { + x86_linux_arch_size arch_size = x86_linux_ptrace_get_arch_size (tid); + bool is_64bit = arch_size.is_64bit (); + bool is_x32 = arch_size.is_x32 (); + + if (sizeof (void *) == 4 && is_64bit && !is_x32) + error (_("Can't debug 64-bit process with 32-bit GDBserver")); + #ifdef __x86_64__ - if (machine == EM_X86_64) + if (is_64bit && !is_x32) return tdesc_amd64_linux_no_xml.get (); else #endif return tdesc_i386_linux_no_xml.get (); } -#if !defined __x86_64__ && defined HAVE_PTRACE_GETFPXREGS - if (machine == EM_386 && have_ptrace_getfpxregs == TRIBOOL_UNKNOWN) - { - elf_fpxregset_t fpxregs; - - if (ptrace (PTRACE_GETFPXREGS, tid, 0, (long) &fpxregs) < 0) - { - have_ptrace_getfpxregs = TRIBOOL_FALSE; - have_ptrace_getregset = TRIBOOL_FALSE; - return i386_linux_read_description (X86_XSTATE_X87); - } - else - have_ptrace_getfpxregs = TRIBOOL_TRUE; - } -#endif - - if (have_ptrace_getregset == TRIBOOL_UNKNOWN) - { - uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))]; - struct iovec iov; - - iov.iov_base = xstateregs; - iov.iov_len = sizeof (xstateregs); - - /* Check if PTRACE_GETREGSET works. */ - if (ptrace (PTRACE_GETREGSET, tid, - (unsigned int) NT_X86_XSTATE, (long) &iov) < 0) - have_ptrace_getregset = TRIBOOL_FALSE; - else - { - have_ptrace_getregset = TRIBOOL_TRUE; - - /* Get XCR0 from XSAVE extended state. */ - xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET - / sizeof (uint64_t))]; - - /* No MPX on x32. */ - if (machine == EM_X86_64 && !is_elf64) - xcr0 &= ~X86_XSTATE_MPX; - - xsave_len = x86_xsave_length (); - - /* Use PTRACE_GETREGSET if it is available. */ - for (regset = x86_regsets; - regset->fill_function != NULL; regset++) - if (regset->get_request == PTRACE_GETREGSET) - regset->size = xsave_len; - else if (regset->type != GENERAL_REGS) - regset->size = 0; - } - } + /* If have_ptrace_getregset is changed to true by calling + x86_linux_tdesc_for_tid then we will perform some additional + initialisation. */ + bool have_ptrace_getregset_was_unknown + = have_ptrace_getregset == TRIBOOL_UNKNOWN; - /* Check the native XCR0 only if PTRACE_GETREGSET is available. */ - xcr0_features = (have_ptrace_getregset == TRIBOOL_TRUE - && (xcr0 & X86_XSTATE_ALL_MASK)); + /* Get pointers to where we should store the xcr0 and xsave_layout + values. These will be filled in by x86_linux_tdesc_for_tid the first + time that the function is called. Subsequent calls will not modify + the stored values. */ + std::pair storage + = i387_get_xsave_storage (); - if (xcr0_features) - i387_set_xsave_mask (xcr0, xsave_len); + const target_desc *tdesc + = x86_linux_tdesc_for_tid (tid, storage.first, storage.second); - if (machine == EM_X86_64) + if (have_ptrace_getregset_was_unknown + && have_ptrace_getregset == TRIBOOL_TRUE) { -#ifdef __x86_64__ - const target_desc *tdesc = NULL; + int xsave_len = x86_xsave_length (); - if (xcr0_features) + /* Use PTRACE_GETREGSET if it is available. */ + for (regset_info *regset = x86_regsets; + regset->fill_function != nullptr; + regset++) { - tdesc = amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK, - !is_elf64); + if (regset->get_request == PTRACE_GETREGSET) + regset->size = xsave_len; + else if (regset->type != GENERAL_REGS) + regset->size = 0; } - - if (tdesc == NULL) - tdesc = amd64_linux_read_description (X86_XSTATE_SSE_MASK, !is_elf64); - return tdesc; -#endif - } - else - { - const target_desc *tdesc = NULL; - - if (xcr0_features) - tdesc = i386_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK); - - if (tdesc == NULL) - tdesc = i386_linux_read_description (X86_XSTATE_SSE); - - return tdesc; } - gdb_assert_not_reached ("failed to return tdesc"); + return tdesc; } /* Update all the target description of all processes; a new GDB diff --git a/gdbserver/linux-x86-tdesc.cc b/gdbserver/linux-x86-tdesc.cc index cd3b5d80e37..af3735aa895 100644 --- a/gdbserver/linux-x86-tdesc.cc +++ b/gdbserver/linux-x86-tdesc.cc @@ -23,8 +23,10 @@ #include "gdbsupport/x86-xstate.h" #ifdef __x86_64__ #include "arch/amd64.h" +#include "arch/amd64-linux-tdesc.h" #endif #include "x86-tdesc.h" +#include "arch/i386-linux-tdesc.h" /* Return the right x86_linux_tdesc index for a given XCR0. Return X86_TDESC_LAST if can't find a match. */ diff --git a/gdbserver/linux-x86-tdesc.h b/gdbserver/linux-x86-tdesc.h index f9561b129ae..576aaf5e165 100644 --- a/gdbserver/linux-x86-tdesc.h +++ b/gdbserver/linux-x86-tdesc.h @@ -46,11 +46,4 @@ int amd64_get_ipa_tdesc_idx (const struct target_desc *tdesc); const struct target_desc *i386_get_ipa_tdesc (int idx); -#ifdef __x86_64__ -const struct target_desc *amd64_linux_read_description (uint64_t xcr0, - bool is_x32); -#endif - -const struct target_desc *i386_linux_read_description (uint64_t xcr0); - #endif /* GDBSERVER_LINUX_X86_TDESC_H */