From 3520b9be81a77bd7568197379575ee76ca9aa75d Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Mon, 4 Mar 2024 16:50:23 +0900 Subject: [PATCH] Fix many svace issues at dali-toolkit (integer overflow + etc) This is a combination of 3 commits. Let we remove many cases of dali-toolkit when we can be call 0u - 1u, which is undefined behavior. Signed-off-by: Eunki, Hong (Scene3D) Fix svace issue : convert uint32_t to std::streamoff + minor Fix svace issue when we try to convert from 32bit unsigned integer to (might) 64bit signed integer. + Fix the name of function collision issue Signed-off-by: Eunki, Hong Fix svace issue : Avoid to divide by zero cases Change-Id: If11884693253cd86f89cb98704e93473166faca1 Signed-off-by: Eunki, Hong --- dali-scene3d/internal/algorithm/navigation-mesh-impl.cpp | 8 ++++---- dali-scene3d/internal/algorithm/navigation-mesh-impl.h | 4 ++-- dali-scene3d/internal/loader/dli-loader-impl.cpp | 4 ++-- dali-scene3d/internal/loader/gltf2-util.cpp | 8 ++++---- dali-scene3d/public-api/algorithm/navigation-mesh.cpp | 6 +++--- dali-scene3d/public-api/algorithm/navigation-mesh.h | 8 ++++---- dali-scene3d/public-api/loader/ktx-loader.cpp | 2 +- dali-scene3d/public-api/loader/mesh-definition.cpp | 4 ++-- dali-toolkit/internal/builder/replacement.cpp | 4 +++- .../internal/controls/bubble-effect/bubble-emitter-impl.cpp | 9 ++++++--- .../internal/controls/bubble-effect/bubble-renderer.cpp | 4 ++-- dali-toolkit/internal/controls/scene3d-view/gltf-loader.cpp | 4 +++- .../internal/text/bounded-paragraph-helper-functions.cpp | 6 +++++- .../internal/text/controller/text-controller-impl.cpp | 2 +- dali-toolkit/internal/text/layouts/layout-engine.cpp | 8 +++++++- .../internal/text/rendering/atlas/text-atlas-renderer.cpp | 3 ++- dali-toolkit/internal/text/rendering/text-typesetter.cpp | 2 +- dali-toolkit/internal/visuals/gradient/gradient.cpp | 11 +++++++---- 18 files changed, 59 insertions(+), 38 deletions(-) diff --git a/dali-scene3d/internal/algorithm/navigation-mesh-impl.cpp b/dali-scene3d/internal/algorithm/navigation-mesh-impl.cpp index 81b999d..c5f4ed1 100644 --- a/dali-scene3d/internal/algorithm/navigation-mesh-impl.cpp +++ b/dali-scene3d/internal/algorithm/navigation-mesh-impl.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. @@ -185,7 +185,7 @@ bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& out return FindFloor(position, outPosition, faceIndex); } -bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& faceIndex) +bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& outFaceIndex) { [[maybe_unused]] auto newPos = PointSceneToLocal(Dali::Vector3(position)); @@ -217,8 +217,8 @@ bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& out std::sort(results.begin(), results.end(), [](const IntersectResult& lhs, const IntersectResult& rhs) { return lhs.distance < rhs.distance; }); outPosition = PointLocalToScene(results.front().point); - faceIndex = results.front().faceIndex; - mCurrentFace = results.front().faceIndex; + outFaceIndex = results.front().faceIndex; + mCurrentFace = outFaceIndex; return true; } diff --git a/dali-scene3d/internal/algorithm/navigation-mesh-impl.h b/dali-scene3d/internal/algorithm/navigation-mesh-impl.h index 6a82787..e2ee0fc 100644 --- a/dali-scene3d/internal/algorithm/navigation-mesh-impl.h +++ b/dali-scene3d/internal/algorithm/navigation-mesh-impl.h @@ -2,7 +2,7 @@ #define DALI_SCENE3D_INTERNAL_NAVIGATION_MESH_H /* - * 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. @@ -115,7 +115,7 @@ public: /** * @copydoc Dali::Scene3D::Algorithm::NavigationMesh::FindFloor() */ - bool FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& faceIndex); + bool FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& outFaceIndex); /** * @copydoc Dali::Scene3D::Algorithm::NavigationMesh::GetFace() diff --git a/dali-scene3d/internal/loader/dli-loader-impl.cpp b/dali-scene3d/internal/loader/dli-loader-impl.cpp index 1ed195d..6cdb1c4 100644 --- a/dali-scene3d/internal/loader/dli-loader-impl.cpp +++ b/dali-scene3d/internal/loader/dli-loader-impl.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. @@ -1585,7 +1585,7 @@ void DliLoaderImpl::Impl::ParseAnimations(const TreeNode* tnAnimations, LoadPara ReadInt(tnKeyFramesBin->GetChild("byteOffset"), byteOffset); DALI_ASSERT_ALWAYS(byteOffset >= 0); - binAniFile.seekg(byteOffset, std::ios::beg); + binAniFile.seekg(static_cast(byteOffset), std::ios::beg); int numKeys = 0; ReadInt(tnKeyFramesBin->GetChild("numKeys"), numKeys); diff --git a/dali-scene3d/internal/loader/gltf2-util.cpp b/dali-scene3d/internal/loader/gltf2-util.cpp index 7979786..6e572b7 100644 --- a/dali-scene3d/internal/loader/gltf2-util.cpp +++ b/dali-scene3d/internal/loader/gltf2-util.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. @@ -602,7 +602,7 @@ TextureDefinition ConvertTextureInfo(const gltf2::TextureInfo& textureInfo, Conv { auto& stream = context.mOutput.mResources.mBuffers[bufferIndex].GetBufferStream(); stream.clear(); - stream.seekg(textureInfo.mTexture->mSource->mBufferView->mByteOffset, stream.beg); + stream.seekg(static_cast(static_cast(textureInfo.mTexture->mSource->mBufferView->mByteOffset)), stream.beg); std::vector dataBuffer; dataBuffer.resize(textureInfo.mTexture->mSource->mBufferView->mByteLength); stream.read(reinterpret_cast(dataBuffer.data()), static_cast(static_cast(textureInfo.mTexture->mSource->mBufferView->mByteLength))); @@ -1304,7 +1304,7 @@ void LoadDataFromAccessor(ConversionContext& context, uint32_t bufferIndex, Vect } auto& stream = buffer.GetBufferStream(); stream.clear(); - stream.seekg(offset, stream.beg); + stream.seekg(static_cast(static_cast(offset)), stream.beg); stream.read(reinterpret_cast(dataBuffer.Begin()), static_cast(static_cast(size))); } @@ -1514,7 +1514,7 @@ void ProcessSkins(const gltf2::Document& document, ConversionContext& context) DALI_LOG_ERROR("Failed to load from stream\n"); } mStream.clear(); - mStream.seekg(accessor.mBufferView->mByteOffset + accessor.mByteOffset, mStream.beg); + mStream.seekg(static_cast(static_cast(accessor.mBufferView->mByteOffset + accessor.mByteOffset)), mStream.beg); } virtual void Provide(Matrix& inverseBindMatrix) override diff --git a/dali-scene3d/public-api/algorithm/navigation-mesh.cpp b/dali-scene3d/public-api/algorithm/navigation-mesh.cpp index 4aff076..1d7f90c 100644 --- a/dali-scene3d/public-api/algorithm/navigation-mesh.cpp +++ b/dali-scene3d/public-api/algorithm/navigation-mesh.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. @@ -46,9 +46,9 @@ NavigationMesh::~NavigationMesh() = default; return mImpl->GetVertexCount(); } -bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& faceIndex) +bool NavigationMesh::FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& outFaceIndex) { - return mImpl->FindFloor(position, outPosition, faceIndex); + return mImpl->FindFloor(position, outPosition, outFaceIndex); } bool NavigationMesh::FindFloorForFace(const Dali::Vector3& position, FaceIndex faceIndex, bool dontCheckNeighbours, Dali::Vector3& outPosition) diff --git a/dali-scene3d/public-api/algorithm/navigation-mesh.h b/dali-scene3d/public-api/algorithm/navigation-mesh.h index f856155..34088dd 100644 --- a/dali-scene3d/public-api/algorithm/navigation-mesh.h +++ b/dali-scene3d/public-api/algorithm/navigation-mesh.h @@ -2,7 +2,7 @@ #define DALI_SCENE3D_NAVIGATION_MESH_H /* - * 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. @@ -152,12 +152,12 @@ public: /** * @brief Looks for the floor under specified position * @param[in] position Position to investigate - * @param[in] outPosition Position on the floor in found - * @param[out] faceIndex Index of NavigationMesh face associated with floor + * @param[out] outPosition Position on the floor in found + * @param[out] outFaceIndex Index of NavigationMesh face associated with floor * * @return True if floor has been found, False otherwise */ - bool FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& faceIndex); + bool FindFloor(const Dali::Vector3& position, Dali::Vector3& outPosition, FaceIndex& outFaceIndex); /** * @brief Looks for a floor starting from specified face diff --git a/dali-scene3d/public-api/loader/ktx-loader.cpp b/dali-scene3d/public-api/loader/ktx-loader.cpp index 010dc89..4d33ef5 100644 --- a/dali-scene3d/public-api/loader/ktx-loader.cpp +++ b/dali-scene3d/public-api/loader/ktx-loader.cpp @@ -206,7 +206,7 @@ bool LoadKtxData(const std::string& path, EnvironmentMapData& environmentMapData } // Skip the key-values: - if(fp.seekg(header.bytesOfKeyValueData, fp.cur).good() == false) + if(fp.seekg(static_cast(static_cast(header.bytesOfKeyValueData)), fp.cur).good() == false) { return false; } diff --git a/dali-scene3d/public-api/loader/mesh-definition.cpp b/dali-scene3d/public-api/loader/mesh-definition.cpp index 4fc60e4..6728790 100644 --- a/dali-scene3d/public-api/loader/mesh-definition.cpp +++ b/dali-scene3d/public-api/loader/mesh-definition.cpp @@ -101,7 +101,7 @@ const char* QUAD("quad"); bool ReadBlob(const MeshDefinition::Blob& descriptor, std::istream& source, uint8_t* target) { source.clear(); - if(!source.seekg(descriptor.mOffset, std::istream::beg)) + if(!source.seekg(static_cast(static_cast(descriptor.mOffset)), std::istream::beg)) { return false; } @@ -122,7 +122,7 @@ bool ReadBlob(const MeshDefinition::Blob& descriptor, std::istream& source, uint { readSize += descriptor.mStride; target += descriptor.mElementSizeHint; - source.seekg(diff, std::istream::cur); + source.seekg(static_cast(static_cast(diff)), std::istream::cur); } return readSize == totalSize; } diff --git a/dali-toolkit/internal/builder/replacement.cpp b/dali-toolkit/internal/builder/replacement.cpp index 5825da5..4d59ee7 100644 --- a/dali-toolkit/internal/builder/replacement.cpp +++ b/dali-toolkit/internal/builder/replacement.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 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. @@ -112,6 +112,8 @@ bool ResolvePartialReplacement(const std::string& initialValue, Property::Value& } else { + DALI_ASSERT_ALWAYS((startPos > 0u && size > 0u) && "Replacement keyword getter have some problem!\n"); + const std::string str(initialValue.substr(startPos, size)); Property::Value* value = FindReplacement(str, overrideMap, defaultMap); diff --git a/dali-toolkit/internal/controls/bubble-effect/bubble-emitter-impl.cpp b/dali-toolkit/internal/controls/bubble-effect/bubble-emitter-impl.cpp index 781a1e0..12b7a13 100644 --- a/dali-toolkit/internal/controls/bubble-effect/bubble-emitter-impl.cpp +++ b/dali-toolkit/internal/controls/bubble-effect/bubble-emitter-impl.cpp @@ -351,11 +351,14 @@ void BubbleEmitter::SetBubbleParameter(BubbleRenderer& bubbleRenderer, unsigned { Vector2 dir(direction); - int halfRange = displacement.x / 2; + int rangeX = std::max(static_cast(displacement.x), 1); // To avoid divide by zero issue. + int rangeY = std::max(static_cast(displacement.y), 1); // To avoid divide by zero issue. + + int halfRangeX = displacement.x / 2; // for the y coordinate, always negative, so bubbles always go upwards - Vector2 randomVec(rand_r(&mRandomSeed) % static_cast(displacement.x) - halfRange, -rand_r(&mRandomSeed) % static_cast(displacement.y)); + Vector2 randomVec(rand_r(&mRandomSeed) % rangeX - halfRangeX, -rand_r(&mRandomSeed) % rangeY); dir.Normalize(); - randomVec.x -= dir.x * halfRange; + randomVec.x -= dir.x * halfRangeX; randomVec.y *= 1.0f - fabsf(dir.x) * 0.33f; if(randomVec.y > 0.0f) diff --git a/dali-toolkit/internal/controls/bubble-effect/bubble-renderer.cpp b/dali-toolkit/internal/controls/bubble-effect/bubble-renderer.cpp index 73c636d..33c5eef 100644 --- a/dali-toolkit/internal/controls/bubble-effect/bubble-renderer.cpp +++ b/dali-toolkit/internal/controls/bubble-effect/bubble-renderer.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 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. @@ -59,7 +59,7 @@ void BubbleRenderer::Initialize(unsigned int numberOfBubble, const Vector2& move mIndexInvertedMovementArea = mRenderer.RegisterUniqueProperty("uInvertedMovementArea", Vector2(1.f, 1.f) / movementArea); mIndicesOffset.resize(9); - int offset = movementArea.Length() / 10.f; + int offset = std::max(static_cast(movementArea.Length() / 10.f), 1); // To avoid divide by zero issue. uint32_t seed = static_cast(time(NULL)); diff --git a/dali-toolkit/internal/controls/scene3d-view/gltf-loader.cpp b/dali-toolkit/internal/controls/scene3d-view/gltf-loader.cpp index 66f7677..b1cd70d 100644 --- a/dali-toolkit/internal/controls/scene3d-view/gltf-loader.cpp +++ b/dali-toolkit/internal/controls/scene3d-view/gltf-loader.cpp @@ -38,6 +38,8 @@ namespace Gltf { namespace { +constexpr float MIN_DURATION_SECONDS = 1e-2f; + // Utility functions const TreeNode* Tidx(const TreeNode* node, uint32_t index) { @@ -434,7 +436,7 @@ float LoadKeyFrames(const AnimationSamplerInfo& currentSampler, const Property:: LoadDataFromAccessor(currentSampler.output, outputBufferData, path, accessorArray, bufferViewArray, bufferArray); uint32_t keyframeNum = inputBufferData.Size(); - float lengthAnimation = inputBufferData[inputBufferData.Size() - 1]; + float lengthAnimation = std::max((inputBufferData.Size() > 0u ? inputBufferData[inputBufferData.Size() - 1] : MIN_DURATION_SECONDS), MIN_DURATION_SECONDS); for(uint32_t i = 0; i < keyframeNum; i++) { if(propIndex == Dali::Actor::Property::ORIENTATION) diff --git a/dali-toolkit/internal/text/bounded-paragraph-helper-functions.cpp b/dali-toolkit/internal/text/bounded-paragraph-helper-functions.cpp index 7b207f4..219f7bf 100644 --- a/dali-toolkit/internal/text/bounded-paragraph-helper-functions.cpp +++ b/dali-toolkit/internal/text/bounded-paragraph-helper-functions.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 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. @@ -167,6 +167,10 @@ void MergeBoundedParagraphRunsWhenRemoveCharacters(const Vector& boundedParagraphRuns.Remove(paragraphToDelete); numberOfRuns--; + if(lastRunIndexToUpdate == 0u) + { + break; + } lastRunIndexToUpdate--; continue; diff --git a/dali-toolkit/internal/text/controller/text-controller-impl.cpp b/dali-toolkit/internal/text/controller/text-controller-impl.cpp index 4336221..f70d8af 100644 --- a/dali-toolkit/internal/text/controller/text-controller-impl.cpp +++ b/dali-toolkit/internal/text/controller/text-controller-impl.cpp @@ -701,7 +701,7 @@ void Controller::Impl::CalculateTextUpdateIndices(Length& numberOfCharacters) mTextUpdateInfo.mRequestedNumberOfCharacters = mTextUpdateInfo.mNumberOfCharactersToAdd - mTextUpdateInfo.mNumberOfCharactersToRemove; mTextUpdateInfo.mStartGlyphIndex = mModel->mVisualModel->mGlyphs.Count(); - mTextUpdateInfo.mStartLineIndex = mModel->mVisualModel->mLines.Count() - 1u; + mTextUpdateInfo.mStartLineIndex = (mModel->mVisualModel->mLines.Count() > 0u) ? mModel->mVisualModel->mLines.Count() - 1u : 0u; // Nothing else to do; return; diff --git a/dali-toolkit/internal/text/layouts/layout-engine.cpp b/dali-toolkit/internal/text/layouts/layout-engine.cpp index 7bbf2cc..57f4d6c 100644 --- a/dali-toolkit/internal/text/layouts/layout-engine.cpp +++ b/dali-toolkit/internal/text/layouts/layout-engine.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 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. @@ -1478,6 +1478,12 @@ struct Engine::Impl { const Vector& glyphs = layoutParameters.textModel->mVisualModel->mGlyphs; + if(glyphs.Size() == 0u) + { + // Do nothing. + return; + } + // Need to add a new line with no characters but with height to increase the layoutSize.height const GlyphInfo& glyphInfo = glyphs[glyphs.Count() - 1u]; diff --git a/dali-toolkit/internal/text/rendering/atlas/text-atlas-renderer.cpp b/dali-toolkit/internal/text/rendering/atlas/text-atlas-renderer.cpp index 5788d33..5ccaad0 100644 --- a/dali-toolkit/internal/text/rendering/atlas/text-atlas-renderer.cpp +++ b/dali-toolkit/internal/text/rendering/atlas/text-atlas-renderer.cpp @@ -559,7 +559,8 @@ struct AtlasRenderer::Impl { GlyphInfo glyph; bool addHyphen = ((hyphenIndex < hyphensCount) && hyphenIndices && ((i + startIndexOfGlyphs) == hyphenIndices[hyphenIndex])); - if(addHyphen && hyphens) + // TODO : Shouldn't we have to control here when i == 0 cases? + if(addHyphen && hyphens && i > 0u) { glyph = hyphens[hyphenIndex]; i--; diff --git a/dali-toolkit/internal/text/rendering/text-typesetter.cpp b/dali-toolkit/internal/text/rendering/text-typesetter.cpp index fe164f6..1c31199 100644 --- a/dali-toolkit/internal/text/rendering/text-typesetter.cpp +++ b/dali-toolkit/internal/text/rendering/text-typesetter.cpp @@ -163,7 +163,7 @@ void TypesetGlyph(GlyphData& __restrict__ data, // Whether the given glyph is a color one. const bool isColorGlyph = data.glyphBitmap.isColorEmoji || data.glyphBitmap.isColorBitmap; const uint32_t glyphPixelSize = Pixel::GetBytesPerPixel(data.glyphBitmap.format); - const uint32_t glyphAlphaIndex = glyphPixelSize - 1u; + const uint32_t glyphAlphaIndex = (glyphPixelSize > 0u) ? glyphPixelSize - 1u : 0u; // Determinate iterator range. const int32_t lineIndexRangeMin = std::max(0, -yOffset); diff --git a/dali-toolkit/internal/visuals/gradient/gradient.cpp b/dali-toolkit/internal/visuals/gradient/gradient.cpp index 34da8a6..e96cf32 100644 --- a/dali-toolkit/internal/visuals/gradient/gradient.cpp +++ b/dali-toolkit/internal/visuals/gradient/gradient.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 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. @@ -91,7 +91,8 @@ Dali::Texture Gradient::GenerateLookupTexture() { std::sort(mGradientStops.Begin(), mGradientStops.End()); - unsigned int numStops = mGradientStops.Count(); + uint32_t numStops = mGradientStops.Count(); + DALI_ASSERT_ALWAYS(numStops > 0u && "The number of gradient stop should not be zero!"); /** * If the stops have not covered the whole zero to one range, @@ -179,8 +180,10 @@ Dali::Texture Gradient::GenerateLookupTexture() unsigned int Gradient::EstimateTextureResolution() { - float minInterval = 1.0; - for(unsigned int i = 0, numStops = mGradientStops.Count(); i < numStops - 1u; i++) + float minInterval = 1.0; + const uint32_t numStops = mGradientStops.Count(); + DALI_ASSERT_ALWAYS(numStops > 0u && "The number of gradient stop should not be zero!"); + for(uint32_t i = 0; i < numStops - 1u; i++) { float interval = mGradientStops[i + 1].mOffset - mGradientStops[i].mOffset; minInterval = interval > minInterval ? minInterval : interval; -- 2.7.4