[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #1 from Rainer Orth <ro at gcc dot gnu.org> 2011-04-29 10:17:20 UTC ---
Created attachment 24143
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24143
assembler output before patch

[Bug middle-end/48819] New: [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

           Summary: [4.7 regression] 350 execution failures in 64-bit
                    libjava testsuite on SPARC
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ro@gcc.gnu.org
                CC: ebotcazou@gcc.gnu.org, rguenth@gcc.gnu.org
              Host: sparc-sun-solaris2.*
            Target: sparc-sun-solaris2.*
             Build: sparc-sun-solaris2.*


Between 20110421 and 20110426, 350 execution failures occured in the 64-bit
libjava testsuite on Solaris 2/SPARC: e.g. libjava.lang/ArrayStore.jar:

* regular output:

java.lang.ArrayIndexOutOfBoundsException
java.lang.ArrayStoreException
java.lang.ArrayIndexOutOfBoundsException
foobar

* broken output:

Exception in thread "main" java.lang.NullPointerException
   at ArrayStore.main(ArrayStore.exe)

A reghunt revealed that this was introduced by this patch:

2011-04-22  Richard Guenther  <rguenther@suse.de>

       * tree.c (build_int_cst): Properly create canonicalized integer
       constants.
       (build_int_cst_type): Remove scary comments.

The miscompilation happens in ArrayStore.jar itself:

* If I run the new ArrayStore.exe with the old libgcj.so, the test fails.

* If I run the old ArrayStore.exe with the new libgcj.so, the test passes.

Comparing ArrayStore.s, I find

---
/var/gcc/reghunt/108330/sparc-sun-solaris2.11/libjava/testsuite/ArrayStore.s   
2011-04-29 11:22:09.104007700 +0200
+++ ArrayStore.s    2011-04-29 11:22:40.711673900 +0200
@@ -1181,17 +1181,17 @@ _CD_ArrayStore:
     .xword    _Utf15
     .xword    _Utf14
     .byte    0
-    .byte    3
     .byte    0
-    .byte    4
     .byte    0
     .byte    0
     .byte    0
     .byte    0
     .byte    0
-    .byte    2
     .byte    0
-    .byte    5
+    .byte    0
+    .byte    0
+    .byte    0
+    .byte    0
     .byte    0
     .byte    0
     .byte    0

I'm attaching the two versions of ArrayStore.s

  Rainer

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #2 from Rainer Orth <ro at gcc dot gnu.org> 2011-04-29 10:18:29 UTC ---
Created attachment 24144
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24144
assembler output after patch

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 10:30:33 UTC ---
Can you check if

Index: tree.c
===================================================================
--- tree.c      (revision 173151)
+++ tree.c      (working copy)
@@ -1019,7 +1019,7 @@ build_int_cst (tree type, HOST_WIDE_INT
 {
   /* Support legacy code.  */
   if (!type)
-    type = integer_type_node;
+    return build_int_cst_wide (integer_type_node, low, low < 0 ? -1 : 0);

   return double_int_to_tree (type, shwi_to_double_int (low));
 }

fixes it?

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #4 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-04-29 10:40:14 UTC ---
> --- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 10:30:33 UTC ---
> Can you check if
>
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 173151)
> +++ tree.c      (working copy)
> @@ -1019,7 +1019,7 @@ build_int_cst (tree type, HOST_WIDE_INT
>  {
>    /* Support legacy code.  */
>    if (!type)
> -    type = integer_type_node;
> +    return build_int_cst_wide (integer_type_node, low, low < 0 ? -1 : 0);
>
>    return double_int_to_tree (type, shwi_to_double_int (low));
>  }
>
> fixes it?

Yep, at least rebuilding jc1 with that patch and rebuilding
ArrayStore.exe fixes the testcase.

Thanks.
        Rainer

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 10:44:54 UTC ---
Issues may arise if build_int_cst is called with NULL_TREE and the value
is using all HOST_WIDE_INT bits (I suppose 64 on sparc-sun-solaris2?) but
integer_type_node has less precision.

Java maintainers, please go over build_int_cst (NULL_TREE, ...) users
and replace NULL_TREE with an appropriate type.  Using size_int (...)
might be a good idea in some places (wtf get_tag_node ...).

I'm not sure what _CD_ArrayStore is.

If the patch helps the following should point to the bug in the Java frontend:

Index: tree.c
===================================================================
--- tree.c      (revision 173151)
+++ tree.c      (working copy)
@@ -1019,7 +1019,12 @@ build_int_cst (tree type, HOST_WIDE_INT
 {
   /* Support legacy code.  */
   if (!type)
-    type = integer_type_node;
+    {
+      double_int a = shwi_to_double_int (low);
+      double_int b = double_int_sext (a, TYPE_PRECISION (integer_type_node));
+      gcc_assert (double_int_equal_p (a, b));
+      return build_int_cst_wide (integer_type_node, low, low < 0 ? -1 : 0);
+    }

   return double_int_to_tree (type, shwi_to_double_int (low));
 }

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #6 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-04-29 12:32:34 UTC ---
> --- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 10:44:54 UTC ---
> Issues may arise if build_int_cst is called with NULL_TREE and the value
> is using all HOST_WIDE_INT bits (I suppose 64 on sparc-sun-solaris2?) but

Indeed.

> I'm not sure what _CD_ArrayStore is.

It's contructed in gcc/java/constants.c (build_constant_data_ref).

> If the patch helps the following should point to the bug in the Java frontend:

I get

In file included from <built-in>:3:0:
ArrayStore.java:51:0: internal compiler error: in build_int_cst, at tree.c:1017

with the following stacktrace:

#0  fancy_abort (file=0x910608 "/var/gcc/reghunt/trunk/gcc/tree.c", line=1017, 
    function=0x912a28 "build_int_cst")
    at /var/gcc/reghunt/trunk/gcc/diagnostic.c:893
#1  0x005fbc18 in build_int_cst (type=0x0, low=562971428257792)
    at /var/gcc/reghunt/trunk/gcc/tree.c:1017
#2  0x000da1cc in build_constants_constructor ()
    at /var/gcc/reghunt/trunk/gcc/java/constants.c:547
#3  0x000b57c8 in make_class_data (type=0xfef33620)
    at /var/gcc/reghunt/trunk/gcc/java/class.c:2059
#4  0x000bba4c in finish_class ()
    at /var/gcc/reghunt/trunk/gcc/java/class.c:2261
#5  0x000eeed0 in parse_class_file ()
    at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:1662
#6  0x000ef34c in parse_zip_file_entries ()
    at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:2103
#7  0x000f021c in java_parse_file ()
    at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:1933
#8  0x00440868 in compile_file () at /var/gcc/reghunt/trunk/gcc/toplev.c:579
#9  do_compile () at /var/gcc/reghunt/trunk/gcc/toplev.c:1913
#10 0x0044137c in toplev_main (argc=22, argv=0xffbff3b4)
    at /var/gcc/reghunt/trunk/gcc/toplev.c:1976
#11 0x000a5744 in _start ()

    Rainer

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> 2011-04-29 12:40:19 UTC ---
On Fri, 29 Apr 2011, ro at CeBiTec dot Uni-Bielefeld.DE wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819
> 
> --- Comment #6 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-04-29 12:32:34 UTC ---
> > --- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 10:44:54 UTC ---
> > Issues may arise if build_int_cst is called with NULL_TREE and the value
> > is using all HOST_WIDE_INT bits (I suppose 64 on sparc-sun-solaris2?) but
> 
> Indeed.
> 
> > I'm not sure what _CD_ArrayStore is.
> 
> It's contructed in gcc/java/constants.c (build_constant_data_ref).
> 
> > If the patch helps the following should point to the bug in the Java frontend:
> 
> I get
> 
> In file included from <built-in>:3:0:
> ArrayStore.java:51:0: internal compiler error: in build_int_cst, at tree.c:1017
> 
> with the following stacktrace:
> 
> #0  fancy_abort (file=0x910608 "/var/gcc/reghunt/trunk/gcc/tree.c", line=1017, 
>     function=0x912a28 "build_int_cst")
>     at /var/gcc/reghunt/trunk/gcc/diagnostic.c:893
> #1  0x005fbc18 in build_int_cst (type=0x0, low=562971428257792)
>     at /var/gcc/reghunt/trunk/gcc/tree.c:1017
> #2  0x000da1cc in build_constants_constructor ()
>     at /var/gcc/reghunt/trunk/gcc/java/constants.c:547
> #3  0x000b57c8 in make_class_data (type=0xfef33620)
>     at /var/gcc/reghunt/trunk/gcc/java/class.c:2059
> #4  0x000bba4c in finish_class ()
>     at /var/gcc/reghunt/trunk/gcc/java/class.c:2261
> #5  0x000eeed0 in parse_class_file ()
>     at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:1662
> #6  0x000ef34c in parse_zip_file_entries ()
>     at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:2103
> #7  0x000f021c in java_parse_file ()
>     at /var/gcc/reghunt/trunk/gcc/java/jcf-parse.c:1933
> #8  0x00440868 in compile_file () at /var/gcc/reghunt/trunk/gcc/toplev.c:579
> #9  do_compile () at /var/gcc/reghunt/trunk/gcc/toplev.c:1913
> #10 0x0044137c in toplev_main (argc=22, argv=0xffbff3b4)
>     at /var/gcc/reghunt/trunk/gcc/toplev.c:1976
> #11 0x000a5744 in _start ()

Which shows

          CONSTRUCTOR_PREPEND_VALUE (d,
                                     fold_convert (ptr_type_node,
                                                   (build_int_cst 
(NULL_TREE,

temp))));

ick.  So, I suppose

         CONSTRUCTOR_PREPEND_VALUE (d, build_int_cst (ptr_type_node, 
temp));

would fix it as well.  Patch is obvious if it works for your testcase.

Richard.

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #8 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-04-29 12:50:00 UTC ---
> ick.  So, I suppose
>
>          CONSTRUCTOR_PREPEND_VALUE (d, build_int_cst (ptr_type_node, 
> temp));
>
> would fix it as well.  Patch is obvious if it works for your testcase.

Indeed it does.  I'll rebuild/retest libjava to make sure all is fine
before installing.

Thanks.
        Rainer

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-04-29 14:17:30 UTC ---
PR48769 looks like a duplicate of this PR. As I said in this pr I am traveling
and have limited testing capabilities.

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #10 from Rainer Orth <ro at gcc dot gnu.org> 2011-04-29 19:36:15 UTC ---
Author: ro
Date: Fri Apr 29 19:36:12 2011
New Revision: 173183

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173183
Log:
2011-04-29  Richard Guenther  <rguenther@suse.de>

    PR middle-end/48819
    * constants.c (build_constants_constructor): Use ptr_type_node for
    temp.

Modified:
    trunk/gcc/java/ChangeLog
    trunk/gcc/java/constants.c

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

--- Comment #11 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-04-29 19:42:13 UTC ---
> --- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-04-29 14:17:30 UTC ---
> PR48769 looks like a duplicate of this PR. As I said in this pr I am traveling
> and have limited testing capabilities.

Indeed, though I can only test 32-bit PowerPC Darwin.  The similarity of
affected tests is appaling.

    Rainer

[Bug middle-end/48819] [4.7 regression] 350 execution failures in 64-bit libjava testsuite on SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48819

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2011-04/msg02354.htm
                   |                            |l
         Resolution|                            |FIXED
   Target Milestone|---                         |4.7.0

--- Comment #12 from Rainer Orth <ro at gcc dot gnu.org> 2011-04-29 19:46:07 UTC ---
Fixed.