[ecj] Patch: FYI: fix ordering bugs, others

[ecj] Patch: FYI: fix ordering bugs, others

[Tom Tromey]

I'm checking this in on the gcj-eclipse branch.

This patch is a bit messy since it is conceptually a few different
fixes in one.  I forgot how they interact, though, and there didn't
seem to be much reason to try to separate them all out again.

* PR 27643, a bug involving the order in which we read class files.
  This is fixed by moving around the java_mark_class_local logic.
  Note that this breaks .java compilation; I didn't look into this
  because...
* This patch removes the ability to compile .java files.
* This patch fixes is_compiled_class to return the correct results.
  Right now it is sometimes wrong, e.g. it assumes that any file
  coming from a .zip must be compiled into this object.
* We disable the gnu.gcj.gcj-compiled attribute warning for now.
  ecj doesn't emit this attribute.  We can salvage this idea later by
  adding a gcj-specific annotation.
* There is a lurking bug where if you compile Class along with some
  other files, you will get bad results unless Class happens to come
  first.  This is a logic error in make_class_data; fixed in this
  patch.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* jcf-io.c (find_class): Set source_ok to 0.
	* jcf-parse.c (jcf_parse): Disable gnu.gcj.gcj-compiled warning.
	(parse_class_file): Don't call java_mark_class_local.
	(java_parse_file): Skip .java files.  Call java_mark_class_local
	before lowering any code.
	(parse_zip_file_entries): Don't call duplicate_class_warning
	here.
	(process_zip_dir): ... call it here.
	* class.c (add_field): Don't mark field external if it is being
	compiled into this object.
	(make_class_data): Handle situation where class_dtable_decl is
	created before Class is compiled.
	(is_compiled_class): Don't assume files in zip are compiled into
	this object.
	(layout_class_method): Don't mark method external if it is being
	compiled into this object.

