(Vulkan) Fix minor coverity issues 78/319878/1
authorEunki, Hong <eunkiki.hong@samsung.com>
Wed, 19 Feb 2025 05:37:37 +0000 (14:37 +0900)
committerEunki, Hong <eunkiki.hong@samsung.com>
Wed, 19 Feb 2025 05:37:37 +0000 (14:37 +0900)
1. Non-initalized structs (spriv.h, command-buffer.h)
2. Make mMemory nullptr if allocation failed. (image-impl, buffer-impl)
3. Avoid nullptr copy, or negative length string for some case (texture, shader-impl)

Change-Id: Ib2b99af36d430e4159ef7eb7619b040a08f3c688
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
dali/internal/graphics/gles-impl/gles-graphics-shader.cpp
dali/internal/graphics/vulkan-impl/vulkan-buffer-impl.cpp
dali/internal/graphics/vulkan-impl/vulkan-command-buffer-impl.h
dali/internal/graphics/vulkan-impl/vulkan-image-impl.cpp
dali/internal/graphics/vulkan-impl/vulkan-shader-impl.cpp
dali/internal/graphics/vulkan-impl/vulkan-spirv.h
dali/internal/graphics/vulkan-impl/vulkan-texture.cpp

index 7dbd23a48e06871367e8191601622c3214375fe0..069d9f8af7d59e7b02395cf9bea7cdf8188b6d31 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -108,24 +108,32 @@ struct ShaderImpl::Impl
         const auto src    = !sourcePreprocessed.empty() ? reinterpret_cast<const char*>(sourcePreprocessed.data()) : reinterpret_cast<const char*>(createInfo.sourceData);
 
         // null-terminated char already included. So we should remove last character (null terminator) from size.
-        GLint size = (!sourcePreprocessed.empty() ? GLint(sourcePreprocessed.size()) : createInfo.sourceSize) - 1u;
+        GLint size = static_cast<GLint>(!sourcePreprocessed.empty() ? static_cast<uint32_t>(sourcePreprocessed.size()) : createInfo.sourceSize) - 1;
 
-        gl->ShaderSource(shader, 1, const_cast<const char**>(&src), &size);
-        gl->CompileShader(shader);
-
-        GLint status{0};
-        gl->GetShaderiv(shader, GL_COMPILE_STATUS, &status);
-        if(status != GL_TRUE)
+        if(src != nullptr && size >= 0)
+        {
+          gl->ShaderSource(shader, 1, const_cast<const char**>(&src), &size);
+          gl->CompileShader(shader);
+
+          GLint status{0};
+          gl->GetShaderiv(shader, GL_COMPILE_STATUS, &status);
+          if(status != GL_TRUE)
+          {
+            char    output[4096];
+            GLsizei outputSize{0u};
+            gl->GetShaderInfoLog(shader, 4096, &outputSize, output);
+            DALI_LOG_ERROR("Code: %.*s\n", size, reinterpret_cast<const char*>(src));
+            DALI_LOG_ERROR("glCompileShader() failed: \n%s\n", output);
+            gl->DeleteShader(shader);
+            return false;
+          }
+          glShader = shader;
+        }
+        else
         {
-          char    output[4096];
-          GLsizei outputSize{0u};
-          gl->GetShaderInfoLog(shader, 4096, &outputSize, output);
-          DALI_LOG_ERROR("Code: %.*s\n", size, reinterpret_cast<const char*>(src));
-          DALI_LOG_ERROR("glCompileShader() failed: \n%s\n", output);
-          gl->DeleteShader(shader);
+          DALI_LOG_ERROR("glCompileShader() failed: source is nullptr, or size is negative! src : %p, size : %d\n", src, size);
           return false;
         }
-        glShader = shader;
       }
       return true;
     }
index 1ffcbf6723e62e47b5d00640734caab70ee904dd..bf8c5c466e0787eeb2d2250341a7663ec763ed63 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -79,7 +79,7 @@ void BufferImpl::Initialize(vk::MemoryPropertyFlags memoryProperties)
   {
     DALI_LOG_INFO(gVulkanFilter, Debug::General, "Unable to allocate memory for the buffer of size %d!", int(requirements.size));
 
-    mMemory = nullptr;
+    mMemory.reset();
   }
 
   // Bind
