[Bug java/64044] New: Java emits bogus .class$ decls

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] New: Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

            Bug ID: 64044
           Summary: Java emits bogus .class$ decls
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: java
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
                CC: aph at gcc dot gnu.org

Currently the Java FE is lucky that not all -findirect-dispatch testcases are
miscompiled as gimple-fold.c:get_symbol_constant_value has

      /* Variables declared 'const' without an initializer
         have zero as the initializer if they may not be
         overridden at link or run time.  */
      if (!val
          && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
               || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
        return build_zero_cst (TREE_TYPE (sym));

which for unkown reason avoids to fold the reads from .class$ at the beginning
of all functions to NULL (because the above doesn't allow pointer types).

I ran into this when doing the same during value-numbering but "forgetting"
to paper over the Java FE bug.

The Java FE emits (for example)

 <var_decl 0x7ffff6c6e360 Array_2.class$
    type <pointer_type 0x7ffff6c69a80
        type <record_type 0x7ffff6c69930 java.lang.Class readonly type_4 BLK
            size <integer_cst 0x7ffff6c43858 constant 2240>
            unit size <integer_cst 0x7ffff6c43828 constant 280>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6c69930 fields
<field_decl 0x7ffff6c521c8 D.89>
            pointer_to_this <pointer_type 0x7ffff6c699d8>>
        readonly public unsigned DI
        size <integer_cst 0x7ffff6c25bb8 constant 64>
        unit size <integer_cst 0x7ffff6c25bd0 constant 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff6c69a80
        pointer_to_this <pointer_type 0x7ffff6c7f888>>
    readonly constant addressable public static unsigned ignored DI file
Array_2.java line 4 col 0 size <integer_cst 0x7ffff6c25bb8 64> unit size
<integer_cst 0x7ffff6c25bd0 8>
    align 64 context <record_type 0x7ffff6c5e540 Array_2>
    (mem/u/f/c:DI (symbol_ref:DI ("_ZN7Array_27class$$E") [flags 0x2]
<var_decl 0x7ffff6c6e360 Array_2.class$>) [2 Array_2.class$+0 S8 A64])>

which misses an initializer (it seems to be not initialized in .s either,
thus the flags on it are wrong?  Probably they get initialized by the
runtime?).
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Simple patch to reproduce the issue:

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c   (revision 218005)
+++ gcc/gimple-fold.c   (working copy)
@@ -255,7 +255,8 @@ get_symbol_constant_value (tree sym)
         overridden at link or run time.  */
       if (!val
           && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
-              || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
+             || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))
+             || POINTER_TYPE_P (TREE_TYPE (sym))))
        return build_zero_cst (TREE_TYPE (sym));
     }


it should use && is_gimple_reg_type (TREE_TYPE (sym)) in the end.
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

Andrew Haley <aph at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aph at redhat dot com

--- Comment #2 from Andrew Haley <aph at redhat dot com> ---
So, is the solution to this trivially not to mark the .class$ decls as
TREE_CONST ?
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Suggested fix:

Index: gcc/java/class.c
===================================================================
--- gcc/java/class.c    (revision 218005)
+++ gcc/java/class.c    (working copy)
@@ -1084,8 +1084,6 @@ build_classdollar_field (tree type)
                                               TYPE_QUAL_CONST)),
                        TYPE_QUAL_CONST)));
       TREE_STATIC (decl) = 1;
-      TREE_CONSTANT (decl) = 1;
-      TREE_READONLY (decl) = 1;
       TREE_PUBLIC (decl) = 1;
       java_hide_decl (decl);
       DECL_IGNORED_P (decl) = 1;

not sure why it doesn't end up in .rodata.
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 24 Nov 2014, aph at redhat dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044
>
> Andrew Haley <aph at redhat dot com> changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |aph at redhat dot com
>
> --- Comment #2 from Andrew Haley <aph at redhat dot com> ---
> So, is the solution to this trivially not to mark the .class$ decls as
> TREE_CONST ?

Yes, see the patch I proposed (in testing right now, I'll post it
and ask for approval later today unless you want to pre-approve here)
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

--- Comment #5 from Andrew Haley <aph at gcc dot gnu.org> ---
(In reply to [hidden email] from comment #4)

> On Mon, 24 Nov 2014, aph at redhat dot com wrote:
>
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044
> >
> > Andrew Haley <aph at redhat dot com> changed:
> >
> >            What    |Removed                     |Added
> > ----------------------------------------------------------------------------
> >                  CC|                            |aph at redhat dot com
> >
> > --- Comment #2 from Andrew Haley <aph at redhat dot com> ---
> > So, is the solution to this trivially not to mark the .class$ decls as
> > TREE_CONST ?
>
> Yes, see the patch I proposed (in testing right now, I'll post it
> and ask for approval later today unless you want to pre-approve here)

Fine by me.  I did that because I wanted some way to tell GCC that it could
treat the field as readonly, but TREE_CONST doesn't do that.
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk.
Reply | Threaded
Open this post in threaded view
|

[Bug java/64044] Java emits bogus .class$ decls

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64044

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed by r218018