Index: jcf-io.c
===================================================================
--- jcf-io.c	(revision 114362)
+++ jcf-io.c	(working copy)
@@ -1,5 +1,5 @@
 /* Utility routines for finding and reading Java(TM) .class files.
-   Copyright (C) 1996, 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005
+   Copyright (C) 1996, 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -450,6 +450,9 @@
   char *buffer;
   hashval_t hash;
 
+  /* FIXME: ecj hack.  */
+  source_ok = 0;
+
   /* Create the hash table, if it does not already exist.  */
   if (!memoized_class_lookups)
     memoized_class_lookups = htab_create (37, 
Index: jcf-parse.c
===================================================================
--- jcf-parse.c	(revision 114362)
+++ jcf-parse.c	(working copy)
@@ -1,5 +1,5 @@
 /* Parser for Java(TM) .class files.
-   Copyright (C) 1996, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005
+   Copyright (C) 1996, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -804,9 +804,12 @@
       /* If we don't have the right archive, emit a verbose warning.
 	 If we're generating bytecode, emit the warning only if
 	 -fforce-classes-archive-check was specified. */
+#if 0
+      /* ECJ HACK: ignore this.  */
       if (!jcf->right_zip
 	  && (!flag_emit_class_files || flag_force_classes_archive_check))
 	fatal_error ("the %<java.lang.Object%> that was found in %qs didn't have the special zero-length %<gnu.gcj.gcj-compiled%> attribute.  This generally means that your classpath is incorrectly set.  Use %<info gcj \"Input Options\"%> to see the info page describing how to set the classpath", jcf->filename);
+#endif
     }
   else
     all_class_list = tree_cons (NULL_TREE,
@@ -857,8 +860,6 @@
 
   gen_indirect_dispatch_tables (current_class);
 
-  java_mark_class_local (current_class);
-
   for (method = TYPE_METHODS (current_class);
        method != NULL_TREE; method = TREE_CHAIN (method))
     {
@@ -1151,8 +1152,13 @@
 	  next++;
 	}
 
-      if (list[0]) 
+      /* Exclude .java files.  */
+      if (strlen (list) > 5 && ! strcmp (list + strlen (list) - 5, ".java"))
 	{
+	  /* Nothing. */
+	}
+      else if (list[0]) 
+	{
 	  node = get_identifier (list);
 
 	  filename_count++;
@@ -1293,8 +1299,15 @@
       parse_source_file_3 ();
     }
 
+  /* Do this before lowering any code.  */
   for (node = current_file_list; node; node = TREE_CHAIN (node))
     {
+      if (CLASS_FILE_P (node))
+	java_mark_class_local (TREE_TYPE (node));
+    }
+
+  for (node = current_file_list; node; node = TREE_CHAIN (node))
+    {
       input_location = DECL_SOURCE_LOCATION (node);
       if (CLASS_FILE_P (node))
 	{
@@ -1409,15 +1422,6 @@
 	    current_jcf = TYPE_JCF (class);
 	    output_class = current_class = class;
 
-	    if (CLASS_FROM_CURRENTLY_COMPILED_P (current_class))
-	      {
-	        /* We've already compiled this class.  */
-		duplicate_class_warning (current_jcf->filename);
-		break;
-	      }
-	    
-	    CLASS_FROM_CURRENTLY_COMPILED_P (current_class) = 1;
-
 	    /* This is a dummy class, and now we're compiling it for
 	       real.  */
 	    gcc_assert (! TYPE_DUMMY (class));
@@ -1532,6 +1536,16 @@
 
       class = lookup_class (get_identifier (class_name));
 
+      if (CLASS_FROM_CURRENTLY_COMPILED_P (class))
+	{
+	  /* We've already compiled this class.  */
+	  duplicate_class_warning (file_name);
+	  continue;
+	}
+      /* This function is only called when processing a zip file seen
+	 on the command line.  */
+      CLASS_FROM_CURRENTLY_COMPILED_P (class) = 1;
+
       jcf->read_state  = finput;
       jcf->filbuf      = jcf_filbuf_from_stdio;
       jcf->java_source = 0;
Index: class.c
===================================================================
--- class.c	(revision 114362)
+++ class.c	(working copy)
@@ -793,9 +793,9 @@
       /* Always make field externally visible.  This is required so
 	 that native methods can always access the field.  */
       TREE_PUBLIC (field) = 1;
-      /* Considered external until we know what classes are being
-	 compiled into this object file.  */
-      DECL_EXTERNAL (field) = 1;
+      /* Considered external unless we are compiling it into this
+	 object file.  */
+      DECL_EXTERNAL (field) = (is_compiled_class (class) != 2);
     }
 
   return field;
@@ -1650,15 +1650,28 @@
     {
       tree dtable = get_dispatch_table (type, this_class_addr);
       uses_jv_markobj = uses_jv_markobj_p (dtable);
-      dtable_decl = build_dtable_decl (type);
+      if (type == class_type_node && class_dtable_decl != NULL_TREE)
+	{
+	  /* We've already created some other class, and consequently
+	     we made class_dtable_decl.  Now we just want to fill it
+	     in.  */
+	  dtable_decl = class_dtable_decl;
+	}
+      else
+	{
+	  dtable_decl = build_dtable_decl (type);
+	  TREE_STATIC (dtable_decl) = 1;
+	  DECL_ARTIFICIAL (dtable_decl) = 1;
+	  DECL_IGNORED_P (dtable_decl) = 1;
+	}
+
+      TREE_PUBLIC (dtable_decl) = 1;
       DECL_INITIAL (dtable_decl) = dtable;
-      TREE_STATIC (dtable_decl) = 1;
-      DECL_ARTIFICIAL (dtable_decl) = 1;
-      DECL_IGNORED_P (dtable_decl) = 1;
-      TREE_PUBLIC (dtable_decl) = 1;
       if (! flag_indirect_classes)
 	rest_of_decl_compilation (dtable_decl, 1, 0);
-      if (type == class_type_node)
+      /* Maybe we're compiling Class as the first class.  If so, set
+	 class_dtable_decl to the decl we just made.  */
+      if (type == class_type_node && class_dtable_decl == NULL_TREE)
 	class_dtable_decl = dtable_decl;
     }
 
@@ -1776,8 +1789,10 @@
       DECL_ARTIFICIAL (class_dtable_decl) = 1;
       DECL_IGNORED_P (class_dtable_decl) = 1;
       if (is_compiled_class (class_type_node) != 2)
-	DECL_EXTERNAL (class_dtable_decl) = 1;
-      rest_of_decl_compilation (class_dtable_decl, 1, 0);
+	{
+	  DECL_EXTERNAL (class_dtable_decl) = 1;
+	  rest_of_decl_compilation (class_dtable_decl, 1, 0);
+	}
     }
 
   super = CLASSTYPE_SUPER (type);
@@ -2059,11 +2074,9 @@
     return 1;
   if (TYPE_ARRAY_P (class))
     return 0;
-  if (class == current_class)
-    return 2;
 
   seen_in_zip = (TYPE_JCF (class) && JCF_SEEN_IN_ZIP (TYPE_JCF (class)));
-  if (CLASS_FROM_CURRENTLY_COMPILED_P (class) || seen_in_zip)
+  if (CLASS_FROM_CURRENTLY_COMPILED_P (class))
     {
       /* The class was seen in the current ZIP file and will be
 	 available as a compiled class in the future but may not have
@@ -2443,9 +2456,10 @@
   tree method_name = DECL_NAME (method_decl);
 
   TREE_PUBLIC (method_decl) = 1;
-  /* Considered external until we know what classes are being
-     compiled into this object file.  */
-  DECL_EXTERNAL (method_decl) = 1;
+  /* Considered external unless it is being compiled into this object
+     file.  */
+  DECL_EXTERNAL (method_decl) = ((is_compiled_class (this_class) != 2)
+				 || METHOD_NATIVE (method_decl));
 
   if (ID_INIT_P (method_name))
     {

Re: [ecj] Patch: FYI: fix ordering bugs, others

[Andrew Haley]

Tom Tromey writes:
 > I'm checking this in on the gcj-eclipse branch.
 > 
 > This patch is a bit messy since it is conceptually a few different
 > fixes in one.  I forgot how they interact, though, and there didn't
 > seem to be much reason to try to separate them all out again.
 > 
 > * PR 27643, a bug involving the order in which we read class files.
 >   This is fixed by moving around the java_mark_class_local logic.
 >   Note that this breaks .java compilation; I didn't look into this
 >   because...
 > * This patch removes the ability to compile .java files.
 > * This patch fixes is_compiled_class to return the correct results.
 >   Right now it is sometimes wrong, e.g. it assumes that any file
 >   coming from a .zip must be compiled into this object.

I'd like to back-port the fix for PR 27643 to older branches.  In what
way does it break .java compilation?  It looks to mek like the change
you made only affects .class files.

Andrew.


 > Index: jcf-io.c
 > ===================================================================
 > --- jcf-io.c	(revision 114362)
 > +++ jcf-io.c	(working copy)
 > @@ -1,5 +1,5 @@
 >  /* Utility routines for finding and reading Java(TM) .class files.
 > -   Copyright (C) 1996, 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005
 > +   Copyright (C) 1996, 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006
 >     Free Software Foundation, Inc.
 >  
 >  This file is part of GCC.
 > @@ -450,6 +450,9 @@
 >    char *buffer;
 >    hashval_t hash;
 >  
 > +  /* FIXME: ecj hack.  */
 > @@ -857,8 +860,6 @@
 >  
 >    gen_indirect_dispatch_tables (current_class);
 >  
 > -  java_mark_class_local (current_class);
 > -
 >    for (method = TYPE_METHODS (current_class);
 >         method != NULL_TREE; method = TREE_CHAIN (method))
 >      {
 > @@ -1293,8 +1299,15 @@
 >        parse_source_file_3 ();
 >      }
 >  
 > +  /* Do this before lowering any code.  */
 >    for (node = current_file_list; node; node = TREE_CHAIN (node))
 >      {
 > +      if (CLASS_FILE_P (node))
 > +	java_mark_class_local (TREE_TYPE (node));
 > +    }
 > +
 > +  for (node = current_file_list; node; node = TREE_CHAIN (node))
 > +    {
 >        input_location = DECL_SOURCE_LOCATION (node);
 >        if (CLASS_FILE_P (node))
 >  	{

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903

Re: [ecj] Patch: FYI: fix ordering bugs, others

[Tom Tromey]

>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:

>> * PR 27643, a bug involving the order in which we read class files.
>> This is fixed by moving around the java_mark_class_local logic.
>> Note that this breaks .java compilation; I didn't look into this
>> because...

Andrew> I'd like to back-port the fix for PR 27643 to older branches.  In what
Andrew> way does it break .java compilation?  It looks to mek like the change
Andrew> you made only affects .class files.

I don't remember, sorry.

Tom

Re: [ecj] Patch: FYI: fix ordering bugs, others

[Andrew Haley]

Tom Tromey writes:
 > >>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:
 > 
 > >> * PR 27643, a bug involving the order in which we read class files.
 > >> This is fixed by moving around the java_mark_class_local logic.
 > >> Note that this breaks .java compilation; I didn't look into this
 > >> because...
 > 
 > Andrew> I'd like to back-port the fix for PR 27643 to older branches.  In what
 > Andrew> way does it break .java compilation?  It looks to mek like the change
 > Andrew> you made only affects .class files.
 > 
 > I don't remember, sorry.

I've looked, and I'm sure your patch for PR 27643 is wrong, sorry.  :-(

It disables the use of hidden aliases when compiling from .jar files.
This is because the code path where you moved the call to
java_mark_class_local() isn't executed when compiling from .jar.

A simpler fix for this PR is, rather than trying to move the call to
java_mark_class_local(), to fix up the RTL when we mark the decl.  If
a call has already been made to the external symbol rather than its
hidden alias, that really doesn't matter.

Andrew.



2007-12-17  Andrew Haley  <aph@redhat.com>

	PR java/27643
	* decl.c (java_mark_cni_decl_local): If the ASSEMBLER_NAME is
	already set, call java_mangle_decl() and make_decl_rtl() to
	rewrite its name as a hidden alias.
 
Index: java/decl.c
===================================================================
--- java/decl.c (revision 123182)
+++ java/decl.c (working copy)
@@ -2180,18 +2180,27 @@
 static void
 java_mark_cni_decl_local (tree decl)
 {
-  /* Setting DECL_LOCAL_CNI_METHOD_P changes the behavior of the mangler.
-     We expect that we should not yet have referenced this decl in a 
-     context that requires it.  Check this invariant even if we don't have
-     support for hidden aliases.  */
-  gcc_assert (!DECL_ASSEMBLER_NAME_SET_P (decl));
-
 #if !defined(HAVE_GAS_HIDDEN) || !defined(ASM_OUTPUT_DEF)
   return;
 #endif
 
   DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
   DECL_LOCAL_CNI_METHOD_P (decl) = 1;
+
+  /* Setting DECL_LOCAL_CNI_METHOD_P changes the behavior of the
+     mangler.  We might have already referenced this native method and
+     therefore created its name, but even if we have it won't hurt.
+     We'll just go via its externally visible name, rather than its
+     hidden alias.  However, we must force things so that the correct
+     mangling is done.  */
+
+  if (DECL_ASSEMBLER_NAME_SET_P (decl))
+    java_mangle_decl (decl);
+  if (DECL_RTL_SET_P (decl))
+    {
+      SET_DECL_RTL (decl, 0);
+      make_decl_rtl (decl);
+    }
 }
 
 /* Use the preceding two functions and mark all members of the class.  */

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903

Re: [ecj] Patch: FYI: fix ordering bugs, others

[Andrew Haley]

I committed this.

Andrew.


2007-12-18  Andrew Haley  <aph@redhat.com>

        PR java/27643
        * jcf-parse.c (java_parse_file): Remove call to
        java_mark_class_local.
        (parse_class_file): Reinstate call to java_mark_class_local here.
        * decl.c (java_mark_cni_decl_local): If the ASSEMBLER_NAME is
        already set, call java_mangle_decl() and make_decl_rtl() to
        rewrite its name as a hidden alias.

Index: decl.c
===================================================================
--- decl.c      (revision 131034)
+++ decl.c      (working copy)
@@ -1890,18 +1890,27 @@
 static void
 java_mark_cni_decl_local (tree decl)
 {
-  /* Setting DECL_LOCAL_CNI_METHOD_P changes the behavior of the mangler.
-     We expect that we should not yet have referenced this decl in a 
-     context that requires it.  Check this invariant even if we don't have
-     support for hidden aliases.  */
-  gcc_assert (!DECL_ASSEMBLER_NAME_SET_P (decl));
-
 #if !defined(HAVE_GAS_HIDDEN) || !defined(ASM_OUTPUT_DEF)
   return;
 #endif
 
   DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
   DECL_LOCAL_CNI_METHOD_P (decl) = 1;
+
+  /* Setting DECL_LOCAL_CNI_METHOD_P changes the behavior of the
+     mangler.  We might have already referenced this native method and
+     therefore created its name, but even if we have it won't hurt.
+     We'll just go via its externally visible name, rather than its
+     hidden alias.  However, we must force things so that the correct
+     mangling is done.  */
+
+  if (DECL_ASSEMBLER_NAME_SET_P (decl))
+    java_mangle_decl (decl);
+  if (DECL_RTL_SET_P (decl))
+    {
+      SET_DECL_RTL (decl, 0);
+      make_decl_rtl (decl);
+    }
 }
 
 /* Use the preceding two functions and mark all members of the class.  */
Index: jcf-parse.c
===================================================================
--- jcf-parse.c (revision 131034)
+++ jcf-parse.c (working copy)
@@ -1596,6 +1596,8 @@
   file_start_location = input_location;
   (*debug_hooks->start_source_file) (input_line, input_filename);
 
+  java_mark_class_local (current_class);
+
   gen_indirect_dispatch_tables (current_class);
 
   for (method = TYPE_METHODS (current_class);
@@ -1967,13 +1969,6 @@
        }
     }
 
-  /* Do this before lowering any code.  */
-  for (node = current_file_list; node; node = TREE_CHAIN (node))
-    {
-      if (CLASS_FILE_P (node))
-       java_mark_class_local (TREE_TYPE (node));
-    }
-
   for (node = current_file_list; node; node = TREE_CHAIN (node))
     {
       input_location = DECL_SOURCE_LOCATION (node);

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903