RFR: 8284877: Check type compatibility before looking up method from receiver's vtable [v2]

[Yi Yang]

On Thu, 21 Apr 2022 06:04:27 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> Hi, can I have a review for this enhancement? This patch adds type compatibility check before method lookup for robustness. In some internal cases, serialization framework may improperly generate an object of wrong type, which leads JVM randomly crashes during method resolution.
>> 
>> For example:
>> 
>> invokevirtual selected method: receiver-class:java.util.ArrayList, resolved-class:com.taobao.forest.domain.util.LongMapSupportArrayList, resolved_method:com.taobao.forest.domain.util.LongMapSupportArrayList.toMap()Ljava/util/Map;, selected_method:0x458, vtable_index:56#
>> 
>> The real type of receiver is ArrayList, while the resolved method is LongMapSupportArrayList.toMap. VM attempts to select method as if looking up from receiver's vtable via vtable index of resolved method(i.e. attempts to lookup `toMap()` from 
>>  ArrayList), an invalid method or incorrect method would be selected, thus causing some strange crashes.
>> 
>> I think it's reasonable to add a type compatibility check before method lookup. If such an incompatible call is found, JVM could throw an exception instead.
>
> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add jtreg test

I reopened this because it successfully catches some JNI mistakes internally. There is a recent feedback. Our user was calling a constructor of a class using JNI, passing a String object as the final parameter, successfully constructing a FileStatus object as follows:


public class FileStatus implements Writable, Comparable {
	....
	public FileStatus(...., Path path) {
		...
		this.path = path;
	}
}


Before this check was added, the user did not notice this mistake and their code worked fine because they called `odpsFileStatus.getPath().toString()` later, and the `toString` virtual call happens to be in the String class.

With this patch, the check successfully caught the user's misuse and threw an exception `java.lang.IncompatibleClassChangeError: The type of receiver java.lang.String should be or subtype of com.aliyun.odps.fs.Path during toString`.

Therefore, I believe this patch can detect some errors in JNI programming other than Unsafe misuse. Considering the minimal performance impact of this change, I hope upstream will reconsider this patch. @DamonFool @dholmes-ora @ExE-Boss 

Thanks

-------------

PR Comment: https://git.openjdk.org/jdk/pull/8241#issuecomment-1590992803