Refactor TR_AOTMethodInfo out of TR_InlinedCallSite

Previously, `TR_InlinedCallSite::_methodInfo` stored either a method pointer during a regular compilation, or `TR_AOTMethodInfo *` during an AOT compilation, despite `_methodInfo` having `TR_OpaqueMethodBlock *` type. This makes the code prone to subtle bugs like #5845. This commit moves storage of `TR_AOTMethodInfo *` outside of `TR_InlinedCallSite`, putting it in `TR_InlinedCallSiteInfo` which allows to simplify the definition of `TR_InlinedCallSite` and places where it is used. Closes: #5844 Signed-off-by: Dmitry Ten <Dmitry.Ten@ibm.com>
eclipse · Mar 16, 2021 · 3533681 · 3533681
1 parent 631fc1d
commit 3533681
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 57 deletions.
diff --git a/compiler/compile/OMRCompilation.cpp b/compiler/compile/OMRCompilation.cpp
@@ -1293,36 +1293,29 @@ bool OMR::Compilation::incInlineDepth(TR::ResolvedMethodSymbol * method, TR::Nod
    int32_t cpIndex = callSymRef->getCPIndex();
    TR_ByteCodeInfo &bcInfo = callNode->getByteCodeInfo();
 
+   TR_AOTMethodInfo *aotMethodInfo = NULL;
    if (self()->compileRelocatableCode())
       {
-      TR_AOTMethodInfo *aotMethodInfo = (TR_AOTMethodInfo *)self()->trMemory()->allocateHeapMemory(sizeof(TR_AOTMethodInfo));
+      aotMethodInfo = reinterpret_cast<TR_AOTMethodInfo *>(self()->trMemory()->allocateHeapMemory(sizeof(TR_AOTMethodInfo)));
       aotMethodInfo->resolvedMethod = method->getResolvedMethod();
       aotMethodInfo->cpIndex = cpIndex;
       aotMethodInfo->receiver = receiverClass;
       aotMethodInfo->callSymRef = callSymRef;
       aotMethodInfo->reloKind = self()->getReloTypeForMethodToBeInlined(guard, callNode, receiverClass);
-
-      return self()->incInlineDepth(reinterpret_cast<TR_OpaqueMethodBlock *>(aotMethodInfo), method, bcInfo, callSymRef, directCall, argInfo);
-      }
-   else
-      {
-      return self()->incInlineDepth(method->getResolvedMethod()->getPersistentIdentifier(), method, bcInfo, callSymRef, directCall, argInfo);
       }
+   return self()->incInlineDepth(method->getResolvedMethod()->getPersistentIdentifier(), method, bcInfo, callSymRef, directCall, argInfo, aotMethodInfo);
    }
 
 bool OMR::Compilation::incInlineDepth(TR::ResolvedMethodSymbol * method, TR_ByteCodeInfo & bcInfo, int32_t cpIndex, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo)
    {
+   TR_AOTMethodInfo *aotMethodInfo = NULL;
    if (self()->compileRelocatableCode())
       {
-      TR_AOTMethodInfo *aotMethodInfo = (TR_AOTMethodInfo *)self()->trMemory()->allocateHeapMemory(sizeof(TR_AOTMethodInfo));
+      aotMethodInfo = reinterpret_cast<TR_AOTMethodInfo *>(self()->trMemory()->allocateHeapMemory(sizeof(TR_AOTMethodInfo)));
       aotMethodInfo->resolvedMethod = method->getResolvedMethod();
       aotMethodInfo->cpIndex = cpIndex;
-      return self()->incInlineDepth((TR_OpaqueMethodBlock*)aotMethodInfo, method, bcInfo, callSymRef, directCall, argInfo);
-      }
-   else
-      {
-      return self()->incInlineDepth(method->getResolvedMethod()->getPersistentIdentifier(), method, bcInfo, callSymRef, directCall, argInfo);
       }
+   return self()->incInlineDepth(method->getResolvedMethod()->getPersistentIdentifier(), method, bcInfo, callSymRef, directCall, argInfo, aotMethodInfo);
    }
 
 ncount_t OMR::Compilation::generateAccurateNodeCount()
@@ -1357,7 +1350,7 @@ bool OMR::Compilation::foundOnTheStack(TR_ResolvedMethod *method, int32_t occurr
   return false;
   }
 
