RFR: 8320002: Remove obsolete CDS check in Reflection::verify_class_access()

[Calvin Cheung]

On Mon, 5 Feb 2024 01:15:45 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> The code that Calvin is removing is very old code. I guess at one time during the initial development of modules in JDK 9, even when the access was perfectly OK, the "real" check would fail because some information were not properly set up. So as a hack, someone added:
>> 
>> 
>> if (DumpSharedSpaces) {
>>   return ACCESS_OK
>> }
>> if (some_real_check()) { // <-- would cause failures/assert/etc if DumpSharedSpaces==true
>> }
>> 
>> 
>> Unfortunately there's no history for why this hack was added:
>> 
>> https://github.com/openjdk/jdk/blame/6e86904a94d2ed2815aa6e3364c048dac595320d/src/hotspot/share/runtime/reflection.cpp#L458-L466
>> 
>> However, that problem doesn't exist anymore, so we can remove this hack.
>> 
>> Yes, this changes behavior. However, we run a very limited set of Java code during "java -Xshare:dump". Specifically, we never execute any user-supplied code. So even if we perform the real checks, we should always get ACCESS_OK anyway. If we don't, that will be a bug, and we better know about it.
>
>> Yes, this changes behavior. However, we run a very limited set of Java code during "java -Xshare:dump". Specifically, we never execute any user-supplied code. So even if we perform the real checks, we should always get ACCESS_OK anyway. If we don't, that will be a bug, and we better know about it.
> 
> Seems to me like this is/was simply a quick exit path: we know/expect all checks to pass so why bother actually performing them. I'm fine with now actually performing them, in case at some point something is added that could fail, as long as we don't expect anything to actually start failing now.
> 
> Thanks.

Thanks @dholmes-ora, @iklam, @matias9927 for the review.

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

PR Comment: https://git.openjdk.org/jdk/pull/17232#issuecomment-1955101655