From 58afd83180f955fde3133abcf9670b655b22187f Mon Sep 17 00:00:00 2001 From: Adam Bialogonski Date: Thu, 18 Jan 2024 14:25:34 +0000 Subject: [PATCH] [Tizen] Prevents writing array uniforms with out-of-bound indices and accidental overwrite of next uniform. Change-Id: I5e991cae1765d951edc0690028ef9f1ac2d165a6 --- .../test-graphics-reflection.cpp | 33 ++--- automated-tests/src/dali/utc-Dali-Renderer.cpp | 142 ++++++++++++++++++++- dali/internal/render/shaders/program.cpp | 21 ++- 3 files changed, 176 insertions(+), 20 deletions(-) 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 --- 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 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 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 --- 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{ + {"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 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::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{{"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 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 --- 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 -- 2.7.4