-bool OMR::Compilation::incInlineDepth(TR_OpaqueMethodBlock *methodInfo, TR::ResolvedMethodSymbol * method, TR_ByteCodeInfo & bcInfo, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo)
+bool OMR::Compilation::incInlineDepth(TR_OpaqueMethodBlock *methodInfo, TR::ResolvedMethodSymbol * method, TR_ByteCodeInfo & bcInfo, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo, TR_AOTMethodInfo *aotMethodInfo)
    {
    int32_t maxCallerIndex = TR_ByteCodeInfo::maxCallerIndex;
    //This restriction is due to a limited number of bits allocated to callerIndex in TR_ByteCodeInfo
@@ -1381,7 +1374,7 @@ bool OMR::Compilation::incInlineDepth(TR_OpaqueMethodBlock *methodInfo, TR::Reso
       }
 
 
-   uint32_t callSiteIndex = _inlinedCallSites.add( TR_InlinedCallSiteInfo(methodInfo, bcInfo, method, callSymRef, directCall) );
+   uint32_t callSiteIndex = _inlinedCallSites.add( TR_InlinedCallSiteInfo(methodInfo, bcInfo, method, callSymRef, directCall, aotMethodInfo) );
    _inlinedCallStack.push(callSiteIndex);
    _inlinedCallArgInfoStack.push(argInfo);
 
@@ -1798,7 +1791,7 @@ TR_OpaqueMethodBlock *OMR::Compilation::getMethodFromNode(TR::Node * node)
    TR_ByteCodeInfo bcInfo = node->getByteCodeInfo();
    TR_OpaqueMethodBlock *method = NULL;
    if (bcInfo.getCallerIndex() >= 0 && self()->getNumInlinedCallSites() > 0)
-      method = self()->compileRelocatableCode() ? ((TR_ResolvedMethod *)node->getAOTMethod())->getPersistentIdentifier() : self()->getInlinedCallSite(bcInfo.getCallerIndex())._methodInfo;
+      method = self()->getInlinedCallSite(bcInfo.getCallerIndex())._methodInfo;
    else
       method = self()->getCurrentMethod()->getPersistentIdentifier();
    return method;
@@ -2404,6 +2397,12 @@ OMR::Compilation::setCannotAttemptOSRDuring(uint32_t index, bool cannotOSR)
    _inlinedCallSites[index].setCannotAttemptOSRDuring(cannotOSR);
    }
 
+TR_AOTMethodInfo *
+OMR::Compilation::getInlinedAOTMethodInfo(uint32_t index)
+   {
+   return _inlinedCallSites[index].aotMethodInfo();
+   }
+
 TR_InlinedCallSite *
 OMR::Compilation::getCurrentInlinedCallSite()
    {

diff --git a/compiler/compile/OMRCompilation.hpp b/compiler/compile/OMRCompilation.hpp
@@ -621,7 +621,6 @@ class OMR_EXTENSIBLE Compilation
    uint16_t getMaxInlineDepth() {return _maxInlineDepth;}
    bool incInlineDepth(TR::ResolvedMethodSymbol *, TR::Node *callNode, bool directCall, TR_VirtualGuardSelection *guard, TR_OpaqueClassBlock *receiverClass, TR_PrexArgInfo *argInfo = 0);
    bool incInlineDepth(TR::ResolvedMethodSymbol *, TR_ByteCodeInfo &, int32_t cpIndex, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo = 0);
-   bool incInlineDepth(TR_OpaqueMethodBlock *, TR::ResolvedMethodSymbol *, TR_ByteCodeInfo &, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo = 0);
    void decInlineDepth(bool removeInlinedCallSitesEntries = false);
    int16_t  adjustInlineDepth(TR_ByteCodeInfo & bcInfo);
    void     resetInlineDepth();
@@ -631,6 +630,11 @@ class OMR_EXTENSIBLE Compilation
    int32_t getInlinedCalls() { return _inlinedCalls; }
    void incInlinedCalls() { _inlinedCalls++; }
 
+
+protected:
+   bool incInlineDepth(TR_OpaqueMethodBlock *, TR::ResolvedMethodSymbol *, TR_ByteCodeInfo &, TR::SymbolReference *callSymRef, bool directCall, TR_PrexArgInfo *argInfo, TR_AOTMethodInfo *aotMethodInfo);
+
+public:
    TR_ExternalRelocationTargetKind getReloTypeForMethodToBeInlined(TR_VirtualGuardSelection *guard, TR::Node *callNode, TR_OpaqueClassBlock *receiverClass) { return TR_NoRelocation; }
 
    class TR_InlinedCallSiteInfo
@@ -641,17 +645,24 @@ class OMR_EXTENSIBLE Compilation
       int32_t *_osrCallSiteRematTable;
       bool _directCall;
       bool _cannotAttemptOSRDuring;
+      TR_AOTMethodInfo *_aotMethodInfo;
 
       public:
 
-      TR_InlinedCallSiteInfo(TR_OpaqueMethodBlock *methodInfo,
+      TR_InlinedCallSiteInfo(TR_OpaqueMethodBlock *method,
                              TR_ByteCodeInfo &bcInfo,
                              TR::ResolvedMethodSymbol *resolvedMethod,
                              TR::SymbolReference *callSymRef,
-                             bool directCall):
-         _resolvedMethod(resolvedMethod), _callSymRef(callSymRef), _directCall(directCall), _osrCallSiteRematTable(0), _cannotAttemptOSRDuring(false)
+                             bool directCall,
+                             TR_AOTMethodInfo *aotMethodInfo = NULL):
+         _resolvedMethod(resolvedMethod),
+         _callSymRef(callSymRef),
+         _directCall(directCall),
+         _osrCallSiteRematTable(0),
+         _cannotAttemptOSRDuring(false),
+         _aotMethodInfo(aotMethodInfo)
          {
-         _site._methodInfo = methodInfo;
+         _site._methodInfo = method;
          _site._byteCodeInfo = bcInfo;
          }
 
@@ -664,6 +675,7 @@ class OMR_EXTENSIBLE Compilation
       bool directCall() { return _directCall; }
       bool cannotAttemptOSRDuring() { return _cannotAttemptOSRDuring; }
       void setCannotAttemptOSRDuring(bool cannotOSR) { _cannotAttemptOSRDuring = cannotOSR; }
+      TR_AOTMethodInfo *aotMethodInfo() { return _aotMethodInfo; }
       };
 
    uint32_t getNumInlinedCallSites();
