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

SA's vm object vtable matching code sometimes matches on incorrect type

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: 15, 16, 17, 18
    • Fix Version/s: 17
    • Component/s: hotspot
    • Labels:
      None
    • Subcomponent:
    • Resolved In Build:
      b30

      Backports

        Description

        When SA instantiates a wrapper class for a vm object, it tries to determine the type of the vm object so the correct SA wrapper class is used. This is done by looking at the vtable of the vm object. Given the address of a vm object, SA looks for the vtableptr at various predetermined locations within the vm object. Given what it thinks is a vtableptr, it then gets the native symbol for the vtableptr, and from the vtable native symbol it parses out the name of vm type that the vtable represents. This is done is a somewhat controlled way (you can't just instantiate a wrapper for any vm object). There are certain groups of vm types with common ancestry, and when you think you have a pointer to a vm object that is of a type in that group, you use a special wrapper constructor. For example, for JavaThreads we have the following code:

                virtualConstructor = new VirtualConstructor(db);
                // Add mappings for all known thread types
                virtualConstructor.addMapping("JavaThread", JavaThread.class);
                virtualConstructor.addMapping("CompilerThread", CompilerThread.class);
                virtualConstructor.addMapping("CodeCacheSweeperThread", CodeCacheSweeperThread.class);
                virtualConstructor.addMapping("JvmtiAgentThread", JvmtiAgentThread.class);
                virtualConstructor.addMapping("ServiceThread", ServiceThread.class);
                virtualConstructor.addMapping("MonitorDeflationThread", MonitorDeflationThread.class);
                virtualConstructor.addMapping("NotificationThread", NotificationThread.class);

            public JavaThread createJavaThreadWrapper(Address threadAddr) {
        ...
                    JavaThread thread = (JavaThread)virtualConstructor.instantiateWrapperFor(threadAddr);
        ...
                    return thread;
        ...
            }

        VirtualConstructor.instantiateWrapperFor(addr) iterates through these mappings, trying to find the one whose vtable matches the vtable of the object at the specified address. BasicTypeDataBase.addressTypeIsEqualToType(addr, type) is called for each of these mappings until a match is found. The problem is the code in addressTypeIsEqualToType() tries to be platform independent and search all possible locations for the vtableptr, not just the first word of the object. This can lead it to match on the specified type when it finds the vtableptr for that type, but at a location that is not actually where the vtableptr of the vm object is stored, and possibly is a location not even in the vm object.

        This is the search that VirtualConstructor.instantiateWrapperFor(addr) does:

             // 1. The first word of the object (should handle MSVC++ as
            // well as the solstudio compilers with compatibility set to
            // v5.0 or greater)
            // 2. and 3. The last two Address-aligned words of the part of
            // the object defined by its topmost polymorphic superclass.
            // This should handle the solstudio compilers, v4.2 or
            // earlier, as well as any other compilers which place the vptr
            // at the end of the user-defined fields of the first base
            // class with virtual functions.

        So if the first word is not a vtableptr for the specified type, it then also looks at the last two words of the object. But the last two words aren't known with certainty since we don't know the actual type of the object yet (we are trying to figure out if it matches the specified type, so initially we assume the object is of that type). So the code starts with the size of the specified type (this is known from vmstructs), adds that to the specified address, and then looks at the two words before it. This could very well be beyond the end of the object if the object is not actually of the type we are trying to match it to, and could very well point to the address where the vtableptr of the next object in memory is stored.

        So as an example (and relevant to all the related bugs failing due to this CR), if we have a pointer to a vm object of type JavaThread and check to see if it is CompilerThread before checking if it is a JavaThread, the check for the vtableptr at offset 0 will always fail, and we then look at the two words before addr + sizeof(CompilerThread). Since a CompilerThread is bigger than a JavaThread, we end up looking at words after the JavaThread, and could end up looking at the start of an CompilerThread that is right after the JavaThread. As a result a vtableptr match is found there, causing SA to think the JavaThread object is a CompilerThread object.

        We also have the following Solaris specific code adding to the confusion (this comment is right after the above comment):

            // Unfortunately this algorithm did not work properly for the
            // specific case of the ThreadShadow/Thread inheritance situation,
            // because the Solaris compiler seems to cleverly eliminate the
            // vtbl for ThreadShadow since the only virtual is empty. (We
            // should get rid of the ThreadShadow and fix the include
            // databases, but need to postpone this for the present.) The
            // current solution performs the three-location check for this
            // class and all of its known superclasses rather than just the
            // topmost polymorphic one.

        So if there is no match on the first pass, we then get the superclass type of the specified type, and use its size to determine where the two potential vtableptr fields are at the end of the object. Note the comment is a bit misleading in that we don't compare the potential vtableptrs to what is expected for the superclass type. We still only compare them to the vtableptr for the type passed in. This also means that the first check (looking at the address stored at the start of the object) is no different on these subsequent passes than what was already done on the first pass. In any case, this is all Solaris specific. No need to do it for other platforms.

        Since Solaris isn't supported anymore, we can remove the support for looking at the 2 addresses at the end of the object and the support doing the superclass retries. We'll just always assume the vtableptr is at the first address of the vm object. This greatly simplifies the code and fixes the issue.

          Attachments

            Issue Links

              Activity

                People

                Assignee:
                cjplummer Chris Plummer
                Reporter:
                cjplummer Chris Plummer
                Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved: