Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Aditya Vidyadhar Kamath <akamath996@gmail.com>,
	ulrich.weigand@de.ibm.com,  tom@tromey.com
Cc: gdb-patches@sourceware.org, Aditya.Kamath1@ibm.com,
	sangamesh.swamy@in.ibm.com
Subject: Re: [PATCH v2] Accept ksh88 exit status for SIGINT in shell.exp
Date: Wed, 21 Oct 2026 11:27:01 -0400	[thread overview]
Message-ID: <a5285a92-60ca-4f3b-a440-ea0c4a9f0ba5@polymtl.ca> (raw)
In-Reply-To: <20261021091128.28234-2-akamath996@gmail.com>

On 10/21/26 5:11 AM, Aditya Vidyadhar Kamath wrote:
> From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
> 
> This patch is in continuation to Simon's patch [1].
> 
> gdb.base/shell.exp runs "kill -2 $$" under the shell, through both
> "shell" ("!") and "pipe" ("|"), and expects $_shell_exitsignal to be 2
> and $_shell_exitcode to be void, that is, a shell that dies from the
> signal.
> 
> On AIX, /bin/sh and /usr/bin/ksh are ksh88.  ksh88 catches SIGINT and
> calls exit (130) (128 + SIGINT) instead of dying from the signal.  GDB
> therefore sees a normal exit, sets $_shell_exitcode to 130 and leaves
> $_shell_exitsignal void.  This is correct reporting by GDB; the test
> assumes a shell that is killed by the signal.  The raw wait status of
> the shells on AIX 7.3 is:
> 
>   /usr/bin/ksh    exited, code 130
>   /usr/bin/ksh93  killed by signal 2
>   /usr/bin/bash   killed by signal 2
> 
> As a result these four tests failed on AIX:
> 
>   shell interrupt exitcode
>   shell interrupt exitsignal
>   pipe interrupt exitcode
>   pipe interrupt exitsignal
> 
> Add check_shell_interrupt_status, which replaces the duplicated checks.
> On AIX it also accepts $_shell_exitcode = 130 together with
> $_shell_exitsignal = void.
> 
> Tested on AIX 7.3 with a 64-bit GDB: gdb.base/shell.exp now gives
> 125 passes and no failures.  The four tests above failed before.

I like the idea of testing the actual shell's behavior to set our
expectations for what GDB should output.  There are just two
problems:

 - Your check uses TCL's exec, which runs on the build machine.  That
   wouldn't be right in a remote-host scenario.  I think we can restrict
   your check to when running with a local host.  This assumes that you
   are not doing remote-host testing with AIX.

 - The shell and pipe commands don't necessary use the same shell.  The
   shell command reads the SHELL env var, but the pipe command uses
   popen, which is hardcoded to use /bin/sh.  I presume that if you run
   your patch with bash as your shell, you'd see a failure for the pipe
   test.

Can you test this patch, which addresses those two problems?


From 8065b5c254e1455be868101cd20c18c2a193170a Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
Date: Wed, 21 Oct 2026 14:41:29 +0530
Subject: [PATCH] Accept ksh88 exit status for SIGINT in shell.exp

gdb.base/shell.exp runs "kill -2 $$" under the shell, through both
"shell" ("!") and "pipe" ("|"), and expects $_shell_exitsignal to be 2
and $_shell_exitcode to be void, that is, a shell that dies from the
signal.

On AIX, /bin/sh and /usr/bin/ksh are ksh88.  ksh88 catches SIGINT and
calls exit (130) (128 + SIGINT) instead of dying from the signal.  GDB
therefore sees a normal exit, sets $_shell_exitcode to 130 and leaves
$_shell_exitsignal void.  This is correct reporting by GDB; the test
assumes a shell that is killed by the signal.  The raw wait status of
the shells on AIX 7.3 is:

  /usr/bin/ksh    exited, code 130
  /usr/bin/ksh93  killed by signal 2
  /usr/bin/bash   killed by signal 2

As a result these four tests failed on AIX:

  shell interrupt exitcode
  shell interrupt exitsignal
  pipe interrupt exitcode
  pipe interrupt exitsignal

Add a shell_dies_from_sigint proc, which runs "kill -2 $$" with a given
shell outside of GDB and looks at the raw wait status.  Add
check_shell_interrupt_status, which replaces the duplicated checks and
expects either the "killed by signal 2" or the "exited with code 130"
outcome, based on what shell_dies_from_sigint found.

The two commands don't necessarily use the same shell.  The "shell"
command uses $SHELL, or /bin/sh if it is not set, while the "pipe"
command uses popen, which always uses /bin/sh.  On AIX with
SHELL=/usr/bin/bash, the first is killed by the signal and the second
exits with code 130.  Check each command against its own shell.

