From a9bea1a2848250b88fe062f39f1e0a0322a17f14 Mon Sep 17 00:00:00 2001 From: Colm Donelan Date: Mon, 24 Apr 2023 21:51:44 +0100 Subject: IVGCVSW-7404 Replacing asserts with exceptions in CopyTensorContentsGeneric * The source and destination size checks in CopyTensorContentsGeneric are handled by asserts instead of exceptions. * Adding unit tests. Signed-off-by: Colm Donelan Change-Id: Ia00c07158afde6768002dc6059067fd08e47fcff --- src/backends/backendsCommon/WorkloadUtils.hpp | 25 +++- src/backends/backendsCommon/test/CMakeLists.txt | 1 + .../backendsCommon/test/WorkloadUtilsTest.cpp | 140 +++++++++++++++++++++ 3 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 src/backends/backendsCommon/test/WorkloadUtilsTest.cpp diff --git a/src/backends/backendsCommon/WorkloadUtils.hpp b/src/backends/backendsCommon/WorkloadUtils.hpp index 3d8d927345..6350c2542c 100644 --- a/src/backends/backendsCommon/WorkloadUtils.hpp +++ b/src/backends/backendsCommon/WorkloadUtils.hpp @@ -1,5 +1,5 @@ // -// Copyright © 2017 Arm Ltd. All rights reserved. +// Copyright © 2017, 2023 Arm Ltd. All rights reserved. // SPDX-License-Identifier: MIT // @@ -52,11 +52,9 @@ void CopyTensorContentsGeneric(const ITensorHandle* srcTensor, ITensorHandle* ds TensorShape srcStrides = srcTensor->GetStrides(); const TensorShape& srcShape = srcTensor->GetShape(); const auto srcSize = srcTensor->GetStrides()[0] * srcShape[0]; - IgnoreUnused(srcSize); // Only used for asserts TensorShape dstStrides = dstTensor->GetStrides(); const TensorShape& dstShape = dstTensor->GetShape(); const auto dstSize = dstTensor->GetStrides()[0] * dstShape[0]; - IgnoreUnused(dstSize); // Only used for asserts size_t srcDepth = 1; size_t srcBatches = 1; @@ -121,6 +119,14 @@ void CopyTensorContentsGeneric(const ITensorHandle* srcTensor, ITensorHandle* ds srcDataStart = static_cast(srcTensor->Map()); dstDataStart = static_cast(dstTensor->Map()); } + if (srcDataStart == nullptr) + { + throw MemoryValidationException("The source tensor is null."); + } + if (dstDataStart == nullptr) + { + throw MemoryValidationException("The destination tensor is null."); + } size_t copyLength = std::min(srcChannels * srcChannelStride, dstChannels * dstChannelStride); size_t copyWidth = std::min(srcWidth, dstWidth); @@ -165,8 +171,17 @@ void CopyTensorContentsGeneric(const ITensorHandle* srcTensor, ITensorHandle* ds auto dstPtrChannel = dstData; for (unsigned int w = 0; w < copyWidth; ++w) { - ARMNN_ASSERT(srcData >= srcDataStart && srcData + copyLength <= srcDataStart + srcSize); - ARMNN_ASSERT(dstData >= dstDataStart && dstData + copyLength <= dstDataStart + dstSize); + // Sanity check the memory area we've been asked to copy from and to. + if (copyLength > srcSize) + { + throw MemoryValidationException( + "The source tensor size does not match the size of the allocated tensor."); + } + if (copyLength > dstSize) + { + throw MemoryValidationException( + "The destination tensor size will overrun the destination tensor."); + } copy(dstData, srcData, copyLength); dstData += dstWidthStride; srcData += srcWidthStride; diff --git a/src/backends/backendsCommon/test/CMakeLists.txt b/src/backends/backendsCommon/test/CMakeLists.txt index 26b0b433bd..95065dffe4 100644 --- a/src/backends/backendsCommon/test/CMakeLists.txt +++ b/src/backends/backendsCommon/test/CMakeLists.txt @@ -63,6 +63,7 @@ list(APPEND armnnBackendsCommonUnitTests_sources TransposeEndToEndTestImpl.hpp TensorCopyUtils.hpp WorkloadFactoryHelper.hpp + WorkloadUtilsTest.cpp layerTests/AbsTestImpl.cpp layerTests/AbsTestImpl.hpp layerTests/ActivationTestImpl.cpp diff --git a/src/backends/backendsCommon/test/WorkloadUtilsTest.cpp b/src/backends/backendsCommon/test/WorkloadUtilsTest.cpp new file mode 100644 index 0000000000..e429902ce4 --- /dev/null +++ b/src/backends/backendsCommon/test/WorkloadUtilsTest.cpp @@ -0,0 +1,140 @@ +// +// Copyright © 2023 Arm Ltd and Contributors. All rights reserved. +// SPDX-License-Identifier: MIT +// + +#include +#include +#include +#include + +TEST_SUITE("WorkloadUtilsTest") +{ +using namespace armnn; + +TEST_CASE("CopyTensorContents") +{ + // Two tensors of 1 float. Set source to a value and destination to 0. + // Copy source to destination and make sure destination is copied. + std::shared_ptr memoryManager = std::make_shared(); + TensorInfo info({ 1, 1, 1, 1 }, DataType::Float32); + MockTensorHandle srcTensorHandle(info, memoryManager); + MockTensorHandle destTensorHandle(info, memoryManager); + srcTensorHandle.Allocate(); + float* buffer = reinterpret_cast(srcTensorHandle.Map()); + buffer[0] = 2.5f; + + destTensorHandle.Allocate(); + buffer = reinterpret_cast(destTensorHandle.Map()); + buffer[0] = 0.0f; + + auto copyFunc = [](void* dst, const void* src, size_t size) + { + memcpy(dst, src, size); + }; + CopyTensorContentsGeneric(&srcTensorHandle, &destTensorHandle, copyFunc); + // After copy the destination should be 2.5. + buffer = reinterpret_cast(destTensorHandle.Map()); + CHECK(buffer[0] == 2.5f); + // Also make sure the first buffer hasn't changed. + buffer = reinterpret_cast(srcTensorHandle.Map()); + CHECK(buffer[0] == 2.5f); +} + +TEST_CASE("CopyTensorContents_UnallocatedTensors") +{ + // Standard copy lambda + auto copyFunc = [](void* dst, const void* src, size_t size) + { + memcpy(dst, src, size); + }; + + // Two tensors of 1 float. The source will be managed but unallocated. This should throw an exception. + std::shared_ptr memoryManager = std::make_shared(); + TensorInfo info({ 1, 1, 1, 1 }, DataType::Float32); + MockTensorHandle unallocatedSource(info, memoryManager); + unallocatedSource.Manage(); + MockTensorHandle destTensorHandle(info, memoryManager); + destTensorHandle.Allocate(); + CHECK_THROWS_AS(CopyTensorContentsGeneric(&unallocatedSource, &destTensorHandle, copyFunc), + const armnn::MemoryValidationException&); + + // Same test for destination tensor. + MockTensorHandle unallocatedDest(info, memoryManager); + unallocatedDest.Manage(); + MockTensorHandle srcTensorHandle(info, memoryManager); + srcTensorHandle.Allocate(); + CHECK_THROWS_AS(CopyTensorContentsGeneric(&srcTensorHandle, &unallocatedDest, copyFunc), + const armnn::MemoryValidationException&); + +} + +TEST_CASE("CopyTensorContents_DifferentTensorSizes") +{ + std::shared_ptr memoryManager = std::make_shared(); + // Standard copy lambda + auto copyFunc = [](void* dst, const void* src, size_t size) + { + memcpy(dst, src, size); + }; + + // This is an odd test case. We'll make the destination tensor 1 element smaller than the source. + // In this case the tensor will be truncated. + TensorInfo largerInfo({ 1, 1, 1, 6 }, DataType::Float32); + TensorInfo smallerInfo({ 1, 1, 1, 5 }, DataType::Float32); + MockTensorHandle srcLargerTensorHandle(largerInfo, memoryManager); + srcLargerTensorHandle.Allocate(); + float* buffer = reinterpret_cast(srcLargerTensorHandle.Map()); + // We'll set a value in the 5th elements, this should be copied over. + buffer[4] = 5.1f; + MockTensorHandle destSmallerTensorHandle(smallerInfo, memoryManager); + destSmallerTensorHandle.Allocate(); + buffer = reinterpret_cast(destSmallerTensorHandle.Map()); + buffer[4] = 5.2f; // This should be overwritten. + CopyTensorContentsGeneric(&srcLargerTensorHandle, &destSmallerTensorHandle, copyFunc); + CHECK(buffer[4] == 5.1f); + + // Same test case but with destination being larger than source. + MockTensorHandle srcSmallerTensorHandle(smallerInfo, memoryManager); + srcSmallerTensorHandle.Allocate(); + buffer = reinterpret_cast(srcSmallerTensorHandle.Map()); + // We'll set a value in the 5th elements, this should be copied over. + buffer[4] = 5.1f; + MockTensorHandle destLargerTensorHandle(largerInfo, memoryManager); + destLargerTensorHandle.Allocate(); + buffer = reinterpret_cast(destLargerTensorHandle.Map()); + buffer[4] = 5.2f; // This should be overwritten. + buffer[5] = 6.2f; // This should NOT be overwritten. + CopyTensorContentsGeneric(&srcSmallerTensorHandle, &destLargerTensorHandle, copyFunc); + CHECK(buffer[4] == 5.1f); // Has been copied. + CHECK(buffer[5] == 6.2f); // Should be untouched. +} + +TEST_CASE("CopyTensorContents_MixedDataTypes") +{ + // This is almost a pointless test, but it may detect problems with future changes. + // We'll copy a float tensor into a uint8 tensor of the same size. It should + // work without error. + std::shared_ptr memoryManager = std::make_shared(); + // Standard copy lambda + auto copyFunc = [](void* dst, const void* src, size_t size) + { + memcpy(dst, src, size); + }; + + TensorInfo floatInfo({ 1, 1, 1, 2 }, DataType::Float32); + TensorInfo intInfo({ 1, 1, 1, 8 }, DataType::QAsymmU8); + MockTensorHandle floatTensorHandle(floatInfo, memoryManager); + floatTensorHandle.Allocate(); + float* floatBuffer = reinterpret_cast(floatTensorHandle.Map()); + floatBuffer[0] = 1.1f; // This should be 0x3f8ccccd or something very close. + MockTensorHandle uintTensorHandle(intInfo, memoryManager); + uintTensorHandle.Allocate(); + uint8_t* intBuffer = reinterpret_cast(uintTensorHandle.Map()); + intBuffer[0] = 0; + + CopyTensorContentsGeneric(&floatTensorHandle, &uintTensorHandle, copyFunc); + CHECK(intBuffer[0] == 0xcd); // Make sure the data has been copied over. +} + +} -- cgit v1.2.1