RFA: fix PR 7286

RFA: fix PR 7286

[Tom Tromey]

PR 7286 concerns the type of certain decimal integer constants.

According to C99, such a constant is never unsigned, but instead is of
the next wider type (from int, long, and long long) which can
represent its value.

I've included a rather roundabout test case.  I think this particular
boundary condition can only be tested where LONGEST is wider than the
target's word size.  As LONGEST is a define, and not a typedef, the
test determines the size of LONGEST by using a field whose type is
unlikely to change.

Another oddity here is the interpretation of the most negative 'long
long'.  I chose to leave this as unsigned, though it would also make
sense to make this an overflow error.  The latter seemed excessively
pedantic, though I don't feel strongly about this.

Please review.

thanks,
Tom

2009-01-05  Tom Tromey  <tromey@redhat.com>

	PR cli/7286:
	* c-exp.y (parse_number): Handle large decimal constants according
	to C99.

2009-01-05  Tom Tromey  <tromey@redhat.com>

	* gdb.gdb/selftest.exp (test_int_parsing): New proc.
	(test_with_self): Call it.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index d4bbbcc..dbb390e 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1308,18 +1308,19 @@ parse_number (p, len, parsed_float, putithere)
      the case where it is we just always shift the value more than
      once, with fewer bits each time.  */
 
-  un = (ULONGEST)n >> 2;
+  /* A large decimal (not hex or octal) constant (between INT_MAX and
+     UINT_MAX) uses the smallest type that can hold its value.  So,
+     for decimal values, we shift one fewer bit, so that a value that
+     would overflow into the sign bit uses the next higher type.  */
+  if (base == 10)
+    un = (ULONGEST)n >> 1;
+  else
+    un = (ULONGEST)n >> 2;
   if (long_p == 0
       && (un >> (gdbarch_int_bit (parse_gdbarch) - 2)) == 0)
     {
       high_bit = ((ULONGEST)1) << (gdbarch_int_bit (parse_gdbarch) - 1);
 
-      /* A large decimal (not hex or octal) constant (between INT_MAX
-	 and UINT_MAX) is a long or unsigned long, according to ANSI,
-	 never an unsigned int, but this code treats it as unsigned
-	 int.  This probably should be fixed.  GCC gives a warning on
-	 such constants.  */
-
       unsigned_type = parse_type->builtin_unsigned_int;
       signed_type = parse_type->builtin_int;
     }
diff --git a/gdb/testsuite/gdb.gdb/selftest.exp b/gdb/testsuite/gdb.gdb/selftest.exp
index 495ae45..4c9520e 100644
--- a/gdb/testsuite/gdb.gdb/selftest.exp
+++ b/gdb/testsuite/gdb.gdb/selftest.exp
@@ -247,6 +247,54 @@ proc do_steps_and_nexts {} {
     }
 }
 
+proc test_int_parsing {} {
+    global gdb_prompt expect_out
+
+    # Try to extract the size of LONGEST.  If LONGEST is wider than a
+    # pointer, then we can test a boundary case of C integer constant
+    # parsing without overflow.
+    gdb_test_multiple "print sizeof (((union exp_element *) 0)->longconst)" \
+      "determine size of LONGEST" {
+	  -re ".* = \(\[0-9\]+\).*$gdb_prompt $" {
+	      set longest_size $expect_out(1,string)
+	  }
+	  -re ".*$gdb_prompt $" {
+	      # Nothing.
+	  }
+      }
+    # Always pass.  If this fails, we don't want to hear about it.
+    pass "determine size of LONGEST"
+    if {! [info exists longest_size]} {
+	return
+    }
+
+    gdb_test_multiple "print sizeof (void *)" "determine size of void*" {
+	  -re ".* = \(\[0-9\]+\).*$gdb_prompt $" {
+	      set voidp_size $expect_out(1,string)
+	  }
+	  -re ".*$gdb_prompt $" {
+	      # Nothing.
+	  }
+    }
+    # Always pass.  If this fails, we don't want to hear about it.
+    pass "determine size of void*"
+    if {! [info exists voidp_size]} {
+	return
+    }
+
+    if {$longest_size <= $voidp_size} {
+	return
+    }
+
+    if {$voidp_size == 4} {
+	# 32-bit machine.
+	gdb_test "print -2147483648" " = -2147483648"
+    } elseif {$voidp_size == 8} {
+	# 64-bit machine.
+	gdb_test "print -9223372036854775808" " = -9223372036854775808"
+    }
+}
+
 proc test_with_self { executable } {
     global gdb_prompt
     global tool
@@ -279,6 +327,8 @@ proc test_with_self { executable } {
 	return -1
     }
 
+    test_int_parsing
+
     if { $gdb_file_cmd_debug_info != "debug" } then {
 	untested "No debug information, skipping testcase."
 	return -1

Re: RFA: fix PR 7286

[Joseph S. Myers]

On Mon, 5 Jan 2009, Tom Tromey wrote:

> PR 7286 concerns the type of certain decimal integer constants.
> 
> According to C99, such a constant is never unsigned, but instead is of
> the next wider type (from int, long, and long long) which can
> represent its value.

Note that this is something that changed in C99; in C90 such constants 
(wider than signed long) would be unsigned.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: RFA: fix PR 7286

[Tom Tromey]

>>>>> "Joseph" == Joseph S Myers <joseph@codesourcery.com> writes:

>> According to C99, such a constant is never unsigned, but instead is of
>> the next wider type (from int, long, and long long) which can
>> represent its value.

Joseph> Note that this is something that changed in C99; in C90 such constants 
Joseph> (wider than signed long) would be unsigned.

Thanks.

This does raise the question of what C variant gdb targets, or ought
to target.  We could even have them all, with "set lang c99".  The
same question arises for C++.

I think if we are going to have a single variant for a given language,
then gdb ought to follow the most recently published standard.

Tom

Re: RFA: fix PR 7286

[Joseph S. Myers]

On Mon, 5 Jan 2009, Tom Tromey wrote:

> This does raise the question of what C variant gdb targets, or ought
> to target.  We could even have them all, with "set lang c99".  The
> same question arises for C++.

You could also in principle detect C99 translation units with DW_LANG_C99, 
except that I think GCC uses DW_LANG_C89 at present for all C code, 
whatever mode it is compiled in.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: RFA: fix PR 7286

[Tom Tromey]

>>>>> "Joseph" == Joseph S Myers <joseph@codesourcery.com> writes:

Joseph> You could also in principle detect C99 translation units with
Joseph> DW_LANG_C99, except that I think GCC uses DW_LANG_C89 at
Joseph> present for all C code, whatever mode it is compiled in.

I filed a gcc PR for this.

I wonder whether we will also want to whether GNU C was used, and also
whether we will want to know the particular C++ dialect.

Tom