@@ -96,9 +96,8 @@ void BufferImpl::Destroy()
   auto device = mDevice.GetLogicalDevice();
   device.destroyBuffer(mBuffer, mDevice.GetAllocator());
 
-  mMemory.reset();
   mBuffer = nullptr;
-  mMemory = nullptr;
+  mMemory.reset();
 }
 
 Graphics::MemoryRequirements BufferImpl::GetMemoryRequirements()
index 2951d2f046abb4f2077ca918f8703a3834934dd5..73861d4ccdf25f75406e3f064fc673115b36a1d0 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_GRAPHICS_VULKAN_COMMAND_BUFFER_IMPL_H
 
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -188,7 +188,7 @@ private:
   std::vector<DeferredUniformBinding> mDeferredUniformBindings;
 
   // Deferred pipeline to bind if dynamic states not supported
-  Vulkan::Pipeline* mDeferredPipelineToBind;
+  Vulkan::Pipeline* mDeferredPipelineToBind{nullptr};
 
   // Dynamic depth/stencil states for deferred pipeline binding if API < 1.3
   // TODO: check API version
index 352d82a75577db27a8b018ca60648fc4f72329f6..7b5fe35d09a12936de06905242c5786d4d763df5 100644 (file)
@@ -121,8 +121,11 @@ void Image::AllocateAndBind(vk::MemoryPropertyFlags memoryProperties)
   if(result != vk::Result::eSuccess)
   {
     DALI_LOG_INFO(gVulkanFilter, Debug::General, "Unable to allocate memory for the image of size %d!", int(requirements.size));
+
+    mMemory.reset();
   }
-  else if(mMemory) // bind the allocated memory to the image
+
+  if(mMemory) // bind the allocated memory to the image
   {
     VkAssert(mDevice.GetLogicalDevice().bindImageMemory(mImage, mMemory->GetVkHandle(), 0));
   }
