Quantcast

[9 or 10] RFR(S): 8179070: nashorn+octane's box2d causes c2 to crash with "Bad graph detected in compute_lca_of_uses"

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9 or 10] RFR(S): 8179070: nashorn+octane's box2d causes c2 to crash with "Bad graph detected in compute_lca_of_uses"

Roland Westrelin-3

http://cr.openjdk.java.net/~roland/8179070/webrev.00/

The test case below reproduces a similar problem:
http://cr.openjdk.java.net/~roland/8179070/TestClassLoadingConstant.java 

This is a concurrency issue between a compiler thread and a thread
initializing a class. The test case can be used to cause a "malformed
control flow" compilation failure. This only really happens if the
compiler thread is artifically paused at a strategic point during
compilation to allow concurrent class initialization.

When compiling TestClassLoadingConstant.test(), when ciTypeFlow
processes:

o2 = D.unloaded_field;

it finds class D to be loaded but not initialized and class
UnloadedClass to be unloaded. Assuming class loading of C happens
concurrently at this point, ciTypeFlow will then process:

Object o1 = C.unloaded_field;

and find C to be initialized but will still consider UnloadedClass to be
unloaded because it was processed
before. ciTypeFlow::StateVector::do_getstatic() speculates an access to
a field of unloaded type is a null field.  Parse::do_get_xxx() makes the
same assumption to keep everything consistent.

The type of o1 on loop entry after ciTypeFlow is done is the meet of
null from o1 = C.unloaded_field; and LoadedClass from o1 = loaded_field;
so LoadedClass.

When C2 parses:

Object o1 = C.unloaded_field;

it finds the field is a non null constant. The check:

if (field->is_constant()) {

in Parse::do_get_xxx() happens before the code that mirrors the logic of
ciTypeFlow and treats an unloaded field as null.

When C2 parses the loop entry, it creates a Phi for o1 of the type
computed by ciTypeFlow so LoadedClass. That Phi's first input is the
constant of unrelated type UnloadClass. During CCP, the inconsistency
causes the Phi to becomes top and some part of the control flow to die.

I can reproduce the failure with the TestClassLoadingConstant test case
on 9 and 10. I see the failure with box2d on almost all runs of box2d
with 10. I suspect the issues reproduces very easily, because the method
being compiled is huge with a lot of incremental inlining going on so
the window for the concurrent class loading to occur is quite large. I
couldn't reproduce it with 9 though.

The fix seems safe enough that I would say this would better be fixed in
9.

Roland.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9 or 10] RFR(S): 8179070: nashorn+octane's box2d causes c2 to crash with "Bad graph detected in compute_lca_of_uses"

Tobias Hartmann-2
Hi Roland,

On 25.04.2017 10:38, Roland Westrelin wrote:
> http://cr.openjdk.java.net/~roland/8179070/webrev.00/

The fix looks good to me, thanks for the detailed explanation!

> The fix seems safe enough that I would say this would better be fixed in 9.

Yes, I agree. I targeted this for JDK 9, please request approval according to
http://openjdk.java.net/projects/jdk9/fix-request-process

Thanks,
Tobias
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9 or 10] RFR(S): 8179070: nashorn+octane's box2d causes c2 to crash with "Bad graph detected in compute_lca_of_uses"

Roland Westrelin-3

>> http://cr.openjdk.java.net/~roland/8179070/webrev.00/
>
> The fix looks good to me, thanks for the detailed explanation!

Thanks for the review, Tobias!

>> The fix seems safe enough that I would say this would better be fixed in 9.
>
> Yes, I agree. I targeted this for JDK 9, please request approval according to
> http://openjdk.java.net/projects/jdk9/fix-request-process

Will do.

Roland.
Loading...