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

Improper use of is_oop in production code

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P4
    • Resolution: Fixed
    • Affects Version/s: 9
    • Fix Version/s: 10
    • Component/s: hotspot
    • Labels:
      None
    • Subcomponent:
    • Resolved In Build:
      b22
    • CPU:
      generic
    • OS:
      generic

      Description

      is_oop() is commented as being only for use in asserts. Its actual usage is a little more broad than that, also being used in various verification and print functions.

      However, it is also called by java_lang_invoke_DirectMethodHandle::member in a somewhat convoluted way that isn't only for an assert. This is the *only* "production" use of is_oop.

      The existing implementation of DMH::member() is a bit more convoluted than this, but is equivalent to

      {code}
      oop java_lange_invoke_DirectMethodHandle::member(oop dmh) {
      #ifdef ASSERT
        assert(dmh->is_oop(), ...);
        assert(java_lang_invoke_DirectMethodHandle::is_instance(dmh), ...);
      #else
        if (!dmh->is_oop() || !java_lang_invoke_DirectMethodHandle::is_instance(dmh)) {
          return NULL;
        }
      #endif
        return dmh->obj_field(member_offset_in_bytes());
      }
      {code}

      So in a product build is_oop is being called to affect the result.

      The only call (in InterpreterRuntime::member_name_arg_or_null) looks like this:

      {code}
          oop member_name_oop = (oop) member_name;
          if (java_lang_invoke_DirectMethodHandle::is_instance(member_name_oop)) {
            // FIXME: remove after j.l.i.InvokerBytecodeGenerator code shape is updated.
            member_name_oop = java_lang_invoke_DirectMethodHandle::member(member_name_oop);
          }
      {code}

      So just the #ifdef ASSERT part in member() is sufficient, e.g.

      oop java_lange_invoke_DirectMethodHandle::member(oop dmh) {
        assert(dmh->is_oop(), ...);
        assert(java_lang_invoke_DirectMethodHandle::is_instance(dmh), ...);
        return dmh->obj_field(member_offset_in_bytes());
      }

      and then we have is_oop only being called from assert/verify/print contexts.

      That's of course assuming DMH::member is still needed at all, per the FIXME comment. According to the comment describing the caller, this bit of code exists for backward compatibility with JDK (7, 8). So it's possible the call to and implementation of DMH::member can be removed, which would again eliminate the non-assert call to is_oop.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                coleenp Coleen Phillimore
                Reporter:
                kbarrett Kim Barrett
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: