Correct handling of hidden class field comparisions in J9ServerVM

Using jitFieldsAreSame to check two different fields can yield false positives if the fields belong to two different hidden class instances within the same host class. By checking if the fields belong to hidden classes, we can prevent false positives when dealing with hidden class fields, which will prevent incorrect field shadow symref sharing. This commit includes the following: * Added TR_J9ServerVM override for isHiddenClass * Added check in TR_J9ServerVM::jitFieldsAreSame using this new helper Signed-off-by: Nazim Bhuiyan <[email protected]>
eclipse-openj9 · Nov 7, 2023 · a6584f8 · a6584f8
1 parent 9d68a73
commit a6584f8
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 1 deletion.
diff --git a/runtime/compiler/env/VMJ9Server.cpp b/runtime/compiler/env/VMJ9Server.cpp
@@ -468,7 +468,21 @@ TR_J9ServerVM::jitFieldsAreSame(TR_ResolvedMethod * method1, I_32 cpIndex1, TR_R
    {
    bool result = false;
    bool sigSame = true;
-   if (method1->fieldsAreSame(cpIndex1, method2, cpIndex2, sigSame))
+   TR_OpaqueClassBlock * classOfMethod1 = static_cast<TR_ResolvedJ9JITServerMethod *>(method1)->classOfMethod();
+   TR_OpaqueClassBlock * classOfMethod2 = static_cast<TR_ResolvedJ9JITServerMethod *>(method2)->classOfMethod();
+   // Hidden classes generated within the same host class do not have distinct class names,
+   // but share the same field names with different field data types and offsets. Therefore,
+   // name-based check for whether fields are same can result in false positives when it comes
+   // to hidden classes unless the fields are from the same j9class objects.
+   if (classOfMethod1
+       && classOfMethod2
+       && (isHiddenClass(classOfMethod1)
+          || isHiddenClass(classOfMethod2))
+       && classOfMethod1 != classOfMethod2)
+      {
+      result = false;
+      }
+   else if (method1->fieldsAreSame(cpIndex1, method2, cpIndex2, sigSame))
       {
       result = true;
       }
@@ -1943,6 +1957,16 @@ TR_J9ServerVM::isAnonymousClass(TR_OpaqueClassBlock *j9clazz)
    return (J9_ARE_ALL_BITS_SET(extraModifiers, J9AccClassAnonClass));
    }
 
+bool
+TR_J9ServerVM::isHiddenClass(TR_OpaqueClassBlock *j9clazz)
+   {
+   uintptr_t extraModifiers = 0;
+   JITServer::ServerStream *stream = _compInfoPT->getMethodBeingCompiled()->_stream;
+   JITServerHelpers::getAndCacheRAMClassInfo((J9Class *)j9clazz, _compInfoPT->getClientData(), stream, JITServerHelpers::CLASSINFO_ROMCLASS_EXTRAMODIFIERS, (void *)&extraModifiers);
+
+   return (J9_ARE_ALL_BITS_SET(extraModifiers, J9AccClassHidden));
+   }
+
 TR_IProfiler *
 TR_J9ServerVM::getIProfiler()
    {

diff --git a/runtime/compiler/env/VMJ9Server.hpp b/runtime/compiler/env/VMJ9Server.hpp
@@ -203,6 +203,7 @@ class TR_J9ServerVM: public TR_J9VM
    virtual bool transformJlrMethodInvoke(J9Method *callerMethod, J9Class *callerClass) override;
    using TR_J9VM :: isAnonymousClass;
    virtual bool isAnonymousClass(TR_OpaqueClassBlock *j9clazz) override;
+   virtual bool isHiddenClass(TR_OpaqueClassBlock *j9clazz) override;
    virtual TR_IProfiler *getIProfiler() override;
    virtual TR_StaticFinalData dereferenceStaticFinalAddress(void *staticAddress, TR::DataType addressType) override;
    virtual void reserveTrampolineIfNecessary( TR::Compilation *, TR::SymbolReference *symRef, bool inBinaryEncoding) override;