Re: [PATCH] Make out of range type conversions explicit

[Pedro Alves <pedro@palves.net>]

[-- Attachment #1: Type: text/plain, Size: 3113 bytes --]

On 6/30/20 2:58 PM, Gary Benson via Gdb-patches wrote:
> HI all,
> 
> Clang fails to compile two testcases with the following warning:
> implicit conversion from 'X' to 'Y' changes value from x to y
> [-Wconstant-conversion].  This patch adds casts that make the
> value-changing conversions explicit.

It's helpful if you show the full error.  Like:

gdb compile failed, /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:144:20: warning: 
      implicit conversion from 'int' to 'char' changes value from 162 to -94
      [-Wconstant-conversion]
               11, 162, 17);
                   ^~~
/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:151:16: warning: 
      implicit conversion from 'int' to 'char' changes value from 167 to -89
      [-Wconstant-conversion]
               167,
               ^~~
/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:168:16: warning: 
      implicit conversion from 'int' to 'char' changes value from 167 to -89
      [-Wconstant-conversion]
               167,
               ^~~
3 warnings generated.

                === gdb Summary ===

# of untested testcases         1

Above, I think a better fix would be to change init_string to take
unsigned char parameters, since we're really passing down raw bytes.

The other one is:

~~~~~~
gdb compile failed, /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:14: warning: 
      implicit conversion from 'int' to 'signed char' changes value from 130 to
      -126 [-Wconstant-conversion]
  param_reg (130, 120, 33000, 32000);
  ~~~~~~~~~  ^~~
/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:24: warning: 
      implicit conversion from 'int' to 'short' changes value from 33000 to
      -32536 [-Wconstant-conversion]
  param_reg (130, 120, 33000, 32000);
  ~~~~~~~~~            ^~~~~
2 warnings generated.
WARNING: Prototypes not supported, rebuilding with -DNO_PROTOTYPES
gdb compile failed, /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:14: warning: 
      implicit conversion from 'int' to 'signed char' changes value from 130 to
      -126 [-Wconstant-conversion]
  param_reg (130, 120, 33000, 32000);
  ~~~~~~~~~  ^~~
/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:24: warning: 
      implicit conversion from 'int' to 'short' changes value from 33000 to
      -32536 [-Wconstant-conversion]
  param_reg (130, 120, 33000, 32000);
  ~~~~~~~~~            ^~~~~
2 warnings generated.

                === gdb Summary ===

# of untested testcases         1
~~~~~~

Here, param_reg's prototype is:

 static void param_reg (register signed char pr_char,
 		       register unsigned char pr_uchar,
 		       register short pr_short,
 		       register unsigned short pr_ushort);
 
pr_char and pr_short are signed, so how about just passing
down negative numbers.  That's what the testcase expects
GDB will show:

gdb_test "continue" \
    ".*pr_char=-126.*pr_uchar=120.*pr_short=-32536.*pr_ushort=32000.*bkpt = 1.*" \
    "structs2 continue2"

I think it's best to push fix each testcase in its own commit.

See patches attached.

[-- Attachment #2: 0001-Fix-gdb.base-charset.exp-with-Clang.patch --]
[-- Type: text/x-patch, Size: 2192 bytes --]

From c28e13dcfba073df759db149c70f3512d47b287a Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 2 Jul 2020 15:54:36 +0100
Subject: [PATCH 1/2] Fix gdb.base/charset.exp with Clang

gdb.base/charset.exp fails to run with Clang, because of:

 gdb compile failed, /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:144:20: warning:
       implicit conversion from 'int' to 'char' changes value from 162 to -94
       [-Wconstant-conversion]
		11, 162, 17);
		    ^~~
 /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:151:16: warning:
       implicit conversion from 'int' to 'char' changes value from 167 to -89
       [-Wconstant-conversion]
		167,
		^~~
 /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/charset.c:168:16: warning:
       implicit conversion from 'int' to 'char' changes value from 167 to -89
       [-Wconstant-conversion]
		167,
		^~~
 3 warnings generated.

		 === gdb Summary ===

 # of untested testcases         1

