Fixing Clinit - Class initialization on P #4365
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
During static class initialization, static methods belonging to that class are incorrectly allowed to be called by other threads when the caller method is compiled. According to Java specification, all threads except for the one that does the initialization should wait for class initialization <clinit> to complete.
The incorrect behaviour can be observed by the static initializer setting a static variable of some other class to a temporary value that should not be observed by other threads. Note that setting static variable of the same class will not show the problem since similar issue for static variables was fixed.
In particular, this happens when static initializer of class A calls method B.m() that is compiled. B.m() in turn calls static method A.n(). Only initializing thread should be able to call A.n() and not others. However, since currently JIT does the patching (either in the snippet or in the main line code) other threads calling compiled method B.m() may skip resolution process and therefore proceed calling A.n() before A.<clinit> is complete.
This happens because we currently do not check the clinit tag returned by jitResolveStaticMethod in _interpreterUnresolvedStaticGlue in PicBuilder.spp. If clinit bit is set we should skip any patching and:
1) If the method is already jit compiled, obtain the startPC and branch to it.
2) Otherwise, call interpreted version using j2iTransition
Note that, currently, once the jit determines the type of the unresolved method it calls one of several routines to patch and branch into the method. Eg for Native methods we call icallVMprJavaSendNativeStatic. This is due to a dated implementation. Now, all of these specific VM routines call j2iTransition. So, in (2) we will call j2iTransition directly. Non-clinit case can be simplified later.
Signed-off-by: Alen Badel <Alen.Badel@ibm.com>
pshipton
approved these changes
Jan 23, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During static class initialization, static methods belonging to that class are incorrectly allowed to be called by other threads when the caller method is compiled. According to Java specification, all threads except for the one that does the initialization should wait for class initialization to complete.
The incorrect behaviour can be observed by the static initializer setting a static variable of some other class to a temporary value that should not be observed by other threads. Note that setting static variable of the same class will not show the problem since similar issue for static variables was fixed.
In particular, this happens when static initializer of class A calls method B.m() that is compiled. B.m() in turn calls static method A.n(). Only initializing thread should be able to call A.n() and not others. However, since currently JIT does the patching (either in the snippet or in the main line code) other threads calling compiled method B.m() may skip resolution process and therefore proceed calling A.n() before A. is complete.
This happens because we currently do not check the clinit tag returned by jitResolveStaticMethod in _interpreterUnresolvedStaticGlue in PicBuilder.spp. If clinit bit is set we should skip any patching and:
Note that, currently, once the jit determines the type of the unresolved method it calls one of several routines to patch and branch into the method. Eg for Native methods we call icallVMprJavaSendNativeStatic. This is due to a dated implementation. Now, all of these specific VM routines call j2iTransition. So, in (2) we will call j2iTransition directly. Non-clinit case can be simplified later.
Signed-off-by: Alen Badel Alen.Badel@ibm.com