SWDEV-313680 - hipGraphAddKernelNode invalid cases

Change-Id: I4c87c5dd80cd3f78f5c38b2d3447b03b91ffa656


[ROCm/clr commit: ef71f0ded1]

projects/clr/hipamd/src/hip_graph.cpp

@@ -25,31 +25,25 @@
 #include "hip_event.hpp"
 
 thread_local std::vector<hipStream_t> g_captureStreams;
-static std::unordered_set<ihipGraph*> graphSet;
-static amd::Monitor graphSetLock{"Guards global graph set"};
 
-ihipGraph::ihipGraph() {
-  amd::ScopedLock lock(graphSetLock);
-  graphSet.insert(this);
-}
-
-ihipGraph::~ihipGraph() {
-  for (auto node : vertices_) {
-    delete node;
-  }
-  amd::ScopedLock lock(graphSetLock);
-  graphSet.erase(this);
-}
-
-inline void ihipGraphAddNode(hipGraphNode_t graphNode, hipGraph_t graph,
+inline hipError_t ihipGraphAddNode(hipGraphNode_t graphNode, hipGraph_t graph,
                              const hipGraphNode_t* pDependencies, size_t numDependencies) {
   graph->AddNode(graphNode);
   for (size_t i = 0; i < numDependencies; i++) {
+    if (!hipGraphNode::isNodeValid(pDependencies[i])) {
+      return hipErrorInvalidValue;
+    }
     pDependencies[i]->AddEdge(graphNode);
   }
+  return hipSuccess;
 }
 
+
 hipError_t ihipValidateKernelParams(const hipKernelNodeParams* pNodeParams) {
+
+  if (pNodeParams->kernelParams == nullptr) {
+    return hipErrorInvalidValue;
+  }
   hipFunction_t func = nullptr;
   hipError_t status =
       PlatformState::instance().getStatFunc(&func, pNodeParams->func, ihipGetDevice());
@@ -83,6 +77,9 @@ hipError_t ihipGraphAddKernelNode(hipGraphNode_t* pGraphNode, hipGraph_t graph,
       pNodeParams->func == nullptr) {
     return hipErrorInvalidValue;
   }
+  if (!ihipGraph::isGraphValid(graph)) {
+    return hipErrorInvalidValue;
+  }
   hipError_t status = ihipValidateKernelParams(pNodeParams);
   if (hipSuccess != status) {
     return status;
@@ -93,8 +90,8 @@ hipError_t ihipGraphAddKernelNode(hipGraphNode_t* pGraphNode, hipGraph_t graph,
     return hipErrorInvalidDeviceFunction;
   }
   *pGraphNode = new hipGraphKernelNode(pNodeParams, func);
-  ihipGraphAddNode(*pGraphNode, graph, pDependencies, numDependencies);
-  return hipSuccess;
+  status = ihipGraphAddNode(*pGraphNode, graph, pDependencies, numDependencies);
+  return status;
 }
 
 hipError_t ihipGraphAddMemcpyNode(hipGraphNode_t* pGraphNode, hipGraph_t graph,
@@ -811,6 +808,7 @@ hipError_t hipGraphDestroy(hipGraph_t graph) {
   HIP_RETURN(hipSuccess);
 }
 
+
 hipError_t hipGraphAddKernelNode(hipGraphNode_t* pGraphNode, hipGraph_t graph,
                                  const hipGraphNode_t* pDependencies, size_t numDependencies,
                                  const hipKernelNodeParams* pNodeParams) {
@@ -820,7 +818,6 @@ hipError_t hipGraphAddKernelNode(hipGraphNode_t* pGraphNode, hipGraph_t graph,
       (numDependencies > 0 && pDependencies == nullptr)) {
     HIP_RETURN(hipErrorInvalidValue);
   }
-
   HIP_RETURN_DURATION(
       ihipGraphAddKernelNode(pGraphNode, graph, pDependencies, numDependencies, pNodeParams));
 }
@@ -1399,20 +1396,14 @@ hipError_t hipGraphDestroyNode(hipGraphNode_t node) {
   HIP_RETURN(hipSuccess);
 }
 
-bool isGraphValid(ihipGraph* pGraph) {
-  amd::ScopedLock lock(graphSetLock);
-  if (graphSet.find(pGraph) == graphSet.end()) {
-    return false;
-  }
-  return true;
-}
+
 
 hipError_t hipGraphClone(hipGraph_t* pGraphClone, hipGraph_t originalGraph) {
   HIP_INIT_API(hipGraphClone, pGraphClone, originalGraph);
   if (originalGraph == nullptr || pGraphClone == nullptr) {
     HIP_RETURN(hipErrorInvalidValue);
   }
-  if (!isGraphValid(originalGraph)) {
+  if (!ihipGraph::isGraphValid(originalGraph)) {
     HIP_RETURN(hipErrorInvalidValue);
   }
   *pGraphClone = originalGraph->clone();

projects/clr/hipamd/src/hip_graph_internal.cpp

@@ -48,6 +48,8 @@ const char* GetGraphNodeTypeString(uint32_t op) {
 int hipGraphNode::nextID = 0;
 std::unordered_set<hipGraphNode*> hipGraphNode::nodeSet_;
 amd::Monitor hipGraphNode::nodeSetLock_{"Guards global node set"};
+std::unordered_set<ihipGraph*> ihipGraph::graphSet_;
+amd::Monitor ihipGraph::graphSetLock_{"Guards global graph set"};
 
 hipError_t hipGraphMemcpyNode1D::ValidateParams(void* dst, const void* src, size_t count,
                                                 hipMemcpyKind kind) {
@@ -467,6 +469,16 @@ hipError_t hipGraphMemcpyNode::SetCommandParams(const hipMemcpy3DParms* pNodePar
   return hipSuccess;
 }
 
+
+bool ihipGraph::isGraphValid(ihipGraph* pGraph) {
+  amd::ScopedLock lock(graphSetLock_);
+  if (graphSet_.find(pGraph) == graphSet_.end()) {
+    return false;
+  }
+  return true;
+}
+
+
 void ihipGraph::AddNode(const Node& node) {
   vertices_.emplace_back(node);
   ClPrint(amd::LOG_INFO, amd::LOG_CODE, "[hipGraph] Add %s(%p)\n",

projects/clr/hipamd/src/hip_graph_internal.hpp

@@ -231,12 +231,25 @@ struct hipGraphNode {
 struct ihipGraph {
   std::vector<Node> vertices_;
   const ihipGraph* pOriginalGraph_ = nullptr;
+  static std::unordered_set<ihipGraph*> graphSet_;
+  static amd::Monitor graphSetLock_ ;
 
  public:
- ihipGraph();
- ~ihipGraph();
+  ihipGraph() {
+    amd::ScopedLock lock(graphSetLock_);
+    graphSet_.insert(this);
+  };
+
+  ~ihipGraph() {
+    for (auto node : vertices_) {
+      delete node;
+    }
+    amd::ScopedLock lock(graphSetLock_);
+    graphSet_.erase(this);
+  };
+
   // check graphs validity
-  bool isGraphValid(ihipGraph* pGraph);
+  static bool isGraphValid(ihipGraph* pGraph);
 
   /// add node to the graph
   void AddNode(const Node& node);