@@ -678,6 +690,7 @@ class OMR_EXTENSIBLE Compilation
    bool isInlinedDirectCall(uint32_t index);
    bool cannotAttemptOSRDuring(uint32_t index);
    void setCannotAttemptOSRDuring(uint32_t index, bool cannot);
+   TR_AOTMethodInfo *getInlinedAOTMethodInfo(uint32_t index);
 
    TR_InlinedCallSite *getCurrentInlinedCallSite();
    int32_t getCurrentInlinedSiteIndex();

diff --git a/compiler/compile/OSRData.cpp b/compiler/compile/OSRData.cpp
@@ -461,12 +461,7 @@ void TR_OSRCompilationData::checkOSRLimits()
       TR_InlinedCallSite &callSite = comp->getInlinedCallSite(i);
       TR_ByteCodeInfo &bcInfo = callSite._byteCodeInfo;
 
-      TR_OpaqueMethodBlock *methodInfo
-            = comp->compileRelocatableCode()
-              ? reinterpret_cast<TR_AOTMethodInfo *>(callSite._methodInfo)->resolvedMethod->getPersistentIdentifier()
-              : callSite._methodInfo;
-
-      frameSizes[i] = TR::Compiler->vm.OSRFrameSizeInBytes(comp, methodInfo);
+      frameSizes[i] = TR::Compiler->vm.OSRFrameSizeInBytes(comp, callSite._methodInfo);
       frameSizes[i] += (bcInfo.getCallerIndex() == -1) ? rootFrameSize : frameSizes[bcInfo.getCallerIndex()];
       stackFrameSizes[i] = getOSRStackFrameSize(i + 1);
       stackFrameSizes[i] = (bcInfo.getCallerIndex() == -1) ? rootStackFrameSize : stackFrameSizes[bcInfo.getCallerIndex()];

