This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch (please comment): class loader and interpreter fixes


Tom Tromey wrote:
> This patch fixes a few different interpreter and class loader
> problems.  The other patches I checked in over the last couple days
> were originally part of this, but I tried to split it into more
> manageable chunks.

I'm not the best to speak when it comes to gcj internals, but here are
some views based on a spec standpoint.

>
> * _Jv_IsInterpretedClass was implemented incorrectly.
>   This patch adds a new ACC_INTERPRET flag which we use instead.
>   This is a potentially bogus operation.  One idea I had was to put
>   the new flag into Modifier (as package-private) so that if Sun ever
>   adds new flags we'll be more likely to catch the problem.  Comments?

Sun announced JSR 202 this week, with proposals for enhancing the .class
file format and VM requirements (http://jcp.org/en/jsr/detail?id=202) in
the JDK 1.5 timeframe.  As part of the proposal, they said they might
define some of the currently unused ACC_* bits for improving
performance.  But it looks like it will be a well-documented extension
path, so it should be easy to redefine ACC_INTERPRET if there is a clash
introduced.  And I think it should be fine to pirate unused bits from
the .class file for VM use, as long as those bits are never visible via
things like java.lang.reflect.Method.getModifiers().

If you don't add the flag directly to Modifier, at least add a comment
in Modifier cross-referencing to java-interp.h where you do define
ACC_INTERPRET.

>
> * The low-level class cache, which is used to do internal lookups for
>   initiating loaders, would sometimes register things incorrectly.
>   There is confusion in libgcj about whether the system class loader
>   should be represented by NULL or by its actual value.  This patch
>   clears that up, at least for the cache.
>
> * Miranda methods weren't properly handled.  We now handle them by
>   adding new method slots to an abstract class.  We mark these slots
>   as ACC_SYNTHETIC so that they aren't visible to reflection.  (They
>   are still visible to JNI, but does that matter?)  The same comments
>   for ACC_INTERPRET apply to ACC_SYNTHETIC.

This should also work.  Maybe a different name is in order, like
ACC_INVISIBLE? You don't want anyone thinking that the Synthetic
attribute in the .class file and your new ACC_SYNTHETIC access flag are
synonyms, and that setting one would set the other, because Synthetic
attribute methods should still reflect (things like access$0() for
accessing private fields in enclosing classes).

>
> * This fixes a small memory leak in the internal class cache.
>
> * This fixes the vtable layout code to correctly handle final
>   classes -- a new method in a final class doesn't get a vtable slot.
>   Also, we put a special function into "abstract" vtable slots, just
>   in case.

Will this make it possible to simplify code elsewhere?  I haven't looked
at the code, but my guess is that reflection and/or the code for
invokespecial is performing what is now a redundant check to see if the
target method is abstract, rather than performing a blind call on the
method and letting the placeholder throw the exception.

>
> For the Miranda methods fix, I took the approach I did because it
> seemed much more efficient than the alternatives.  If someone can
> think of a better way to do this, I'm happy to implement it.

Looks like a good approach to me.  By adding the Miranda methods in
memory, you have guaranteed that resolution of invokevirtual will now
always find its method in a superclass, avoiding the need to traverse
the interface heirarchy.

However, there is one problem I just ran across -- you also need to
worry about inherited interface fields:

$ cat C.java
interface I { int i = "1".length(); /* non-constant */ }
class C implements I
{
   public static void main(String[] args)
   {
     System.out.println(C.i);
   }
}
$ jikes -source 1.4 C.java
$ gij C
Exception in thread "main" java.lang.IncompatibleClassChangeError: field
C.i was not found.
$ java C
1

gij croaks on compliant code, when Sun's 1.4 VM does not.

Older compilers compile C.i to "getstatic I.i".  But JLS 13 requires
that it compile to "getstatic C.i" and that the VM perform the same sort
of resolution for fields as for methods.  Jikes 1.18 is conditional on
which code it emits, depending on the -source flag, because even Sun's
1.3 VM rejects the correct bytecode.  But gcj always emits the incorrect
code; should I file a bug against gcj for being non-compliant, since gcj
doesn't support -source yet?

Anyways, is it possible to do the same trick for fields as for Miranda
methods?  Since I.i is final, you could create a synthetic field C.i and
copy the value of i there once it is known.  But I am wondering about
initialization semantics - will code always see the semantically correct
value of C.i, no matter what code path is taken?  You may have to give
up, and make field resolution in gij traverse interface hierarchy, even
though method resolution does not.  And if you do end up implementing
interface heirarchy traversal, is it then still worth plugging the holes
of Miranda methods instead of finding them on a traversal as well?

>
> Tom
>

--
This signature intentionally left boring.

Eric Blake             ebb9@email.byu.edu
   BYU student, free software programmer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]