Prevents writing array uniforms with out-of-bound indices and accidental overwrite...

Change-Id: I5e991cae1765d951edc0690028ef9f1ac2d165a6

diff --git a/automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp b/automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp
index 3998fde..22d09e1 100644 (file)
--- a/automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp
+++ b/automated-tests/src/dali/dali-test-suite-utils/test-graphics-reflection.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@ namespace
 {
 static const std::vector<UniformData> UNIFORMS =
   {
+    UniformData("uColor", Property::Type::VECTOR4),
     UniformData("uRendererColor", Property::Type::FLOAT),
     UniformData("uCustom", Property::Type::INTEGER),
     UniformData("uCustom3", Property::Type::VECTOR3),
@@ -47,7 +48,6 @@ static const std::vector<UniformData> UNIFORMS =
     UniformData("sTexture", Property::Type::FLOAT),
     UniformData("sTextureRect", Property::Type::FLOAT),
     UniformData("sGloss", Property::Type::FLOAT),
-    UniformData("uColor", Property::Type::VECTOR4),
     UniformData("uActorColor", Property::Type::VECTOR4),
     UniformData("uModelMatrix", Property::Type::MATRIX),
     UniformData("uModelView", Property::Type::MATRIX),
@@ -140,19 +140,6 @@ TestGraphicsReflection::TestGraphicsReflection(TestGraphicsController& controlle
   mDefaultUniformBlock.members.clear();
 
   int offset = 0;
-  for(const auto& data : UNIFORMS)
-  {
-    mDefaultUniformBlock.members.emplace_back();
-    auto& item   = mDefaultUniformBlock.members.back();
-    item.name    = data.name;
-    item.binding = 0;
-    item.offsets.push_back(offset);
-    item.locations.push_back(gl.GetUniformLocation(programId, data.name.c_str()));
-    item.bufferIndex  = 0;
-    item.uniformClass = Graphics::UniformClass::UNIFORM;
-    item.type         = data.type;
-    offset += GetSizeForType(data.type);
-  }
 
   for(const auto& data : mCustomUniforms)
   {
@@ -233,6 +220,21 @@ TestGraphicsReflection::TestGraphicsReflection(TestGraphicsController& controlle
       offset += GetSizeForType(data.type);
     }
   }
+
+  for(const auto& data : UNIFORMS)
+  {
+    mDefaultUniformBlock.members.emplace_back();
+    auto& item   = mDefaultUniformBlock.members.back();
+    item.name    = data.name;
+    item.binding = 0;
+    item.offsets.push_back(offset);
+    item.locations.push_back(gl.GetUniformLocation(programId, data.name.c_str()));
+    item.bufferIndex  = 0;
+    item.uniformClass = Graphics::UniformClass::UNIFORM;
+    item.type         = data.type;
+    offset += GetSizeForType(data.type);
+  }
+
   mDefaultUniformBlock.size = offset;
 
   mUniformBlocks.push_back(mDefaultUniformBlock);
@@ -329,6 +331,7 @@ bool TestGraphicsReflection::GetUniformBlock(uint32_t index, Dali::Graphics::Uni
     out.members[i].uniformClass = Graphics::UniformClass::UNIFORM;
     out.members[i].offset       = memberUniform.offsets[0];
     out.members[i].location     = memberUniform.locations[0];
+    out.members[i].elementCount = memberUniform.numElements;
   }
 
   return true;

diff --git a/automated-tests/src/dali/utc-Dali-Renderer.cpp b/automated-tests/src/dali/utc-Dali-Renderer.cpp
index 9f6115c..0b09348 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-Renderer.cpp
+++ b/automated-tests/src/dali/utc-Dali-Renderer.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -5071,3 +5071,143 @@ int UtcDaliRendererUniformBlocksUnregisterScene01(void)
 
   END_TEST;
 }
+
+int UtcDaliRendererUniformNameCrop(void)
+{
+  TestApplication application;
+  tet_infoline("Tests against reflection cropping one character too many form array uniform name.\n");
+
+  auto& graphics = application.GetGraphicsController();
+
+  auto uniforms = std::vector<UniformData>{
+    {"uSomeColor", Dali::Property::Type::FLOAT},
+    {"uSomeColors[10]", Dali::Property::Type::FLOAT}};
+  graphics.AddCustomUniforms(uniforms);
+
+  auto& gl = application.GetGlAbstraction();
+  graphics.mCallStack.EnableLogging(true);
+  graphics.mCommandBufferCallStack.EnableLogging(true);
+  gl.mBufferTrace.EnableLogging(true);
+  gl.mBufferTrace.Enable(true);
+
+  gl.mSetUniformTrace.EnableLogging(true);
+  gl.mSetUniformTrace.Enable(true);
+
+  Geometry geometry = CreateQuadGeometry();
+  Shader   shader   = Shader::New("vertexSrc", "fragmentSrc");
+  Renderer renderer = Renderer::New(geometry, shader);
+  Actor    actor    = Actor::New();
+  actor.AddRenderer(renderer);
+  actor[Actor::Property::SIZE] = Vector2(120, 120);
+  application.GetScene().Add(actor);
+
+  std::ostringstream oss;
+  struct UniformIndexPair
+  {
+    Property::Index index;
+    std::string     name;
+    UniformIndexPair(Property::Index index, std::string name)
+    : index(index),
+      name(std::move(name))
+    {
+    }
+  };
+  std::vector<UniformIndexPair> uniformIndices;
+  for(int i = 0; i < 10; ++i)
+  {
+    Property::Index index;
+    oss << "uArray[" << i + 1 << "]";
+    auto value = float(i);
+    index      = renderer.RegisterProperty(oss.str(), value);
+    uniformIndices.emplace_back(index, oss.str());
+    oss.str("");
+    oss.clear();
+  }
+
+  // Cause overwrite, index 10 and uToOverflow should share same memory
+  [[maybe_unused]] auto badArrayIndex  = renderer.RegisterProperty("uSomeColor", 100.0f);
+  [[maybe_unused]] auto badArrayIndex2 = renderer.RegisterProperty("uSomeColors[0]", 200.0f);
+
+  application.GetScene().Add(actor);
+  application.SendNotification();
+  application.Render(0);
+
+  float value = 0.0f;
+  gl.GetUniformValue("uSomeColor", value);
+
+  // Test against the bug when name is one character short and array may be mistaken for
+  // an individual uniform of the same name minut 1 character.
+  DALI_TEST_EQUALS(value, 100.0f, std::numeric_limits<float>::epsilon(), TEST_LOCATION);
+  END_TEST;
+}
+
+int UtcDaliRendererUniformArrayOverflow(void)
+{
+  TestApplication application;
+  tet_infoline("Overflow test whether uColor uniform would be overriden by array with out-of-bound index.\n");
+
+  auto& graphics = application.GetGraphicsController();
+  auto  uniforms = std::vector<UniformData>{{"uArray[10]", Dali::Property::Type::FLOAT}};
+
+  graphics.AddCustomUniforms(uniforms);
+
+  auto& gl = application.GetGlAbstraction();
+  graphics.mCallStack.EnableLogging(true);
+  graphics.mCommandBufferCallStack.EnableLogging(true);
+  gl.mBufferTrace.EnableLogging(true);
+  gl.mBufferTrace.Enable(true);
+
+  gl.mSetUniformTrace.EnableLogging(true);
+  gl.mSetUniformTrace.Enable(true);
+
+  Geometry geometry = CreateQuadGeometry();
+  Shader   shader   = Shader::New("vertexSrc", "fragmentSrc");
+  Renderer renderer = Renderer::New(geometry, shader);
+  Actor    actor    = Actor::New();
+  actor.AddRenderer(renderer);
+  actor[Actor::Property::SIZE] = Vector2(120, 120);
+  application.GetScene().Add(actor);
+
+  std::ostringstream oss;
+  struct UniformIndexPair
+  {
+    Property::Index index;
+    std::string     name;
+    UniformIndexPair(Property::Index index, std::string name)
+    : index(index),
+      name(std::move(name))
+    {
+    }
+  };
+  std::vector<UniformIndexPair> uniformIndices;
+  for(int i = 0; i < 10; ++i)
+  {
+    Property::Index index;
+    oss << "uArray[" << i << "]";
+    auto value = float(i);
+    index      = renderer.RegisterProperty(oss.str(), value);
+    uniformIndices.emplace_back(index, oss.str());
+    oss.str("");
+    oss.clear();
+  }
+
+  // Cause overwrite, index 10 and uToOverflow should share same memory
+  [[maybe_unused]] auto badArrayIndex = renderer.RegisterProperty("uArray[10]", 0.0f);
+
+  application.GetScene().Add(actor);
+  application.SendNotification();
+  application.Render(0);
+
+  Vector4 uniformColor = Vector4::ZERO;
+  gl.GetUniformValue("uColor", uniformColor);
+  tet_printf("uColor value %f, %f, %f, %f\n",
+             uniformColor.r,
+             uniformColor.g,
+             uniformColor.b,
+             uniformColor.a);
+
+  // the r component of uColor uniform must not be changed.
+  // if r is 0.0f then test fails as the array stomped on the uniform's memory.
+  DALI_TEST_EQUALS((uniformColor.r != 0.0f), true, TEST_LOCATION);
+  END_TEST;
+}
\ No newline at end of file

diff --git a/dali/internal/render/shaders/program.cpp b/dali/internal/render/shaders/program.cpp
index b5aa1f8..7ee3aa9 100644 (file)
--- a/dali/internal/render/shaders/program.cpp
+++ b/dali/internal/render/shaders/program.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -225,11 +225,14 @@ bool Program::GetUniform(const std::string_view& name, Hash hashedName, Hash has
   Hash             hash  = hashedName;
   std::string_view match = name;
 
+  int arrayIndex = 0;
+
   if(!name.empty() && name.back() == ']')
   {
-    hash     = hashedNameNoArray;
-    auto pos = name.rfind("[");
-    match    = name.substr(0, pos - 1); // Remove subscript
+    hash       = hashedNameNoArray;
+    auto pos   = name.rfind("[");
+    match      = name.substr(0, pos); // Remove subscript
+    arrayIndex = atoi(&name[pos + 1]);
   }
 
   for(const ReflectionUniformInfo& item : mReflection)
@@ -239,6 +242,16 @@ bool Program::GetUniform(const std::string_view& name, Hash hashedName, Hash has
       if(!item.hasCollision || item.uniformInfo.name == match)
       {
         out = item.uniformInfo;
+
+        // Array out of bounds
+        if(item.uniformInfo.elementCount > 0 && arrayIndex >= int(item.uniformInfo.elementCount))
+        {
+          DALI_LOG_ERROR("Uniform %s, array index out of bound [%d >= %d]!\n",
+                         item.uniformInfo.name.c_str(),
+                         int(arrayIndex),
+                         int(item.uniformInfo.elementCount));
+          return false;
+        }
         return true;
       }
       else