[9] RFR(S): 8050079: crash while compiling java.lang.ref.Finalizer::runFinalizer

[Vladimir Kozlov]

Should you fix problem (2) other way around by adding check in the 
following loop?  If you don't bailout on 'nof_impls > 1' on interface 
what result you get?

I think test should go into new 'dependencies' subdir not 'inlining'.
Why you fork separate JVM process?

Thanks,
Vladimir

On 11/13/14 6:51 AM, Tobias Hartmann wrote:
> Hi,
>
> please review the following patch.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8050079
> Webrev: http://cr.openjdk.java.net/~thartmann/8050079/webrev.00/
>
> Problem (1):
> C1 compiles a call to Object.finalize() in
> 'java.lang.ref.Finalizer::runFinalizer'. To determine if the call is
> monomorphic, the compiler searches through the subclasses of Object until it
> finds a class that overrides finalize(). This check is performed in
> 'ClassHierarchyWalker::find_witness_anywhere' by calling 'is_witness(Klass* k)'
> on the subclasses. After casting with 'InstanceKlass::cast(k)' the (Sub-)Klass k
> is treated as an InstanceKlass and 'InstanceKlass::find_method' is called. The
> crash happens in 'binary_search' when a subclass k is not an InstanceKlass but
> an ObjectArrayKlass and therefore '_methods' is NULL.
>
> Why didn't the bug show up earlier?
> The method Object.finalize() is usually overridden by the subclasses
> 'java.lang.ClassLoader$NativeLibrary' and 'java.lang.Enum' which are loaded
> during startup. That means a call to Object.finalize() is never treated as
> monomorphic and 'ClassHierarchyWalker::find_witness_anywhere' returns before
> encountering an ObjectArrayKlass in the subclass hierarchy of Object. The
> failure only occurs in an internal test environment for JRebel [1]. According to
> the core file of the crash, the usual class hierarchy of the Java Runtime
> Environment was changed by the framework. For example, java.lang.String is no
> longer a subclass of java.lang.Object but of another class called
> 'JaveleonObject' [2]. Especially the two overriding classes
> 'ClassLoader$NativeLibrary' and 'Enum' are not subclasses of Object. This leads
> to the crash because C1 encounters an ObjectArrayKlass in the subclass hierarchy.
> Of course, this bug could also show up if the sequence of subclasses in the
> 'Klass::subklass()' list changes.
>
> Problem (2):
> In case of a worklist overflow we recursively call 'find_witness_anywhere' and
> create a new worklist (see line 1178 of dependencies.cpp). The problem is that
> we now execute the interface related checks that would otherwise not be executed
> (line 1132). For interface subtypes of Object (for example java/lang/Readable)
> we then bail out if 'nof_impls > 1'.
>
> Solution:
> (1) A check for 'oop_is_instance' is added to ignore non-instance Klasses in the
> subclass hierarchy.
> (2) I added a check to only execute the interface related checks if we come from
> a top level call.
>
> I was finally able to write a small jtreg regression test. Although the test is
> a little bit hacky, it is stable and deterministically reproduces the bug. The
> test works by overriding the default implementation of java.lang.Object,
> renaming the method 'finalize' to 'finalizeObject' and thus guaranteeing that
> 'finalizeObject' is never overridden and all calls to it are monomorphic. By
> triggering a C1 compilation of 'Object.finalizeObject()' we trigger the bug
> because there are no overriding instance Klasses in the call hierarchy.
>
> The '-XX:-VerifyDependencies' flag is necessary to not bail out too early
> because of problem (2).
>
> Testing:
> JPRT with new test
>
> Thanks,
> Tobias
>
>
> [1] http://zeroturnaround.com/software/jrebel/
> [2] https://bugs.openjdk.java.net/secure/attachment/23360/replay.log
>