Re: libjava Divide_1 and pr6388 fail on 4.2.0 RC3 for several targets

[Ian Lance Taylor <iant at google dot com>]

David Daney <ddaney@avtrex.com> writes:

> Kaz Kojima wrote:
> > Hi,
> >
> > I've noticed that some libjava tests fail for SH on trunk and
> > 4.2.0 RC3.
> >
> > New tests that FAIL:
> >
> > Divide_1 -O3 -findirect-dispatch output - bytecode->native test
> > Divide_1 -O3 output - bytecode->native test
> > Divide_1 -O3 output - source compiled test
> > Divide_1 -findirect-dispatch output - bytecode->native test
> > Divide_1 output - bytecode->native test
> > Divide_1 output - source compiled test
> > pr6388 -O3 -findirect-dispatch output - bytecode->native test
> > pr6388 -O3 output - bytecode->native test
> > pr6388 -O3 output - source compiled test
> > pr6388 -findirect-dispatch output - bytecode->native test
> > pr6388 output - bytecode->native test
> > pr6388 output - source compiled test
> >
> > I see same FAILs in the RC3 testresults for x86_64, hppa, ia64,
> > ppc, sparc and s390 at gcc-testresults, though not for i686.
> > Is this a known issue?
> >
> >
> It is now.
> 
> Same thing on mipsel-unknown-linux-gnu:
> 
> http://gcc.gnu.org/ml/gcc-testresults/2007-05/msg00265.html
> 
> This is a regression from:
> 
> http://gcc.gnu.org/ml/gcc-testresults/2007-04/msg00666.html
> 
> I was hoping that 4.2.0 would be good, but very recently someone broke
> it.  Don't people test for regressions before committing?

This is again my fault.  Very sorry about this.  I ran the gcc
testsuite but I didn't run the libjava testsuite.

This is a bug in C++ code in libjava.  The libjava code assumes that
signed overflow is implementation defined.  The libjava code looks
like this:

  if (num < 0)
    {
      isNeg = true;
      num = -(num);
      if (num < 0)
	{
	  // Must be MIN_VALUE, so handle this special case.
	  // FIXME use 'unsigned jint' for num.

Since signed overflow is undefined, VRP can conclude that if (num <
0), - num is > 0.  My patch let VRP notice this.  So the special case
is never handled.

This patch appears to fix the problem.  I'm running the libjava tests
now.  Does this look OK to the java maintainers for 4.2 branch and
mainline?  Mark, should I commit to 4.2 branch?

Ian


2007-05-07  Ian Lance Taylor  <iant@google.com>

	* java/lang/natString.cc (_Jv_FormatInt): Avoid undefined signed
	overflow.


Index: natString.cc
===================================================================
--- natString.cc	(revision 124337)
+++ natString.cc	(working copy)
@@ -371,11 +371,11 @@ _Jv_FormatInt (jchar* bufend, jint num)
   if (num < 0)
     {
       isNeg = true;
-      num = -(num);
-      if (num < 0)
+      if (num != (jint) -2147483648U)
+	num = -(num);
+      else
 	{
-	  // Must be MIN_VALUE, so handle this special case.
-	  // FIXME use 'unsigned jint' for num.
+	  // Handle special case of MIN_VALUE.
 	  *--ptr = '8';
 	  num = 214748364;
 	}