[gcjx] Patch: FYI: fix vtable layout

Tom Tromey tromey@redhat.com
Tue Mar 8 17:59:00 GMT 2005


I'm checking this in on the gcjx branch.

While debugging 'hello world' I found that vtable layout was
incorrect.  This is because methods have to be stored by model_class
in declaration order, but they were being sorted instead.  This adds
a new field to model_class to track the methods in the proper order.

This isn't ideal in that it is even more duplication of information
in model_class.  I'll revisit this at some future date.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* header/cni.cc (write_includes): Changed argument types.
	(generate): Use get_sorted_methods.
	* header/cni.hh (cni_code_generator::method_iterator): New
	typedef.
	(cni_code_generator::write_includes): Changed argument types.
	* aot/aotclass.cc (lay_out_vtable): Use get_sorted_methods.  Skip
	final methods from Object.
	(add_item): Don't use slot 0 of constant pool.
	* model/class.cc (resolve_one_method): Updated
	source_order_methods.
	(add_method): Likewise.
	(inherit_methods): Likewise.
	(resolve_members): Likewise.
	* model/class.hh (model_class::source_order_methods): New field.
	(model_class::get_sorted_methods): New method.

Index: aot/aotclass.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/aot/Attic/aotclass.cc,v
retrieving revision 1.1.2.5
diff -u -r1.1.2.5 aotclass.cc
--- aot/aotclass.cc 8 Mar 2005 02:59:17 -0000 1.1.2.5
+++ aot/aotclass.cc 8 Mar 2005 17:17:39 -0000
@@ -43,8 +43,11 @@
   if (super)
     vtable = super->vtable;
 
-  for (AllMethodsIterator i = klass->begin_all_methods ();
-       i != klass->end_all_methods ();
+  model_class *objtype = global->get_compiler ()->java_lang_Object ();
+
+  std::list<model_method *> methods = klass->get_sorted_methods ();
+  for (std::list<model_method *>::const_iterator i = methods.begin ();
+       i != methods.end ();
        ++i)
     {
       // Static methods, private methods, and constructors do not
@@ -56,6 +59,12 @@
 	  || ((*i)->get_modifiers () & ACC_PRIVATE) != 0)
 	continue;
 
+      // Final methods in Object are not entered in the vtable as they
+      // are never called virtually.  FIXME: conceivably, this could
+      // change in Java, and this threatens binary compatibility.
+      if ((*i)->final_p () && (*i)->get_declaring_class () == objtype)
+	continue;
+
       model_class *declaring_class = (*i)->get_declaring_class ();
       if (declaring_class->interface_p ())
 	{
@@ -88,7 +97,7 @@
 		  && (*i)->hides_or_overrides_p (*j, klass))
 		{
 		  // Re-use the previous slot.
-		  vtable[index] = (*i).get ();
+		  vtable[index] = *i;
 		  break;
 		}
 	    }
@@ -96,7 +105,7 @@
 	    continue;
 	}
 
-      vtable.push_back ((*i).get ());
+      vtable.push_back (*i);
     }
 
 #ifdef DEBUG_VTABLE
@@ -123,7 +132,10 @@
     }
 
   pool.push_back (entry);
-  return pool.size () - 1;
+  // Note that we return the new size, not 'pool.size() - 1', because
+  // for historical reasons the 0th entry of the constant pool is not
+  // used.
+  return pool.size ();
 }
 
 int
Index: header/cni.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/header/Attic/cni.cc,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 cni.cc
--- header/cni.cc 13 Jan 2005 03:18:35 -0000 1.1.2.1
+++ header/cni.cc 8 Mar 2005 17:17:39 -0000
@@ -1,6 +1,6 @@
 // Write a CNI header.
 
-// Copyright (C) 2004 Free Software Foundation, Inc.
+// Copyright (C) 2004, 2005 Free Software Foundation, Inc.
 //
 // This file is part of GCC.
 //
@@ -242,8 +242,8 @@
 void
 cni_code_generator::write_includes (std::ostream &out,
 				    model_class *klass,
-				    const AllMethodsIterator & methods_begin,
-				    const AllMethodsIterator & methods_end,
+				    const method_iterator &methods_begin,
+				    const method_iterator &methods_end,
 				    const std::list<ref_field> &fields)
 {
   // These classes are chosen since they are already in 'compiler' and
@@ -274,7 +274,7 @@
 	  << ".h>" << std::endl;
     }
 
