[PATCH] Fix comparison of unsigned long int to int in record_linux_system_call.

[PATCH] Fix comparison of unsigned long int to int in record_linux_system_call.

[Carl Love]

GDB maintainers:

The record_linux_system_call() function has an if/elseif statement to
check for each of the various ioctl values.  The local variable 
tmpulongest is declared as an unsigned long int.  The if statement
compares the variable to the ioctl_* variables which are declared as
ints.  The issue on PowerPC is some of the ioctls have the most
significant bit set to 1.  The comparison of the unsigned long int to
an int fails.  The ioctl_* (for example ioctl_TCSETSW) values should be declared as unsigned values.  This patch changes the declaration for the ioctl_* values to unsigned long ints to match the declaration of the tmpulongest variable.

The patch has been tested and verified on Power 10 and Intel.

Please let me know if this patch is acceptable for mainline.

                Carl Love

--------------------------------------------------

Fix comparison of unsigned long int to int in record_linux_system_call.

The if statement in case gdb_sys_ioctl in function
record_linux_system_call in file gdb/linux-record.c is as follows:

   if (tmpulongest == tdep->ioctl_FIOCLEX
      || tmpulongest == tdep->ioctl_FIONCLEX
    ....
      || tmpulongest == tdep->ioctl_TCSETSW
     ...
   }

The PowerPC ioctl value for ioctl_TCSETW is 0x802c7415.  The variable
ioctl_TCSETW is defined in gdb/linux-record.h as an int.  The TCSETW value
has the MSB set to one so it is a negative integer.  The comparison of the
unsigned long value tmpulongest to a negative integer value for
ioctl_TCSETSW fails.

This patch changes the declarations for the ioctl_* values in struct
linux_record_tdep to unsigned long to fix the comparisons between
tmpulongest and the tdep->ioctl_* values.

An additional test gdb.reverse/test_ioctl_TCSETSW.exp is added to verify
the gdb record_linux_system_call() if statement for the ioctl TCSETSW
succeeds.

This patch has been tested on Power 10 and Intel with no test failures.
---
 gdb/linux-record.h                            | 130 +++++++++---------
 .../gdb.reverse/test_ioctl_TCSETSW.c          |  38 +++++
 .../gdb.reverse/test_ioctl_TCSETSW.exp        |  45 ++++++
 3 files changed, 148 insertions(+), 65 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.c
 create mode 100644 gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.exp

diff --git a/gdb/linux-record.h b/gdb/linux-record.h
index 219c67f888d..39d7d4b54d0 100644
--- a/gdb/linux-record.h
+++ b/gdb/linux-record.h
@@ -92,71 +92,71 @@ struct linux_record_tdep
   int size_time_t;
 
   /* The values of the second argument of system call "sys_ioctl".  */