The check uses TCL's exec proc, which runs on the build machine, so skip
it when the host is remote.  If the host is remtoe, expect the shell to
be killed by the signal, as before.

Tested on AIX 7.3 with a 64-bit GDB: gdb.base/shell.exp now gives
125 passes and no failures.  The four tests above failed before.

Change-Id: I2ef115e7095d675baded68595393af8649a9345e
---
 gdb/testsuite/gdb.base/shell.exp | 57 +++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 2e6c63585eb0..de2bfcfe3a26 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -17,6 +17,50 @@

 load_lib completion-support.exp

+# Not every shell reacts to "kill -2 $$" the same way.  Most are killed
+# by the signal, but ksh88 (/bin/sh and /usr/bin/ksh on AIX) catches
+# SIGINT and exits with status 130 (128 + SIGINT) instead.  GDB reports
+# what it sees, so in that case $_shell_exitcode is 130 and
+# $_shell_exitsignal is void.
+#
+# Find out which kind of shell SHELL is by running "kill -2 $$" with it
+# outside of GDB.  Return true if the shell is killed by the signal, and
+# false if it exits with status 130.
+#
+# The check runs on the build machine, so only do it if not using a remote
+# host setup.  If using a remote host setup, assume the "normal" behavior.
+
+proc shell_dies_from_sigint { shell } {
+    if { [is_remote host] } {
+	return 1
+    }
+
+    if { [catch {exec $shell -c {kill -2 $$}} msg opts] } {
+	set errcode [dict get $opts -errorcode]
+	if { [lindex $errcode 0] eq "CHILDSTATUS"
+	     && [lindex $errcode 2] == 130 } {
+	    return 0
+	}
+    }
+
+    return 1
+}
+
+# Check the convenience variables after a shell command that ran
+# "kill -2 $$".  PREFIX is "shell" or "pipe", and is used to name the
+# tests.  SHELL is the shell that GDB used to run the command.
+
+proc check_shell_interrupt_status { prefix shell } {
+    if { [shell_dies_from_sigint $shell] } {
+	gdb_test "p \$_shell_exitcode" " = void" "$prefix interrupt exitcode"
+	gdb_test "p \$_shell_exitsignal" " = 2" "$prefix interrupt exitsignal"
+    } else {
+	gdb_test "p \$_shell_exitcode" " = 130" "$prefix interrupt exitcode"
+	gdb_test "p \$_shell_exitsignal" " = void" \
+	    "$prefix interrupt exitsignal"
+    }
+}
+
 gdb_exit
 gdb_start

@@ -36,9 +80,14 @@ gdb_test "p \$_shell_exitsignal" " = void" "shell fail exitsignal"

 # This test will not work when the shell is CMD.EXE.
 if { ! [ishost *-*-mingw*] } {
+    # The "shell" command uses $SHELL, or /bin/sh if that is not set.
+    set shell /bin/sh
+    if { [info exists ::env(SHELL)] } {
+	set shell $::env(SHELL)
+    }
+
     gdb_test_no_output "! kill -2 $$"
-    gdb_test "p \$_shell_exitcode" " = void" "shell interrupt exitcode"
-    gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
+    check_shell_interrupt_status "shell" $shell
 }

 # Test the $_shell convenience function.
@@ -130,8 +179,8 @@ gdb_test "p \$_shell_exitsignal" " = void" "pipe fail exitsignal"
 # This test will not work when the shell is CMD.EXE.
 if { ! [ishost *-*-mingw*] } {
     gdb_test "|p 123| kill -2 $$" ""
-    gdb_test "p \$_shell_exitcode" " = void" "pipe interrupt exitcode"
-    gdb_test "p \$_shell_exitsignal" " = 2" "pipe interrupt exitsignal"
+    # The "pipe" command uses popen, which always uses /bin/sh.
+    check_shell_interrupt_status "pipe" /bin/sh
 }

 # Error handling verifications.

base-commit: 13db356ae83f72de0828e11826a256a4a28d17b1
-- 
2.56.0


  reply	other threads:[~2026-10-21 15:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21  9:11 Aditya Vidyadhar Kamath
2026-10-21 15:27 ` Simon Marchi [this message]
2026-10-22  6:41   ` Aditya Kamath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5285a92-60ca-4f3b-a440-ea0c4a9f0ba5@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=Aditya.Kamath1@ibm.com \
    --cc=akamath996@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sangamesh.swamy@in.ibm.com \
    --cc=tom@tromey.com \
    --cc=ulrich.weigand@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox