Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8194409

[Nestmates] Update cpCache and related handling for invokeinterface of private interface methods



    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: repo-valhalla
    • Fix Version/s: repo-valhalla
    • Component/s: hotspot
    • Labels:


      When we switched to using invokeinterface, instead of invokespecial, for private interface method invocations, it all "just worked" (more or less). But it turns out that it works in relation to the interpreter and cpCache in a way that causes the logic to follow a path reserved for the extreme corner case of invokeinterface targetting a non-public method of java.lang.Object. With the fix for JDK-8154587 that path will now hit an assertion failure if the target method is not from Object.

      Karen writes:

      So to be more precise - final methods and private methods do NOT get an entry in the vtable or itable.
      So the vtable/itable are selection caches - which we do not look in for final or private methods.

      We do however look in the resolution cache - which is the cpCache, which is set up by the linkResolver CallInfo::set_xxx logic.
      For private methods we set a special bit in the cpCache - we set the is_vfinal flag, and set_f2_as_vfinal_method so we store the selected == resolved method directly
      in the cpCache and future invocations go there directly rather than indexing through the selection caches.

      Today we do this special handling for invoke virtual of a private method - and I think we want to extend this model to also work for invokeinterface

      Let’s do the invoke virtual case for a private method. For the interpreter, start in
           calls resolve_cache_and_index
             get_cache_and_index_and_bytecode_at_bcp(Rcache, index, temp, byte_no, 1, index_size)
               1st time through: InterpreterRuntime::resolve_from_cache (interpreter/interpreterRuntime.cpp)
                   LinkResolver::resolve_invoke(CallInfo, receiver, pool, cpcache, byte code)
                     invoke virtual specific:
                          runtime_resolve_virtual_method: see details below: if vtable_index == nonvirtual_vtable_index, can_be_statically_bound: selected_method = resolved_method
                             set_common: CallInfo::direct_call for private rather than CallInfo::vtable_call, which just sets up CallInfo to return
                  if cp_cache_entry->is_resolved(byte code): return
                  first time through: switch on CallInfo->kind
                     cp_cache_entry->set_direct/vtable/itable_call -> this is where we set up the cpCache based on CallInfo from LinkResolver (invoke virtual specific)
                         set_direct_or_vtable_call with index == Method::nonvirtual_vtable_index
                             invoke virtual: if !is_vtable_call, set_method_flags: turn on is_vfinal (this is for the private case)
                             set_f2_as_vfinal_method: so f2 will contain Method* directly (note: invoke virtual uses F2, invoke special uses f1, invoke interface can use both so can not share entries) /resolve_fro
              everytime: calls get_cache_and_index_at_bpc (interp_masm_x86.cpp): returns cpCache index
           TemplateTable::invokevirtual_helper(cpCache index, recv, flags) (invoke virtual specific)
              if is_vfinal_shift
                method = index
                jump_from_interpreted(method, rax)

      So the special handing here for private methods is in three places:
         LinkResolver: CallInfo::direct_call
         cpCache: turn on is_vfinal
         templateTable:: check for is_vfinal to jump directly

      My suggestion is that we need to have the equivalent special handling for private methods for invoke interface
      TemplateTable::invoke interface
        when we return, there is no invokeinterface_helper - we check for is_forced_virtual_shift and switch to invokevirtual
          - recommend: before that check - check is_vfinal_shift - and if so do exactly what invokevirtual_helper does for is_vfinal_shift
            (I might refactor that small part to be vfinal_helper - maybe)

        in LinkResolver::
              I think you already tried this change: if vtable_index == nonvirtual_vtable_index, assert can_be_statically_bound and call
              your choice - not sure the best way to factor this (it is sort of convoluted to me right now)
               - the goal is to come out with kind for set_direct

         in constantPoolCache.cpp: ConstantPoolCacheEntry::set_direct -> set_direct_or_vtable_call with index == Method::nonvirtual_vtable_index
              in the switch case invoke interface:
                you have two cases here now:
                1) method has a vtable_index — current case -> change_to_virtual = true
                2) (more common - I would check it first): nonvirtual_vtable_index
                     steal code for invoke virtual !is_vtable_call: I believe the key is to set is_vfinal (not is_forced_virtual, not is_final)
                     and set_f2_as_vfinal_method(method()

      I think that set of three changes should get you where you actually want to be.




            dholmes David Holmes
            dholmes David Holmes
            0 Vote for this issue
            1 Start watching this issue