diff --git a/compiler/env/FrontEnd.cpp b/compiler/env/FrontEnd.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -266,7 +266,7 @@ TR_FrontEnd::getLineNumberForMethodAndByteCodeIndex(TR_OpaqueMethodBlock *, int3
 TR_OpaqueMethodBlock *
 TR_FrontEnd::getInlinedCallSiteMethod(TR_InlinedCallSite *ics)
    {
-   return (TR_OpaqueMethodBlock *)(ics->_vmMethodInfo);
+   return ics->_methodInfo;
    }
 
 TR_OpaqueClassBlock *

diff --git a/compiler/env/jittypes.h b/compiler/env/jittypes.h
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -125,7 +125,6 @@ typedef struct TR_ByteCodeInfo
 typedef struct TR_InlinedCallSite
    {
    TR_OpaqueMethodBlock   * _methodInfo;
-   #define _vmMethodInfo _methodInfo
    struct TR_ByteCodeInfo   _byteCodeInfo;
    } TR_InlinedCallSite;
 

diff --git a/compiler/il/OMRNode.cpp b/compiler/il/OMRNode.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -3391,21 +3391,10 @@ TR_OpaqueMethodBlock*
 OMR::Node::getOwningMethod(TR::Compilation *comp, TR_ByteCodeInfo &bcInfo)
    {
    TR_OpaqueMethodBlock *method = NULL; 
-
-   if (comp->compileRelocatableCode()) 
-      { 
-      if (0 <= bcInfo.getCallerIndex()) 
-         method = (TR_OpaqueMethodBlock *)(((TR_AOTMethodInfo *)comp->getInlinedCallSite(bcInfo.getCallerIndex())._vmMethodInfo)->resolvedMethod->getPersistentIdentifier()); 
-      else 
-         method = (TR_OpaqueMethodBlock *)(comp->getCurrentMethod()->getPersistentIdentifier()); 
-      } 
-   else  
-      { 
-      if (0 <= bcInfo.getCallerIndex()) 
-         method = (TR_OpaqueMethodBlock *)(comp->getInlinedCallSite(bcInfo.getCallerIndex())._vmMethodInfo); 
-      else 
-         method = (TR_OpaqueMethodBlock *)(comp->getCurrentMethod()->getNonPersistentIdentifier()); 
-      } 
+   if (0 <= bcInfo.getCallerIndex()) 
+      method = comp->getInlinedCallSite(bcInfo.getCallerIndex())._methodInfo; 
+   else 
+      method = comp->getCurrentMethod()->getPersistentIdentifier(); 
 
    return method; 
    }
@@ -3423,8 +3412,7 @@ OMR::Node::getAOTMethod()
       }
    else
       {
-      TR_AOTMethodInfo *aotMethodInfo = (TR_AOTMethodInfo *)c->getInlinedCallSite(index)._methodInfo;
-      return (void*)aotMethodInfo->resolvedMethod;
+      return c->getInlinedResolvedMethod(index);
       }
    }
 

diff --git a/compiler/optimizer/RedundantAsyncCheckRemoval.cpp b/compiler/optimizer/RedundantAsyncCheckRemoval.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -900,7 +900,7 @@ bool TR_RedundantAsyncCheckRemoval::originatesFromShortRunningMethod(TR_RegionSt
 	    }
 	 TR_InlinedCallSite &ics = comp()->getInlinedCallSite(callerIndex);
 	 if (!comp()->isShortRunningMethod(callerIndex) &&
-	     TR::Compiler->mtd.hasBackwardBranches((TR_OpaqueMethodBlock*)ics._vmMethodInfo))
+	     TR::Compiler->mtd.hasBackwardBranches(ics._methodInfo))
 	    break;
 	 //set callerIndex to its caller
 	 callerIndex = comp()->getInlinedCallSite(callerIndex)._byteCodeInfo.getCallerIndex();

diff --git a/compiler/ras/DebugCounter.cpp b/compiler/ras/DebugCounter.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this

diff --git a/compiler/ras/Tree.cpp b/compiler/ras/Tree.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corp. and others
+ * Copyright (c) 2000, 2021 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -896,10 +896,7 @@ TR_Debug::printIRTrees(TR::FILE *pOutFile, const char * title, TR::ResolvedMetho
          if (targetMethodHandleIndex != TR::KnownObjectTable::UNKNOWN)
             trfprintf(pOutFile, "obj%d.", targetMethodHandleIndex);
 
-         trfprintf(pOutFile, "%s\n",
-            _comp->compileRelocatableCode() ?
-                       ((TR_AOTMethodInfo *)ics._methodInfo)->resolvedMethod->signature(comp()->trMemory(), heapAlloc) :
-                       fe()->sampleSignature(ics._methodInfo, 0, 0, _comp->trMemory()));
+         trfprintf(pOutFile, "%s\n", fe()->sampleSignature(ics._methodInfo, 0, 0, _comp->trMemory()));
 
          if (debug("printInlinePath"))
             {