index b62dae2febca8ea41778040d9f92764554f43e5b..8067ba8cc5e9b9fe90856637e1e877e12229493b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -76,19 +76,31 @@ struct ShaderImpl::Impl
     bool success = true;
     if(createInfo.sourceMode == ShaderSourceMode::TEXT)
     {
-      SPIRVGeneratorInfo info;
-      info.pipelineStage = createInfo.pipelineStage;
-      auto shaderCode    = std::string_view(reinterpret_cast<char*>(sourcePreprocessed.data()));
-      info.shaderCode    = shaderCode;
+      const auto src = !sourcePreprocessed.empty() ? reinterpret_cast<const char*>(sourcePreprocessed.data()) : reinterpret_cast<const char*>(createInfo.sourceData);
 
-      spirv = std::make_unique<SPIRVGenerator>(info);
+      // null-terminated char already included. So we should remove last character (null terminator) from size.
+      int32_t size = static_cast<int32_t>(!sourcePreprocessed.empty() ? static_cast<uint32_t>(sourcePreprocessed.size()) : createInfo.sourceSize) - 1;
 
-      spirv->Generate();
-      if(spirv->IsValid())
+      if(src != nullptr && size >= 0)
       {
-        // substitute data and size with compiled code
-        createInfo.sourceSize = spirv->Get().size() * 4u;
-        createInfo.sourceData = spirv->Get().data();
+        SPIRVGeneratorInfo info;
+        info.pipelineStage = createInfo.pipelineStage;
+        auto shaderCode    = std::string_view(src, size);
+        info.shaderCode    = shaderCode;
+
+        spirv = std::make_unique<SPIRVGenerator>(info);
+
+        spirv->Generate();
+        if(spirv->IsValid())
+        {
+          // substitute data and size with compiled code
+          createInfo.sourceSize = spirv->Get().size() * 4u;
+          createInfo.sourceData = spirv->Get().data();
+        }
+        else
+        {
+          success = false;
+        }
       }
       else
       {
index 3af8094d2f7d9ed6ea1f8fd716fc15c6d2f688d3..23919a73be3f9f6a4b016d3662d80e3aad543426 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_GRAPHICS_VULKAN_SPIRV_H
 
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -39,10 +39,10 @@ class ShaderImpl;
  */
 struct SPIRVGeneratorInfo
 {
-  std::string   shaderCode;
-  PipelineStage pipelineStage;
+  std::string   shaderCode{};
+  PipelineStage pipelineStage{PipelineStage::TOP_OF_PIPELINE}; ///< Invalid stage
 
-  struct SPIRVGeneratorExtraInfo* extraInfo; ///< Reserved
+  struct SPIRVGeneratorExtraInfo* extraInfo{nullptr}; ///< Reserved
 };
 
 /**
index 0306dc72da9145050b3827ff020745aa8ec94fbd..0fbfc5663cb553fb5b69ca4fdc6901da45405121 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2025 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.
@@ -928,8 +928,7 @@ bool Texture::TryConvertPixelData(const void* pData, uint32_t sizeInBytes, uint3
     return false;
   }
 
-  auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item)
-                         { return item.oldFormat == mConvertFromFormat; });
+  auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item) { return item.oldFormat == mConvertFromFormat; });
 
   // No suitable format, return empty array
   if(it == COLOR_CONVERSION_TABLE.end())
@@ -951,8 +950,7 @@ bool Texture::TryConvertPixelData(const void* pData, uint32_t sizeInBytes, uint3
     return false;
   }
 
-  auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item)
-                         { return item.oldFormat == mConvertFromFormat; });
+  auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item) { return item.oldFormat == mConvertFromFormat; });
 
   // No suitable format, return empty array
   if(it == COLOR_CONVERSION_TABLE.end())
@@ -1017,6 +1015,22 @@ void Texture::CopyMemoryDirect(
     return;
   }
 
+  uint8_t* srcPtr = nullptr;
+  if(sourceInfo.sourceType == Dali::Graphics::TextureUpdateSourceInfo::Type::MEMORY)
+  {
+    srcPtr = reinterpret_cast<uint8_t*>(sourceInfo.memorySource.memory);
+  }
+  else if(sourceInfo.sourceType == Dali::Graphics::TextureUpdateSourceInfo::Type::PIXEL_DATA)
+  {
+    auto pixelBufferData = Dali::Integration::GetPixelDataBuffer(sourceInfo.pixelDataSource.pixelData);
+    srcPtr               = pixelBufferData.buffer + info.srcOffset;
+  }
+  else
+  {
+    // TODO: other sources NOT support yet.
+    return;
+  }
+
   // try to initialise resource
   InitializeImageView();
 
@@ -1042,17 +1056,6 @@ void Texture::CopyMemoryDirect(
   auto dstRowLength = subresourceLayout.rowPitch;
   auto dstPtr       = ptr + int(dstRowLength) * info.dstOffset2D.y + sizeInBytes * info.dstOffset2D.x;
 
-  uint8_t* srcPtr = nullptr;
-  if(sourceInfo.sourceType == Dali::Graphics::TextureUpdateSourceInfo::Type::MEMORY)
-  {
-    srcPtr = reinterpret_cast<uint8_t*>(sourceInfo.memorySource.memory);
-  }
-  else if(sourceInfo.sourceType == Dali::Graphics::TextureUpdateSourceInfo::Type::PIXEL_DATA)
-  {
-    auto pixelBufferData = Dali::Integration::GetPixelDataBuffer(sourceInfo.pixelDataSource.pixelData);
-    srcPtr               = pixelBufferData.buffer + info.srcOffset;
-  }
-
   auto srcRowLength = int(info.srcExtent2D.width) * sizeInBytes;
 
   if(formatInfo.compressed)
@@ -1101,8 +1104,7 @@ vk::Format Texture::ValidateFormat(vk::Format sourceFormat)
   // if format isn't supported, see whether suitable conversion is implemented
   if(!formatFlags)
   {
-    auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item)
-                           { return item.oldFormat == sourceFormat; });
+    auto it = std::find_if(COLOR_CONVERSION_TABLE.begin(), COLOR_CONVERSION_TABLE.end(), [&](auto& item) { return item.oldFormat == sourceFormat; });
 
     // No suitable format, return empty array
     if(it != COLOR_CONVERSION_TABLE.end())