RFR: 8341444: Unnecessary check for JSRs in CDS

[Coleen Phillimore]

On Fri, 4 Oct 2024 05:50:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 8341444: Unnecessary check for JSRs in CDS
>
> src/hotspot/share/interpreter/bytecodeStream.hpp line 198:
> 
>> 196:       if (len == 0) len = Bytecodes::length_at(_method(), bcp);
>> 197:       if (len <= 0 || (_bci > _end_bci - len) || (_bci - len >= _next_bci)) {
>> 198:         fatal("Should have been caught by verifier");
> 
> Verification may be disabled, This may be a risky change in behaviour.

If verification is disabled, this fatal is better than what might come next.  But there may be callers now or in the future that check for Bytecodes::_illegal, so I don't think we should have this assert even though we don't have any code that hits it.

> src/hotspot/share/oops/instanceKlass.cpp line 2459:
> 
>> 2457:   // don't have any methods share the Universe::_the_empty_method_array which is in the RO region.
>> 2458:   if (_methods != nullptr && _methods->length() > 0 && !can_be_verified_at_dumptime()) {
>> 2459:     // To handle jsr bytecode, new Method* maybe stored into _methods
> 
> Based on the comment we only need to do this if the method does contain a JSR bytecode - now we will do it for all cases where verification can't be done at dumptime. ??

Yes, we will put these in RW space the time.  Using the BytecodeStream to look for JSRs in unverified code is risky, so we avoid doing this optimization.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21330#discussion_r1788058857
PR Review Comment: https://git.openjdk.org/jdk/pull/21330#discussion_r1788063152