Harden shader cache against memory corruption

Valgrind reports that the vectors used in creating shaders are deleted
before being used in graphics implementation.

Change-Id: Ife99c84530ebe1b373407bdf909c4da6a5489c94

diff --git a/dali/internal/render/renderers/render-renderer.cpp b/dali/internal/render/renderers/render-renderer.cpp
index 2915ad5..014936a 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.cpp
+++ b/dali/internal/render/renderers/render-renderer.cpp
@@ -777,16 +777,16 @@ void Renderer::Render(Context&                                             conte
   // Get the program to use
   // The program cache owns the Program object so we don't need to worry about this raw allocation here.
   ShaderDataPtr shaderData = mRenderDataProvider->GetShader().GetShaderData();
-
+  const std::vector<char>& vertShader = shaderData->GetShaderForPipelineStage(Graphics::PipelineStage::VERTEX_SHADER);
+  const std::vector<char>& fragShader = shaderData->GetShaderForPipelineStage(Graphics::PipelineStage::FRAGMENT_SHADER);
   Dali::Graphics::Shader& vertexShader = mShaderCache->GetShader(
-    shaderData->GetShaderForPipelineStage(Graphics::PipelineStage::VERTEX_SHADER),
+    vertShader,
     Graphics::PipelineStage::VERTEX_SHADER,
     shaderData->GetSourceMode());
 
-  Dali::Graphics::Shader& fragmentShader = mShaderCache->GetShader(
-    shaderData->GetShaderForPipelineStage(Graphics::PipelineStage::FRAGMENT_SHADER),
-    Graphics::PipelineStage::FRAGMENT_SHADER,
-    shaderData->GetSourceMode());
+  Dali::Graphics::Shader& fragmentShader = mShaderCache->GetShader( fragShader,
+                                                                    Graphics::PipelineStage::FRAGMENT_SHADER,
+                                                                    shaderData->GetSourceMode());
 
   std::vector<Graphics::ShaderState> shaderStates{
     Graphics::ShaderState()

diff --git a/dali/internal/render/renderers/render-renderer.h b/dali/internal/render/renderers/render-renderer.h
index 0df272e..09694b4 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.h
+++ b/dali/internal/render/renderers/render-renderer.h
@@ -493,8 +493,8 @@ private:
 
   Graphics::UniquePtr<Graphics::CommandBuffer> mGraphicsCommandBuffer{};
 
-  ProgramCache*        mProgramCache{};
-  Render::ShaderCache* mShaderCache{};
+  ProgramCache*        mProgramCache{nullptr};
+  Render::ShaderCache* mShaderCache{nullptr};
 
   Render::UniformBufferManager*               mUniformBufferManager{};
   std::vector<Graphics::UniformBufferBinding> mUniformBufferBindings{};

diff --git a/dali/internal/render/renderers/shader-cache.cpp b/dali/internal/render/renderers/shader-cache.cpp
index 2a86735..d4f5f16 100644 (file)
--- a/dali/internal/render/renderers/shader-cache.cpp
+++ b/dali/internal/render/renderers/shader-cache.cpp
@@ -28,7 +28,7 @@ ShaderCache::ShaderCache(Dali::Graphics::Controller& controller)
 {
 }
 
-Dali::Graphics::Shader& ShaderCache::GetShader(const std::vector<char> shaderCode, Graphics::PipelineStage stage, Graphics::ShaderSourceMode type)
+Dali::Graphics::Shader& ShaderCache::GetShader(const std::vector<char>& shaderCode, Graphics::PipelineStage stage, Graphics::ShaderSourceMode type)
 {
   for(auto&& item : mItems)
   {
@@ -44,11 +44,10 @@ Dali::Graphics::Shader& ShaderCache::GetShader(const std::vector<char> shaderCod
   shaderCreateInfo.SetSourceSize(shaderCode.size());
   shaderCreateInfo.SetSourceMode(type);
 
-  auto shader = mController.CreateShader(shaderCreateInfo, nullptr);
+  Graphics::UniquePtr<Graphics::Shader> shader = mController.CreateShader(shaderCreateInfo, nullptr);
 
-  auto retval = shader.get();
   mItems.emplace_back(std::move(shader), shaderCode, stage, type);
-  return *retval;
+  return *mItems.back().shader.get();
 }
 
 } // namespace Render

diff --git a/dali/internal/render/renderers/shader-cache.h b/dali/internal/render/renderers/shader-cache.h
index b041c28..5f00aba 100644 (file)
--- a/dali/internal/render/renderers/shader-cache.h
+++ b/dali/internal/render/renderers/shader-cache.h
@@ -40,7 +40,7 @@ struct ShaderCache
     Item(Item&&)      = default;
 
     Item(Graphics::UniquePtr<Dali::Graphics::Shader> shader,
-         const std::vector<char>                     shaderCode,
+         const std::vector<char>&                    shaderCode,
          Graphics::PipelineStage                     stage,
          Graphics::ShaderSourceMode                  type)
     : shader(std::move(shader)),
@@ -53,7 +53,7 @@ struct ShaderCache
     ~Item() = default;
 
     Graphics::UniquePtr<Dali::Graphics::Shader> shader{nullptr};
-    const std::vector<char>                     shaderCode;
+    std::vector<char>                           shaderCode;
     Graphics::PipelineStage                     stage;
     Graphics::ShaderSourceMode                  type;
   };
@@ -73,7 +73,7 @@ struct ShaderCache
    * @param[in] type The type of the shader (i.e. text or binary)
    * @return the graphics shader
    */
-  Dali::Graphics::Shader& GetShader(const std::vector<char> shaderCode, Graphics::PipelineStage stage, Graphics::ShaderSourceMode type);
+  Dali::Graphics::Shader& GetShader(const std::vector<char>& shaderCode, Graphics::PipelineStage stage, Graphics::ShaderSourceMode type);
 
 private:
   std::vector<Item>           mItems;