From adba303720bf1c502d1b0145651102e437770204 Mon Sep 17 00:00:00 2001
From: Romaric Jodin <rjodin@chromium.org>
Date: Tue, 26 Nov 2024 20:04:26 +0100
Subject: [PATCH] fix lifetime of cvk_entry_point (#721)

cvk_entry_point are passed to other structures as a pointer. But it
can be deleted at any point when do_build it called.

To avoid invalid access, make entry point a shared_ptr in the program
map.

This has been detected running the compiler suite of the CTS on
ChromeOS (flaky behaviour).
---
 src/kernel.hpp  |  8 ++++----
 src/program.cpp | 14 ++++++--------
 src/program.hpp |  6 +++---
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/kernel.hpp b/src/kernel.hpp
index 02e2c7c6..533f8fe6 100644
--- a/src/kernel.hpp
+++ b/src/kernel.hpp
@@ -161,7 +161,7 @@ struct cvk_kernel : public _cl_kernel, api_object<object_magic::kernel> {
 
     std::mutex m_lock;
     cvk_program_holder m_program;
-    cvk_entry_point* m_entry_point;
+    std::shared_ptr<cvk_entry_point> m_entry_point;
     std::string m_name;
     std::vector<kernel_argument> m_args;
     std::shared_ptr<cvk_kernel_argument_values> m_argument_values;
@@ -177,7 +177,7 @@ using cvk_kernel_holder = refcounted_holder<cvk_kernel>;
 
 struct cvk_kernel_argument_values {
 
-    cvk_kernel_argument_values(cvk_entry_point* entry_point)
+    cvk_kernel_argument_values(std::shared_ptr<cvk_entry_point> entry_point)
         : m_entry_point(entry_point), m_is_enqueued(false),
           m_args(m_entry_point->args()), m_pod_arg(nullptr),
           m_kernel_resources(m_entry_point->num_resource_slots()),
@@ -203,7 +203,7 @@ struct cvk_kernel_argument_values {
     }
 
     static std::shared_ptr<cvk_kernel_argument_values>
-    create(cvk_entry_point* entry_point) {
+    create(std::shared_ptr<cvk_entry_point> entry_point) {
         auto val = std::make_shared<cvk_kernel_argument_values>(entry_point);
 
         if (!val->init()) {
@@ -416,7 +416,7 @@ struct cvk_kernel_argument_values {
     }
 
     std::mutex m_lock;
-    cvk_entry_point* m_entry_point;
+    std::shared_ptr<cvk_entry_point> m_entry_point;
     std::unique_ptr<std::vector<uint8_t>> m_pod_data;
     bool m_is_enqueued;
     const std::vector<kernel_argument>& m_args;
diff --git a/src/program.cpp b/src/program.cpp
index feeafa5a..da372602 100644
--- a/src/program.cpp
+++ b/src/program.cpp
@@ -1731,28 +1731,26 @@ cvk_entry_point::cvk_entry_point(cvk_device* dev, cvk_program* program,
     TRACE_CNT(descriptor_set_allocated_counter, 0);
 }
 
-cvk_entry_point* cvk_program::get_entry_point(std::string& name,
-                                              cl_int* errcode_ret) {
+std::shared_ptr<cvk_entry_point>
+cvk_program::get_entry_point(std::string& name, cl_int* errcode_ret) {
     std::lock_guard<std::mutex> lock(m_lock);
 
     // Check for existing entry point in cache
     if (m_entry_points.count(name)) {
         *errcode_ret = CL_SUCCESS;
-        return m_entry_points.at(name).get();
+        return m_entry_points.at(name);
     }
 
     // Create and initialize entry point
-    cvk_entry_point* entry_point =
-        new cvk_entry_point(m_context->device(), this, name);
+    std::shared_ptr<cvk_entry_point> entry_point =
+        std::make_shared<cvk_entry_point>(m_context->device(), this, name);
     *errcode_ret = entry_point->init();
     if (*errcode_ret != CL_SUCCESS) {
-        delete entry_point;
         return nullptr;
     }
 
     // Add to cache for reuse by other kernels
-    m_entry_points.insert(
-        {name, std::unique_ptr<cvk_entry_point>(entry_point)});
+    m_entry_points.insert({name, entry_point});
 
     return entry_point;
 }
diff --git a/src/program.hpp b/src/program.hpp
index d786819c..75fa824a 100644
--- a/src/program.hpp
+++ b/src/program.hpp
@@ -799,8 +799,8 @@ struct cvk_program : public _cl_program, api_object<object_magic::program> {
 
     const VkPipelineCache& pipeline_cache() const { return m_pipeline_cache; }
 
-    CHECK_RETURN cvk_entry_point* get_entry_point(std::string& name,
-                                                  cl_int* errcode_ret);
+    CHECK_RETURN std::shared_ptr<cvk_entry_point>
+    get_entry_point(std::string& name, cl_int* errcode_ret);
 
     bool create_module_constant_data_buffer() {
         cl_int err;
@@ -901,7 +901,7 @@ struct cvk_program : public _cl_program, api_object<object_magic::program> {
     std::string m_build_log;
     std::vector<cvk_sampler_holder> m_literal_samplers;
     VkPushConstantRange m_push_constant_range;
-    std::unordered_map<std::string, std::unique_ptr<cvk_entry_point>>
+    std::unordered_map<std::string, std::shared_ptr<cvk_entry_point>>
         m_entry_points;
     std::vector<uint32_t> m_stripped_binary;
     VkPipelineCache m_pipeline_cache;