-  int ioctl_TCGETS;
-  int ioctl_TCSETS;
-  int ioctl_TCSETSW;
-  int ioctl_TCSETSF;
-  int ioctl_TCGETA;
-  int ioctl_TCSETA;
-  int ioctl_TCSETAW;
-  int ioctl_TCSETAF;
-  int ioctl_TCSBRK;
-  int ioctl_TCXONC;
-  int ioctl_TCFLSH;
-  int ioctl_TIOCEXCL;
-  int ioctl_TIOCNXCL;
-  int ioctl_TIOCSCTTY;
-  int ioctl_TIOCGPGRP;
-  int ioctl_TIOCSPGRP;
-  int ioctl_TIOCOUTQ;
-  int ioctl_TIOCSTI;
-  int ioctl_TIOCGWINSZ;
-  int ioctl_TIOCSWINSZ;
-  int ioctl_TIOCMGET;
-  int ioctl_TIOCMBIS;
-  int ioctl_TIOCMBIC;
-  int ioctl_TIOCMSET;
-  int ioctl_TIOCGSOFTCAR;
-  int ioctl_TIOCSSOFTCAR;
-  int ioctl_FIONREAD;
-  int ioctl_TIOCINQ;
-  int ioctl_TIOCLINUX;
-  int ioctl_TIOCCONS;
-  int ioctl_TIOCGSERIAL;
-  int ioctl_TIOCSSERIAL;
-  int ioctl_TIOCPKT;
-  int ioctl_FIONBIO;
-  int ioctl_TIOCNOTTY;
-  int ioctl_TIOCSETD;
-  int ioctl_TIOCGETD;
-  int ioctl_TCSBRKP;
-  int ioctl_TIOCTTYGSTRUCT;
-  int ioctl_TIOCSBRK;
-  int ioctl_TIOCCBRK;
-  int ioctl_TIOCGSID;
-  int ioctl_TCGETS2;
-  int ioctl_TCSETS2;
-  int ioctl_TCSETSW2;
-  int ioctl_TCSETSF2;
-  int ioctl_TIOCGPTN;
-  int ioctl_TIOCSPTLCK;
-  int ioctl_FIONCLEX;
-  int ioctl_FIOCLEX;
-  int ioctl_FIOASYNC;
-  int ioctl_TIOCSERCONFIG;
-  int ioctl_TIOCSERGWILD;
-  int ioctl_TIOCSERSWILD;
-  int ioctl_TIOCGLCKTRMIOS;
-  int ioctl_TIOCSLCKTRMIOS;
-  int ioctl_TIOCSERGSTRUCT;
-  int ioctl_TIOCSERGETLSR;
-  int ioctl_TIOCSERGETMULTI;
-  int ioctl_TIOCSERSETMULTI;
-  int ioctl_TIOCMIWAIT;
-  int ioctl_TIOCGICOUNT;
-  int ioctl_TIOCGHAYESESP;
-  int ioctl_TIOCSHAYESESP;
-  int ioctl_FIOQSIZE;
+  ULONGEST ioctl_TCGETS;
+  ULONGEST ioctl_TCSETS;
+  ULONGEST ioctl_TCSETSW;
+  ULONGEST ioctl_TCSETSF;
+  ULONGEST ioctl_TCGETA;
+  ULONGEST ioctl_TCSETA;
+  ULONGEST ioctl_TCSETAW;
+  ULONGEST ioctl_TCSETAF;
+  ULONGEST ioctl_TCSBRK;
+  ULONGEST ioctl_TCXONC;
+  ULONGEST ioctl_TCFLSH;
+  ULONGEST ioctl_TIOCEXCL;
+  ULONGEST ioctl_TIOCNXCL;
+  ULONGEST ioctl_TIOCSCTTY;
+  ULONGEST ioctl_TIOCGPGRP;
+  ULONGEST ioctl_TIOCSPGRP;
+  ULONGEST ioctl_TIOCOUTQ;
+  ULONGEST ioctl_TIOCSTI;
+  ULONGEST ioctl_TIOCGWINSZ;
+  ULONGEST ioctl_TIOCSWINSZ;
+  ULONGEST ioctl_TIOCMGET;
+  ULONGEST ioctl_TIOCMBIS;
+  ULONGEST ioctl_TIOCMBIC;
+  ULONGEST ioctl_TIOCMSET;
+  ULONGEST ioctl_TIOCGSOFTCAR;
+  ULONGEST ioctl_TIOCSSOFTCAR;
+  ULONGEST ioctl_FIONREAD;
+  ULONGEST ioctl_TIOCINQ;
+  ULONGEST ioctl_TIOCLINUX;
+  ULONGEST ioctl_TIOCCONS;
+  ULONGEST ioctl_TIOCGSERIAL;
+  ULONGEST ioctl_TIOCSSERIAL;
+  ULONGEST ioctl_TIOCPKT;
+  ULONGEST ioctl_FIONBIO;
+  ULONGEST ioctl_TIOCNOTTY;
+  ULONGEST ioctl_TIOCSETD;
+  ULONGEST ioctl_TIOCGETD;
+  ULONGEST ioctl_TCSBRKP;
+  ULONGEST ioctl_TIOCTTYGSTRUCT;
+  ULONGEST ioctl_TIOCSBRK;
+  ULONGEST ioctl_TIOCCBRK;
+  ULONGEST ioctl_TIOCGSID;
+  ULONGEST ioctl_TCGETS2;
+  ULONGEST ioctl_TCSETS2;
+  ULONGEST ioctl_TCSETSW2;
+  ULONGEST ioctl_TCSETSF2;
+  ULONGEST ioctl_TIOCGPTN;
+  ULONGEST ioctl_TIOCSPTLCK;
+  ULONGEST ioctl_FIONCLEX;
+  ULONGEST ioctl_FIOCLEX;
+  ULONGEST ioctl_FIOASYNC;
+  ULONGEST ioctl_TIOCSERCONFIG;
+  ULONGEST ioctl_TIOCSERGWILD;
+  ULONGEST ioctl_TIOCSERSWILD;
+  ULONGEST ioctl_TIOCGLCKTRMIOS;
+  ULONGEST ioctl_TIOCSLCKTRMIOS;
+  ULONGEST ioctl_TIOCSERGSTRUCT;
+  ULONGEST ioctl_TIOCSERGETLSR;
+  ULONGEST ioctl_TIOCSERGETMULTI;
+  ULONGEST ioctl_TIOCSERSETMULTI;
+  ULONGEST ioctl_TIOCMIWAIT;
+  ULONGEST ioctl_TIOCGICOUNT;
+  ULONGEST ioctl_TIOCGHAYESESP;
+  ULONGEST ioctl_TIOCSHAYESESP;
+  ULONGEST ioctl_FIOQSIZE;
 
   /* The values of the second argument of system call "sys_fcntl"
      and "sys_fcntl64".  */