Fix it by changing init_string to take unsigned char parameters.
---
 gdb/testsuite/gdb.base/charset.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/charset.c b/gdb/testsuite/gdb.base/charset.c
index ec4927da515..20d548b1928 100644
--- a/gdb/testsuite/gdb.base/charset.c
+++ b/gdb/testsuite/gdb.base/charset.c
@@ -77,12 +77,21 @@ short short_array[3];
 int int_array[3];
 long long_array[3];
 
+/* These are unsigned char so we can pass down characters >127 without
+   explicit casts or warnings.  */
+
 void
 init_string (char string[],
-             char x,
-             char alert, char backspace, char form_feed,
-             char line_feed, char carriage_return, char horizontal_tab,
-             char vertical_tab, char cent, char misc_ctrl)
+	     unsigned char x,
+	     unsigned char alert,
+	     unsigned char backspace,
+	     unsigned char form_feed,
+	     unsigned char line_feed,
+	     unsigned char carriage_return,
+	     unsigned char horizontal_tab,
+	     unsigned char vertical_tab,
+	     unsigned char cent,
+	     unsigned char misc_ctrl)
 {
   int i;
 

base-commit: c2ecccb33c307faa21f4d2f47348e7346b032d94
-- 
2.14.5


[-- Attachment #3: 0002-Fix-gdb.base-structs2.exp-with-Clang.patch --]
[-- Type: text/x-patch, Size: 1972 bytes --]

From f10f83231bcf196cf80c8214f6460484e330f97d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 2 Jul 2020 19:32:40 +0100
Subject: [PATCH 2/2] Fix gdb.base/structs2.exp with Clang

gdb.base/structs2.exp fails to run with Clang, because of:

 gdb compile failed, /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:14: warning:
       implicit conversion from 'int' to 'signed char' changes value from 130 to
       -126 [-Wconstant-conversion]
   param_reg (130, 120, 33000, 32000);
   ~~~~~~~~~  ^~~
 /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/structs2.c:16:24: warning:
       implicit conversion from 'int' to 'short' changes value from 33000 to
       -32536 [-Wconstant-conversion]
   param_reg (130, 120, 33000, 32000);
   ~~~~~~~~~            ^~~~~
 2 warnings generated.

		 === gdb Summary ===

 # of untested testcases         1

Fix it by passing actual negative numbers.
---
 gdb/testsuite/gdb.base/structs2.c   | 2 +-
 gdb/testsuite/gdb.base/structs2.exp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/structs2.c b/gdb/testsuite/gdb.base/structs2.c
index 7c8be035221..aac7bce8c15 100644
--- a/gdb/testsuite/gdb.base/structs2.c
+++ b/gdb/testsuite/gdb.base/structs2.c
@@ -13,7 +13,7 @@ main ()
 
   bkpt = 0;
   param_reg (120, 130, 32000, 33000);
-  param_reg (130, 120, 33000, 32000);
+  param_reg (-120, 130, -32000, 33000);
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/structs2.exp b/gdb/testsuite/gdb.base/structs2.exp
index 8a7d9c69378..5722be3109c 100644
--- a/gdb/testsuite/gdb.base/structs2.exp
+++ b/gdb/testsuite/gdb.base/structs2.exp
@@ -49,5 +49,5 @@ if [test_compiler_info gcc-3-*] {
   setup_xfail hppa*-* gcc/15860
 }
 gdb_test "continue" \
-    ".*pr_char=-126.*pr_uchar=120.*pr_short=-32536.*pr_ushort=32000.*bkpt = 1.*" \
+    ".*pr_char=-120.*pr_uchar=130.*pr_short=-32000.*pr_ushort=33000.*bkpt = 1.*" \
     "structs2 continue2"
-- 
2.14.5