Address review comments

The newly introduced Compilation Server message type
VM_getStringUTF8UnabbreviatedLength is never acutally used.  This change
removes it along with the VM_getStringUTF8Length message type, which is
also never used.

This change also adjusts the maximum length for a UTF-8 encoding that
can be returned by getStringUTF8Length from 2^31-1 to 2^31-2.  That
ensures that any code that then uses that length to allocate buffer to
contain the encoded String with a NUL terminator will not overflow a
32-bit signed integer representation for the length plus the NUL byte.

Signed-off-by:  Henry Zongaro <[email protected]>

Author: hzongaro

runtime/compiler/control/JITClientCompilationThread.cpp

@@ -630,24 +630,6 @@ handleServerMessage(JITServer::ClientStream *client, TR_J9VM *fe, JITServer::Mes
          client->write(response, fe->stackWalkerMaySkipFrames(method, clazz));
          }
          break;
-      case MessageType::VM_getStringUTF8Length:
-         {
-         uintptr_t string = std::get<0>(client->getRecvData<uintptr_t>());
-            {
-            TR::VMAccessCriticalSection getStringUTF8Length(fe);
-            client->write(response, fe->getStringUTF8Length(string));
-            }
-         }
-         break;
-      case MessageType::VM_getStringUTF8UnabbreviatedLength:
-         {
-         uintptr_t string = std::get<0>(client->getRecvData<uintptr_t>());
-            {
-            TR::VMAccessCriticalSection getStringUTF8UnabbreviatedLength(fe);
-            client->write(response, fe->getStringUTF8UnabbreviatedLength(string));
-            }
-         }
-         break;
       case MessageType::VM_classInitIsFinished:
          {
          TR_OpaqueClassBlock *clazz = std::get<0>(client->getRecvData<TR_OpaqueClassBlock *>());

runtime/compiler/env/VMJ9.cpp

@@ -5656,7 +5656,9 @@ TR_J9VMBase::getStringUTF8Length(uintptr_t objectPointer)
    TR_ASSERT(objectPointer, "assertion failure");
    uint64_t actualLength = vmThread()->javaVM->internalVMFunctions->getStringUTF8LengthTruncated(vmThread(), (j9object_t)objectPointer, INT64_MAX);
 
-   TR_ASSERT_FATAL(actualLength <= std::numeric_limits<int32_t>::max(), "UTF8-encoded String length of " UINT64_PRINTF_FORMAT " must be in the range permitted for type int32_t.\n", actualLength);
+   // Fail if length+1 cannot be represented as an int32_t value.  The extra byte accounts for
+   // any NUL terminator that might be needed in copying the UTF-8 encoded string into a buffer
+   TR_ASSERT_FATAL(actualLength+1 <= std::numeric_limits<int32_t>::max(), "UTF8-encoded String length of " UINT64_PRINTF_FORMAT " must be in the range permitted for type int32_t, also allowing for a NUL terminator.\n", actualLength);
 
    return (int32_t) actualLength;
    }
@@ -5671,7 +5673,7 @@ TR_J9VMBase::getStringUTF8UnabbreviatedLength(uintptr_t objectPointer)
    }
 
 char *
-TR_J9VMBase::getStringUTF8(uintptr_t objectPointer, char *buffer, int32_t bufferSize)
+TR_J9VMBase::getStringUTF8(uintptr_t objectPointer, char *buffer, uintptr_t bufferSize)
    {
    TR_ASSERT(haveAccess(), "Must have VM access to call getStringUTF8");
 

runtime/compiler/env/VMJ9.h

@@ -1144,7 +1144,7 @@ class TR_J9VMBase : public TR_FrontEnd
    /**
     * \brief Returns the number of UTF-8 encoded bytes needed to represent a Java String object.
     *        The number of bytes needed to UTF-8 encode the String is representable as
-    *        a \c uintptr_t, in general, but this method returns a length of type \c int32_t.
+    *        a \c uint64_t, in general, but this method returns a length of type \c int32_t.
     *        If the length might exceed the range of \c int32_t, use
     *        \ref getStringUTF8UnabbreviatedLength instead.
     *
@@ -1162,7 +1162,7 @@ class TR_J9VMBase : public TR_FrontEnd
     * \return The number of UTF-8 encoded bytes needed to represent the String
     */
    virtual uint64_t  getStringUTF8UnabbreviatedLength(uintptr_t objectPointer);
-   virtual char     *getStringUTF8      (uintptr_t objectPointer, char *buffer, int32_t bufferSize);
+   virtual char     *getStringUTF8(uintptr_t objectPointer, char *buffer, uintptr_t bufferSize);
 
    virtual uint32_t getVarHandleHandleTableOffset(TR::Compilation *);
 

runtime/compiler/net/MessageTypes.cpp

@@ -121,8 +121,6 @@ const char *messageNames[] =
    "VM_getObjectClassFromKnownObjectIndexJLClass",
    "VM_getObjectClassInfoFromObjectReferenceLocation",
    "VM_stackWalkerMaySkipFrames",
-   "VM_getStringUTF8Length",
-   "VM_getStringUTF8UnabbreviatedLength",
    "VM_classInitIsFinished",
    "VM_getClassFromNewArrayType",
    "VM_getArrayClassFromComponentClass",

runtime/compiler/net/MessageTypes.hpp

@@ -130,8 +130,6 @@ enum MessageType : uint16_t
    VM_getObjectClassFromKnownObjectIndexJLClass,
    VM_getObjectClassInfoFromObjectReferenceLocation,
    VM_stackWalkerMaySkipFrames,
-   VM_getStringUTF8Length,
-   VM_getStringUTF8UnabbreviatedLength,
    VM_classInitIsFinished,
    VM_getClassFromNewArrayType,
    VM_getArrayClassFromComponentClass,