8054492: compiler/intrinsics/classcast/NullCheckDroppingsTest.java is an invalid test

[Vladimir Kozlov]

testVarClassCast tests deoptimization for javaMirror == null:

void testVarClassCast(String s) {
     Class cl = (s == null) ? null : String.class;
     try {
         ssink = (String)cl.cast(svalue);

Which is done in LibraryCallKit::inline_Class_cast() by:

mirror = null_check(mirror);

which has Deoptimization::Action_make_not_entrant.

Unfortunately currently the test also pass because unstable_if is 
generated for the first line:

(s == null) ? null : String.class;

If you run the test with TraceDoptimization (or LogCompilation) you will 
see:

Uncommon trap occurred in NullCheckDroppingsTest::testVarClassCast 
(@0x000000010b0670d8) thread=26883 reason=unstable_if action=reinterpret 
unloaded_class_index=-1

In what situation you see it fails? Please, run with TraceDoptimization.

thanks,
Vladimir

On 5/18/15 6:56 AM, Andrew Haley wrote:
> This test has been failing with a failed assertion:
>
> Exception in thread "main" java.lang.AssertionError: void NullCheckDroppingsTest.testVarClassCast(java.lang.String) was not deoptimized
> 	at NullCheckDroppingsTest.checkDeoptimization(NullCheckDroppingsTest.java:331)
> 	at NullCheckDroppingsTest.runTest(NullCheckDroppingsTest.java:309)
> 	at NullCheckDroppingsTest.main(NullCheckDroppingsTest.java:125)
>
> It seems that the reason for the failure is that the nmethod for a
> deoptimized method (testVarClassCast) is not removed.  This is
> perfectly correct because when testVarClassCast() was trapped, it was
> with an action of Deoptimization::Action_maybe_recompile.  When this
> occurs, a method is not marked as dead and continues to be invoked, so
> the test WhiteBox.isMethodCompiled() will still return true.
>
> Therefore, I think this code in checkDeoptimization is incorrect:
>
>          // Check deoptimization event (intrinsic Class.cast() works).
>          if (WHITE_BOX.isMethodCompiled(method) == deopt) {
>              throw new AssertionError(method + " was" + (deopt ? " not" : "") + " deoptimized");
>          }
>
> We either need some code in WhiteBox to check for a deoptimization
> event properly or we should just remove this altogether.
>
> So, thoughts?  Just delete the check?
>
> Andrew.
>