On Tue, 25 Jun 2024 16:45:49 GMT, Matias Saavedra Silva <matsa...@openjdk.org> 
wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains four additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'master' into 8309634-resolve-methods-at-dumptime
>>  - @calvinccheung and @matias9927 comments
>>  - Fixed whitespaces
>>  - 8309634: Resolve CONSTANT_MethodRef at CDS dump time
>
> src/hotspot/share/interpreter/interpreterRuntime.cpp line 930:
> 
>> 928:     CallInfo call_info;
>> 929:     switch (bytecode) {
>> 930:       case Bytecodes::_invokevirtual:   
>> LinkResolver::cds_resolve_virtual_call  (call_info, link_info, CHECK); break;
> 
> I think the the `cds_resolve_xyz_call()` methods might be unnecessary. You 
> can just call the existing methods from LinkResolver besides 
> `resolve_virtual_call`

I updated the code to make it easier to read. `cds_resolve_virtual_call()` 
needs to pass `is_abstract_interpretation=true` to 
`linktime_resolve_virtual_method()`. The reason is explained in this new 
comment:


  // is_abstract_interpretation is true IFF CDS is resolving method references 
without
  // running any actual bytecode. Therefore, we don't have an actual 
recv/recv_klass, so
  // we cannot check the actual selected_method (which is not needed by CDS 
anyway).


Without this guard, we will dereference 
`recv_klass->method_at_vtable(vtable_index)` and will get a SEGV because 
`recv_klass` is null.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19866#discussion_r1653865166

Reply via email to