-  for (AllMethodsIterator i = methods_begin; i != methods_end; ++i)
+  for (method_iterator i = methods_begin; i != methods_end; ++i)
     {
       add ((*i)->get_return_type (), class_set, array_seen,
 	   lang_package, util_package, io_package, super);
@@ -400,8 +400,8 @@
       << std::endl;
 
   std::list<ref_field> fields = klass->get_fields ();
-  write_includes (out, klass, klass->begin_all_methods (),
-                  klass->end_all_methods (), fields);
+  std::list<model_method *> methods = klass->get_sorted_methods ();
+  write_includes (out, klass, methods.begin (), methods.end (), fields);
   out << std::endl;
 
   out << "class " << cxxname (klass, false);
Index: header/cni.hh
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/header/Attic/cni.hh,v
retrieving revision 1.1.2.2
diff -u -r1.1.2.2 cni.hh
--- header/cni.hh 13 Feb 2005 03:39:44 -0000 1.1.2.2
+++ header/cni.hh 8 Mar 2005 17:17:39 -0000
@@ -32,13 +32,16 @@
   // The compiler we're using.
   compiler *comp;
 
+  // Handy typedef.
+  typedef std::list<model_method *>::const_iterator method_iterator;
+
   std::string cxxname (model_type *, bool = true);
   void update_modifiers (std::ostream &, modifier_t, modifier_t &);
   void add (model_type *, std::set<model_class *> &,
 	    bool &, model_package *, model_package *,
 	    model_package *, model_class *);
   void write_includes (std::ostream &, model_class *,
-		       const AllMethodsIterator &, const AllMethodsIterator &,
+		       const method_iterator &, const method_iterator &,
 		       const std::list<ref_field> &);
   void write_method (std::ostream &, model_method *, modifier_t &);
   void write_field (std::ostream &, model_field *, modifier_t &);
Index: model/class.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/class.cc,v
retrieving revision 1.1.2.6
diff -u -r1.1.2.6 class.cc
--- model/class.cc 13 Feb 2005 03:28:19 -0000 1.1.2.6
+++ model/class.cc 8 Mar 2005 17:17:40 -0000
@@ -72,6 +72,7 @@
     return;
 
   all_methods.insert (std::make_pair (method->get_name (), method.get ()));
+  source_order_methods.push_back (method.get ());
 
   resolution_scope scope;
   push_on_scope (&scope);
@@ -466,6 +467,7 @@
 {
   methods.push_back (method);
   all_methods.insert (std::make_pair (method->get_name (), method));
+  source_order_methods.push_back (method.get ());
 }
 
 void
@@ -850,6 +852,7 @@
 	  // FIXME if OTHER == Object && this.interface_p(), then add
 	  // a new abstract method -- ?
 	  all_methods.insert (std::make_pair ((*i)->get_name (), *i));
+	  source_order_methods.push_back ((*i).get ());
 
 	  if (! abstract_p () && (*i)->abstract_p ())
 	    // FIXME: locations.
@@ -1343,6 +1346,7 @@
        ++it)
     {
       all_methods.insert (std::make_pair ((*it)->get_name (), *it));
+      source_order_methods.push_back ((*it).get ());
     }
 
   // Insert our fields into the field map first.
Index: model/class.hh
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/class.hh,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 class.hh
--- model/class.hh 13 Jan 2005 03:18:36 -0000 1.1.2.1
+++ model/class.hh 8 Mar 2005 17:17:40 -0000
@@ -1,6 +1,6 @@
 // Represent the class statement.
 
-// Copyright (C) 2004 Free Software Foundation, Inc.
+// Copyright (C) 2004, 2005 Free Software Foundation, Inc.
 //
 // This file is part of GCC.
 //
@@ -97,6 +97,10 @@
 
   // All methods in this class, including inherited methods.
   AllMethods all_methods;
+  // Like all_methods, but sorted properly for vtable generation
+  // (i.e., source order and inheritance order).
+  // FIXME this should not be needed.
+  std::list<model_method *> source_order_methods;
 
   // Member classes declared here.
   std::map< std::string, owner<model_class> > member_classes;
@@ -679,6 +683,11 @@
   {
     return captured_variables;
   }
+
+  const std::list<model_method *> &get_sorted_methods () const
+  {
+    return source_order_methods;
+  }
 };
 
 #endif // GCJX_MODEL_CLASS_HH



More information about the Java-patches mailing list