From patchwork Mon Feb 26 20:44:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 86401 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 1F2D93858410 for ; Mon, 26 Feb 2024 20:45:15 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by sourceware.org (Postfix) with ESMTPS id 575DB3858C98 for ; Mon, 26 Feb 2024 20:44:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 575DB3858C98 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 575DB3858C98 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.49 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708980293; cv=none; b=vcqI2HuyUsWLyhBkUgSN1oiNRYumbolQdPfQCaGeNzD50Tp0Bp61fGL7ZxQ5gHLmv+QRXf0vXp9aaRU29yEtJoD47UEZ0SBbQY2UPqA0nPEvukBePh0MVpkEv87Ic7bkssl0EO4sZwMeG5RGeJnByw34oG5+QeiLjRjtHNkC7WY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708980293; c=relaxed/simple; bh=UN7qFBWzNl+hatAu3BzfsVtQXaB1Wg4I76G1x48mqRs=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=mwMwRamtcD8gMj1g+q6h0cB5e47gk0jWzuNIfm5Nuprxx7kk7CncDuIFMNMK1B6eu35Cx+LG749gQrDT5RRIOEEbdC6Wji7V6TFBs4z0cPx8UIHzW9r77YdiGzAdT1xtAKnyYh512VDENqY2eXd1sJvsqFHSazhW+KnjBeKvKRk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-33ddd1624beso668073f8f.1 for ; Mon, 26 Feb 2024 12:44:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708980289; x=1709585089; 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=i/MTbe+s/PHLj//b7qnAdR37ZNsk1G4+kKxO7l+/4TM=; b=kKsje3y3y0jt4J7XuhUwCQ5hQBUsFgNQKTYCo6TmKyy4tNw/T2TpxlaWN2q8aIVAGE ojJF7uDnJtxlHx+3mtQPSfgshwKVoAeLDGk5iZ/IQxDcd3fnbjuGsNeLU8ysJmrQe7jd TCqM7qmOjFfGApI/F2tWTfDqH9EaS3ST47WX/ZmIXNbXuCHct9CV10ZW6i98qJ0z7GP5 ybCFLTU65DyTbMHp+byPv5Ij8O9TGDqxbr+Iub2UuMVlMqpDM5m/y5dGAJI+/6ySK7qG lsRTOqpI5LDJXgJyWxSxMrrx6Xy5OVTHEOUD6296pNxaTHVpYNFvpmrmnUE2dCNRPjGh BxjQ== X-Gm-Message-State: AOJu0YxLHdwE6dhKTIFdviw3M8UjfrvqhTor9bTSF1qVbV/gj1EXTMRa 6LobKGnVkvSfHDBq9w0AoEU+ebztMPqCOc6LCBhwncod3XNBL6oPzMBPYsT0zNE= X-Google-Smtp-Source: AGHT+IE/s6KllalWe1GQvcw7eirwCZQKldr3CHebZwN8UcXTUoeLgrVhGRskN7Hw8q5q9D7wAzWAuw== X-Received: by 2002:adf:f541:0:b0:33d:d793:a20f with SMTP id j1-20020adff541000000b0033dd793a20fmr3226836wrp.27.1708980288227; Mon, 26 Feb 2024 12:44:48 -0800 (PST) Received: from localhost ([2001:8a0:f918:ab00:d236:4001:3b41:8ce3]) by smtp.gmail.com with UTF8SMTPSA id w10-20020adff9ca000000b0033d1b760125sm9340406wrr.92.2024.02.26.12.44.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 26 Feb 2024 12:44:47 -0800 (PST) From: Pedro Alves To: gdb-patches@sourceware.org Cc: Takashi Yano Subject: [PATCH] Cygwin: Fix putting inferior in foreground (fix input) Date: Mon, 26 Feb 2024 20:44:45 +0000 Message-ID: <20240226204445.1324953-1-pedro@palves.net> X-Mailer: git-send-email 2.43.2 MIME-Version: 1.0 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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 From: Takashi Yano gdb.base/interrupt.exp reveals that inferior input is broken on Cygwin: (gdb) continue Continuing. talk to me baby Input/output error <<< BAD PASS: gdb.base/interrupt.exp: process is alive a [Thread 10688.0x2590 exited with code 1] [Thread 10688.0x248c exited with code 1] [Thread 10688.0x930 exited with code 1] [Thread 10688.0x2c98 exited with code 1] Program terminated with signal SIGHUP, Hangup. The program no longer exists. (gdb) FAIL: gdb.base/interrupt.exp: child process ate our char a Ambiguous command "a": actions, add-auto-load-safe-path, add-auto-load-scripts-directory, add-inferior... (gdb) ERROR: "" is not a unique command name. The problem is that inflow.c:child_terminal_inferior is failing to put the inferior in the foreground, because we're passing down the inferior's Windows PID instead of the Cygwin PID to Cygwin tcsetpgrp. That is fixed by this commit. Afterwards we will get: (gdb) continue Continuing. talk to me baby PASS: gdb.base/interrupt.exp: process is alive a a <<< GOOD PASS: gdb.base/interrupt.exp: child process ate our char [New Thread 7236.0x1c58] Thread 6 received signal SIGINT, Interrupt. <<< new thread spawned for SIGINT [Switching to Thread 7236.0x1c58] 0x00007ffa6643ea6b in TlsGetValue () from /cygdrive/c/Windows/System32/KERNELBASE.dll (gdb) FAIL: gdb.base/interrupt.exp: send_gdb control C We still have the FAIL seen above because this change has another consequence. By failing to put the inferior in the foreground correctly, Ctrl-C was always reaching GDB first. Now that the inferior is put in the foreground properly, Ctrl-C reaches the inferior first, which results in a Windows Ctrl-C event, which results in Windows injecting a new thread in the inferior to report the Ctrl-C exception => SIGINT. That is, running the test manually: Before patch: (gdb) c Continuing. [New Thread 2352.0x1f5c] [New Thread 2352.0x1988] [New Thread 2352.0x18cc] <<< Ctrl-C pressed. Thread 7 received signal SIGTRAP, Trace/breakpoint trap. [Switching to Thread 2352.0x18cc] 0x00007ffa68930b11 in ntdll!DbgBreakPoint () from /cygdrive/c/Windows/SYSTEM32/ntdll.dll (gdb) Above, GDB got the SIGINT, and it manually passes it down the inferior, which reaches windows_nat_target::interrupt(), which interrupts the inferior with DebugBreakProcess (which injects a new thread in the inferior that executes an int3 instruction). After this patch, we have (with "set debugexceptions on" so DBG_CONTROL_C is visible): (gdb) c Continuing. [New Thread 9940.0x1168] [New Thread 9940.0x5f8] gdb: Target exception MS_VC_EXCEPTION at 0x7ffa6638cf19 gdb: Target exception MS_VC_EXCEPTION at 0x7ffa6638cf19 [New Thread 9940.0x3d8] gdb: Target exception DBG_CONTROL_C at 0x7ffa6643ea6b <<< Ctrl-C Thread 7 received signal SIGINT, Interrupt. <<< new injected thread [Switching to Thread 9940.0x3d8] 0x00007ffa6643ea6b in TlsGetValue () from /cygdrive/c/Windows/System32/KERNELBASE.dll (gdb) This new behavior is exactly the same as what you see with a MinGW GDB build. Also, SIGINT reaching inferior first is what you get on Linux as well currently. I wrote an initial fix for this before I discovered that Cygwin downstream had a similar change, so I then combined the patches. I am adding a Co-Authored-By for that reason. Co-Authored-By: Takashi Yano Change-Id: I3a8c3355784c6a817dbd345ba9dac24be06c4b3f --- gdb/inflow.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) base-commit: 3d2d21728b6db4430ff168ee27e12fc6e2627fad diff --git a/gdb/inflow.c b/gdb/inflow.c index a413957a1dd..3dd70b97fe5 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -40,6 +40,10 @@ #include #endif +#ifdef __CYGWIN__ +#include +#endif + #ifndef O_NOCTTY #define O_NOCTTY 0 #endif @@ -345,11 +349,25 @@ child_terminal_inferior (struct target_ops *self) then restore whatever was the foreground pgrp the last time the inferior was running. See also comments describing terminal_state::process_group. */ -#ifdef HAVE_GETPGID - result = tcsetpgrp (0, getpgid (inf->pid)); -#else - result = tcsetpgrp (0, tinfo->process_group); + pid_t pgrp = tinfo->process_group; +#ifdef __CYGWIN__ + /* The Windows native target uses Win32 routines to run or + attach to processes (CreateProcess / DebugActiveProcess), + so a Cygwin inferior has a Windows PID, rather than a + Cygwin PID. We want to pass the Cygwin PID to Cygwin + tcsetpgrp if we have a Cygwin inferior, so try to convert + first. If we have a non-Cygwin inferior, we'll end up + passing down the WINPID to tcsetpgrp, stored in + terminal_state::process_group. tcsetpgrp still succeeds + in that case, and it seems preferable to switch the + foreground pgrp away from GDB, for consistency. */ + pid_t cygpid = cygwin_internal (CW_WINPID_TO_CYGWIN_PID, inf->pid); + if (cygpid <= cygwin_internal (CW_MAX_CYGWIN_PID)) + pgrp = getpgid (cygpid); +#elif defined (HAVE_GETPGID) + pgrp = getpgid (inf->pid); #endif + result = tcsetpgrp (0, pgrp); if (result == -1) { #if 0