diff --git a/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.c b/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.c
new file mode 100644
index 00000000000..6365f968b30
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2022 Free Software Foundation, Inc.
+
+   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 <http://www.gnu.org/licenses/>.  */
+
+#include <sys/ioctl.h>
+#include <termios.h>
+#include <stdio.h>
+
+/* The purpose of this test is to verify gdb record_linux_system_call()
+   recognizes the call for ioctl TCSETSW.  */
+
+int
+main(void)
+{
+
+  struct termios term;
+  int result;
+  int fd = 0;
+
+  /* The test just needs to generate an ioctl call for TCSETSW to see if gdb
+     record detected it or not.  Success or failure of the ioctl call is
+     irrelevant.  */
+  result = tcsetattr(fd, TCSADRAIN, &term);   /* TCSETSW call */
+  result = 0;   /* TCSETSW called */
+}
diff --git a/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.exp b/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.exp
new file mode 100644
index 00000000000..86a62ebe5e5
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/test_ioctl_TCSETSW.exp
@@ -0,0 +1,45 @@
+# Copyright 2008-2022 Free Software Foundation, Inc.
+
+# 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 <http://www.gnu.org/licenses/>.
+#
+# Test ioctl TCSETSW record for PowerPC.
+#
+
+standard_testfile .c
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] then {
+    untested "could not run to main"
+    continue
+}
+
+# Recording of ioctls calls requires record full
+gdb_test_no_output "record full"
+
+set stop [gdb_get_line_number "TCSETSW call"]
+gdb_test "break $stop" ".*Breakpoint .*" "stop at TCSETSW"
+gdb_test "continue"  ".*Breakpoint .*" "at TCSETSW call"
+
+set test "handle TCSETSW"
+gdb_test_multiple "step"  $test {
+    -re "Process record and replay target doesn't support ioctl request 0x.*$gdb_prompt $" {
+	fail $test
+    }
+    -re ".*result = 0.*$gdb_prompt $" {
+	pass $test
+    }
+}
-- 
2.31.1

Re: [PATCH] Fix comparison of unsigned long int to int in record_linux_system_call.

[Ulrich Weigand]

Carl Love <cel@us.ibm.com> wrote:

>Fix comparison of unsigned long int to int in
record_linux_system_call.

This is OK.


Thanks,
Ulrich