Obsolete/incorrect assertion in natVMClassLoader.cc?

Obsolete/incorrect assertion in natVMClassLoader.cc?

[Dave Korn]

  Testing --enable-libgcj-debug, I've been running into this assertion:

> 
> spawn [open ...]
> libgcj failure: klass->state == JV_STATE_LOADED
>    in function defineClass, file
> /gnu/gcc/gcc/libjava/java/lang/natVMClassLoader.cc, line 99
> FAIL: ExtraClassLoader execution - source compiled test


  According to an old post on the subject:

    http://gcc.gnu.org/ml/java/2005-07/msg00004.html

"The assertion is wrong".  Am I right in taking it to mean that after calling
DefineClass, the class state is indeed *supposed* to be JV_STATE_READ (as it
in fact is, having just been processed by _Jv_ClassReader::parse), rather than
JV_STATE_LOADED (which the assertion tests for)?

    cheers,
      DaveK

Re: Obsolete/incorrect assertion in natVMClassLoader.cc?

[Tom Tromey]

>>>>> "Dave" == Dave Korn <dave.korn.cygwin@googlemail.com> writes:

Dave>   According to an old post on the subject:
Dave>     http://gcc.gnu.org/ml/java/2005-07/msg00004.html

Dave> "The assertion is wrong".  Am I right in taking it to mean that
Dave> after calling DefineClass, the class state is indeed *supposed*
Dave> to be JV_STATE_READ (as it in fact is, having just been
Dave> processed by _Jv_ClassReader::parse), rather than
Dave> JV_STATE_LOADED (which the assertion tests for)?

Yes, I think so.
If you want to fix or delete the assertion, go right ahead :)

Tom

Re: Obsolete/incorrect assertion in natVMClassLoader.cc?

[Dave Korn]

Tom Tromey wrote:

> If you want to fix or delete the assertion, go right ahead :)

  I interpreted that as pre-approval (please, correct me if I was wrong) and
applied the obvious fix with this changelog:

libjava/ChangeLog

	* java/lang/natVMClassLoader.cc
	(java::lang::VMClassLoader::defineClass):  Fix assert.


... after verifying that doing so stopped the asserts from firing.

    cheers,
      DaveK

Re: Obsolete/incorrect assertion in natVMClassLoader.cc?

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

Dave Korn wrote:
> applied the obvious fix 

  Obvious it may be, but why don't I attach the patch anyway? ;-)

    cheers,
      DaveK

[-- Attachment #2: pr38892-part-3.diff --]
[-- Type: text/x-c, Size: 474 bytes --]

Index: java/lang/natVMClassLoader.cc
===================================================================
--- java/lang/natVMClassLoader.cc	(revision 146867)
+++ java/lang/natVMClassLoader.cc	(working copy)
@@ -96,7 +96,7 @@ java::lang::VMClassLoader::defineClass (java::lang
 	}
 
       // if everything proceeded sucessfully, we're loaded.
-      JvAssert (klass->state == JV_STATE_LOADED);
+      JvAssert (klass->state == JV_STATE_READ);
     }
 #endif // INTERPRETER