From patchwork Wed Jan 24 15:00:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 84679 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 26A833858005 for ; Wed, 24 Jan 2024 15:01:50 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-io1-xd30.google.com (mail-io1-xd30.google.com [IPv6:2607:f8b0:4864:20::d30]) by sourceware.org (Postfix) with ESMTPS id A901E38582A9 for ; Wed, 24 Jan 2024 15:00:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A901E38582A9 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A901E38582A9 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::d30 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706108455; cv=none; b=Cd//Oe//RJY8kjPhJMKssAkMqV8gPTYCey0g7paflrBE16r21nVhEzKDw7RXwy3YquqmjndY9vKvXFukw/fJ5ZN0GDJLsMUjJE5oewfXAN2yIMFQl0EXl60GZgGopW7HpowTByWYdj2Wq5ezc8IuYdi8Ayj0S9K4cdTBie8F1jA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706108455; c=relaxed/simple; bh=Yr+7iZVIwi+CM5urITnbhGQaWPMkbJgWC2vjtHP2AnM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Ip9204EmUQPJzxusc7NcwxN4Pz0l4xnmzmeF5mjD89C0QPdDt06FOTgFUcrATe9TrqyDgmJlz6PrggtZnVsryKSPa/edY/hWFAORa8KLpPs1pFXFQYK6FDm8uuwUcs4kHl9teS8B5Bw97XM7K6dZl8AiVwtOD7pIlJ4C+6688b4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-io1-xd30.google.com with SMTP id ca18e2360f4ac-7beddc139c4so243174239f.0 for ; Wed, 24 Jan 2024 07:00:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1706108452; x=1706713252; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=39Yy5jXoWxKot0Y7xZ9WjcaRJDYb4LhhQR5zv9Cu79U=; b=INmGfOrzPFbSMYON1gq4ZlNn8tlbfwNFp0kRfge8FN2EOPLrSWd3HQOT9Yo1yHcVji DgcO2iLlCzzY50Ts3vwcbVukmE6h8/smMu9XBNuCtYvIiHJyXML8rPcpr85l6UHCSx31 y9S+duHkefIN2lZAdG2Mzcy5CiVJUQg8hBLzH+9hZ8R/QH/EhSDpA6Qv+mMd5Edb6Dq8 Cz3R05Qhex8laoyvmLvVoRxyIlrQ//ucXo4WfwjUDgz5hFFBUlt4SxMMw4HSP6WQKhpw +eeaK2dkxKJb0NNUP9sgZwMw8qWKRmfKU5UaH3bPZWUKsXSS2NaFsYffSrvO3igktefz xRWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706108452; x=1706713252; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=39Yy5jXoWxKot0Y7xZ9WjcaRJDYb4LhhQR5zv9Cu79U=; b=qkv0BdJNhhLSCgJgrsF17aB3D5cBtCvSG4yUxroX3YYPNea3XFIavCqdQ45HOWbIAI 20DK9KUdseZM3J13ti/iL1LPMfYN16GOord5jcjoVdxQIAVYw/Cf0FjJiv2gcIh3fhEH 6fo8BmpQocA0Jm6A/hmqX7Cp9RdgS4p7e1EX6LLERuc9F6jL2PYrfs9kfo4a1yC/fv2m VgXGTsdYIE1v1+oTxpTZfM1aM9IJMmGtYToTbEAQehSvCNk6Uv2IO0PUJr/rdmrlUv22 iYpZuagEBVkA9RBN7Q8cRrrl5wct+MhakafnO2SxMmQsCI6/otWcR2Hdl3b7SUrxcO5q 7iJQ== X-Gm-Message-State: AOJu0Yz2Nu8FZEFDeN+uno7rs9dl4otjJ6aGuCOuyGtm7xQzXobmRSc0 WHav/m/3TIfXytpLw9UkzPr8REv3zGj/36K6FFJHxgW/30whq7+751YqYE1bTx89+jASYIPjD9A = X-Google-Smtp-Source: AGHT+IFB/yoledeeghBErYYOjVzkzrz6vH6eyUiTb39foLN90G0vdDImQ4QXv2MYWR6omeRN25+Pnw== X-Received: by 2002:a6b:7c03:0:b0:7be:f466:9b5e with SMTP id m3-20020a6b7c03000000b007bef4669b5emr1686864iok.40.1706108451721; Wed, 24 Jan 2024 07:00:51 -0800 (PST) Received: from localhost.localdomain (97-122-68-157.hlrn.qwest.net. [97.122.68.157]) by smtp.gmail.com with ESMTPSA id f17-20020a02cad1000000b0046f1a2a38b6sm112585jap.121.2024.01.24.07.00.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Jan 2024 07:00:51 -0800 (PST) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Really fix Windows gdbserver segment registers Date: Wed, 24 Jan 2024 08:00:44 -0700 Message-ID: <20240124150044.2881846-1-tromey@adacore.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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+patchwork=sourceware.org@sourceware.org My earlier attempt to mask the segment registers in gdbserver for Windows introduced some bugs. That patch is here: https://sourceware.org/pipermail/gdb-patches/2023-December/205306.html The problem turned out to be that these fields in the Windows 'CONTEXT' type are just 16 bits, so while masking the values on read is fine, writing the truncated values back then causes zeros to be written to some subsequent field. This patch cleans this up by arranging never to write too much data to a field. I also noticed that two register numbers here were never updated for the 64-bit port. This patch fixes this as well, and renames the "FCS" register to follow gdb. Finally, this patch applies the same treatment to windows-nat.c. Reviewed-by: John Baldwin --- gdb/windows-nat.c | 44 +++++++++++++------ gdbserver/win32-i386-low.cc | 85 +++++++++++++++++++++++++++++++++---- 2 files changed, 107 insertions(+), 22 deletions(-) diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 28f142c1c6a..6fdd1f3a151 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -662,23 +662,19 @@ windows_fetch_one_register (struct regcache *regcache, gdb_assert (gdbarch_pc_regnum (gdbarch) >= 0); gdb_assert (!gdbarch_write_pc_p (gdbarch)); - if (r == I387_FISEG_REGNUM (tdep)) + /* GDB treats some registers as 32-bit, where they are in fact only + 16 bits long. These cases must be handled specially to avoid + reading extraneous bits from the context. */ + if (r == I387_FISEG_REGNUM (tdep) || windows_process.segment_register_p (r)) { - long l = *((long *) context_offset) & 0xffff; - regcache->raw_supply (r, (char *) &l); + gdb_byte bytes[4] = {}; + memcpy (bytes, context_offset, 2); + regcache->raw_supply (r, bytes); } else if (r == I387_FOP_REGNUM (tdep)) { long l = (*((long *) context_offset) >> 16) & ((1 << 11) - 1); - regcache->raw_supply (r, (char *) &l); - } - else if (windows_process.segment_register_p (r)) - { - /* GDB treats segment registers as 32bit registers, but they are - in fact only 16 bits long. Make sure we do not read extra - bits from our source buffer. */ - long l = *((long *) context_offset) & 0xffff; - regcache->raw_supply (r, (char *) &l); + regcache->raw_supply (r, &l); } else { @@ -799,7 +795,29 @@ windows_store_one_register (const struct regcache *regcache, context_ptr = (char *) &th->wow64_context; #endif - regcache->raw_collect (r, context_ptr + windows_process.mappings[r]); + struct gdbarch *gdbarch = regcache->arch (); + i386_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); + + /* GDB treats some registers as 32-bit, where they are in fact only + 16 bits long. These cases must be handled specially to avoid + overwriting other registers in the context. */ + if (r == I387_FISEG_REGNUM (tdep) || windows_process.segment_register_p (r)) + { + gdb_byte bytes[4]; + regcache->raw_collect (r, bytes); + memcpy (context_ptr + windows_process.mappings[r], bytes, 2); + } + else if (r == I387_FOP_REGNUM (tdep)) + { + gdb_byte bytes[4]; + regcache->raw_collect (r, bytes); + /* The value of FOP occupies the top two bytes in the context, + so write the two low-order bytes from the cache into the + appropriate spot. */ + memcpy (context_ptr + windows_process.mappings[r] + 2, bytes, 2); + } + else + regcache->raw_collect (r, context_ptr + windows_process.mappings[r]); } /* Store a new register value into the context of the thread tied to diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc index 783e61f6f33..44490300b69 100644 --- a/gdbserver/win32-i386-low.cc +++ b/gdbserver/win32-i386-low.cc @@ -32,8 +32,17 @@ using namespace windows_nat; #define CONTEXT_EXTENDED_REGISTERS 0 #endif -#define FCS_REGNUM 27 -#define FOP_REGNUM 31 +#define I386_FISEG_REGNUM 27 +#define I386_FOP_REGNUM 31 + +#define I386_CS_REGNUM 10 +#define I386_GS_REGNUM 15 + +#define AMD64_FISEG_REGNUM 35 +#define AMD64_FOP_REGNUM 39 + +#define AMD64_CS_REGNUM 18 +#define AMD64_GS_REGNUM 23 #define FLAG_TRACE_BIT 0x100 @@ -459,6 +468,42 @@ static const int amd64_mappings[] = #endif /* __x86_64__ */ +/* Return true if R is the FISEG register. */ +static bool +is_fiseg_register (int r) +{ +#ifdef __x86_64__ + if (!windows_process.wow64_process) + return r == AMD64_FISEG_REGNUM; + else +#endif + return r == I386_FISEG_REGNUM; +} + +/* Return true if R is the FOP register. */ +static bool +is_fop_register (int r) +{ +#ifdef __x86_64__ + if (!windows_process.wow64_process) + return r == AMD64_FOP_REGNUM; + else +#endif + return r == I386_FOP_REGNUM; +} + +/* Return true if R is a segment register. */ +static bool +is_segment_register (int r) +{ +#ifdef __x86_64__ + if (!windows_process.wow64_process) + return r >= AMD64_CS_REGNUM && r <= AMD64_GS_REGNUM; + else +#endif + return r >= I386_CS_REGNUM && r <= I386_GS_REGNUM; +} + /* Fetch register from gdbserver regcache data. */ static void i386_fetch_inferior_register (struct regcache *regcache, @@ -480,15 +525,18 @@ i386_fetch_inferior_register (struct regcache *regcache, #endif context_offset = (char *) &th->context + mappings[r]; - long l; - if (r == FCS_REGNUM) + /* GDB treats some registers as 32-bit, where they are in fact only + 16 bits long. These cases must be handled specially to avoid + reading extraneous bits from the context. */ + if (is_fiseg_register (r) || is_segment_register (r)) { - l = *((long *) context_offset) & 0xffff; - supply_register (regcache, r, (char *) &l); + gdb_byte bytes[4] = {}; + memcpy (bytes, context_offset, 2); + supply_register (regcache, r, bytes); } - else if (r == FOP_REGNUM) + else if (is_fop_register (r)) { - l = (*((long *) context_offset) >> 16) & ((1 << 11) - 1); + long l = (*((long *) context_offset) >> 16) & ((1 << 11) - 1); supply_register (regcache, r, (char *) &l); } else @@ -516,7 +564,26 @@ i386_store_inferior_register (struct regcache *regcache, #endif context_offset = (char *) &th->context + mappings[r]; - collect_register (regcache, r, context_offset); + /* GDB treats some registers as 32-bit, where they are in fact only + 16 bits long. These cases must be handled specially to avoid + overwriting other registers in the context. */ + if (is_fiseg_register (r) || is_segment_register (r)) + { + gdb_byte bytes[4]; + collect_register (regcache, r, bytes); + memcpy (context_offset, bytes, 2); + } + else if (is_fop_register (r)) + { + gdb_byte bytes[4]; + collect_register (regcache, r, bytes); + /* The value of FOP occupies the top two bytes in the context, + so write the two low-order bytes from the cache into the + appropriate spot. */ + memcpy (context_offset + 2, bytes, 2); + } + else + collect_register (regcache, r, context_offset); } static const unsigned char i386_win32_breakpoint = 0xcc;