From fea2310683b2cb6b8a6b0fa147cdf8b4eaab2bb7 Mon Sep 17 00:00:00 2001 From: Tanish Jadhav Date: Sun, 15 Jun 2025 19:44:05 +0530 Subject: [PATCH 1/4] Implement load_into for SharedPtrDataLoader and add test (#11562) --- .../data_loader/shared_ptr_data_loader.h | 25 ++++++++++++++ .../test/shared_ptr_data_loader_test.cpp | 33 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/extension/data_loader/shared_ptr_data_loader.h b/extension/data_loader/shared_ptr_data_loader.h index 551ab4d498c..d8af9bfc187 100644 --- a/extension/data_loader/shared_ptr_data_loader.h +++ b/extension/data_loader/shared_ptr_data_loader.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace executorch { namespace extension { @@ -43,11 +44,35 @@ class SharedPtrDataLoader final : public executorch::runtime::DataLoader { return executorch::runtime::FreeableBuffer( static_cast(data_.get()) + offset, size, /*free_fn=*/nullptr); } + + ET_NODISCARD executorch::runtime::Error load_into( + size_t offset, + size_t size, + const DataLoader::SegmentInfo& segment_info, + void* buffer) const override; ET_NODISCARD executorch::runtime::Result size() const override { return size_; } + ET_NODISCARD executorch::runtime::Error SharedPtrDataLoader::load_into( + size_t offset, + size_t size, + const DataLoader::SegmentInfo& segment_info, + void* buffer) const { + ET_CHECK_OR_RETURN_ERROR( + offset + size <= size_, + executorch::runtime::Error::OutOfBounds, + "offset %zu + size %zu exceeds buffer size %zu", + offset, + size, + size_); + + std::memcpy(buffer, static_cast(data_.get()) + offset, size); + return executorch::runtime::Error::Ok; +} + + private: const std::shared_ptr data_; const size_t size_; diff --git a/extension/data_loader/test/shared_ptr_data_loader_test.cpp b/extension/data_loader/test/shared_ptr_data_loader_test.cpp index 62d71ae0560..7b933d4dd5f 100644 --- a/extension/data_loader/test/shared_ptr_data_loader_test.cpp +++ b/extension/data_loader/test/shared_ptr_data_loader_test.cpp @@ -140,3 +140,36 @@ TEST_F(SharedPtrDataLoaderTest, OutOfBoundsLoadFails) { EXPECT_NE(fb.error(), Error::Ok); } } + +// Unit test to check that SharedPtrDataLoader::load_into copies the correct data. +TEST(SharedPtrDataLoaderTest, LoadIntoCopiesCorrectData) { + std::vector source_data = {10, 20, 30, 40, 50}; + + // Wrap the source data in a shared_ptr without taking ownership. + auto data_ptr = std::shared_ptr(source_data.data(), [](void*) {}); + SharedPtrDataLoader loader(data_ptr, source_data.size()); + + uint8_t buffer[3] = {0}; + + // Load 3 bytes starting from offset 1 (expecting values 20, 30, 40). + auto err = loader.load_into(1, 3, DataLoader::SegmentInfo{}, buffer); + + EXPECT_EQ(err, Error::Ok); + EXPECT_EQ(buffer[0], 20); + EXPECT_EQ(buffer[1], 30); + EXPECT_EQ(buffer[2], 40); +} + +// Unit test to verify that SharedPtrDataLoader::load_into handles out-of-bounds requests. +TEST(SharedPtrDataLoaderTest, LoadIntoRejectsOutOfBoundsAccess) { + std::vector source_data = {10, 20, 30, 40, 50}; + auto data_ptr = std::shared_ptr(source_data.data(), [](void*) {}); + SharedPtrDataLoader loader(data_ptr, source_data.size()); + + uint8_t buffer[3] = {0}; + + // This should fail because offset + size = 4 + 3 = 7 > 5 (size of data). + auto err = loader.load_into(4, 3, DataLoader::SegmentInfo{}, buffer); + + EXPECT_EQ(err, Error::OutOfBounds); +} \ No newline at end of file From cb040351d4282e3296639f1f1729b4f1cc1a6597 Mon Sep 17 00:00:00 2001 From: Tanish Jadhav <91683678+Tanish2101@users.noreply.github.com> Date: Tue, 17 Jun 2025 19:12:34 +0530 Subject: [PATCH 2/4] Updated the shared_ptr_data_loader.h I have merge the declaration and implementation of 'load_into' function. --- extension/data_loader/shared_ptr_data_loader.h | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/extension/data_loader/shared_ptr_data_loader.h b/extension/data_loader/shared_ptr_data_loader.h index d8af9bfc187..8675a501db7 100644 --- a/extension/data_loader/shared_ptr_data_loader.h +++ b/extension/data_loader/shared_ptr_data_loader.h @@ -49,17 +49,7 @@ class SharedPtrDataLoader final : public executorch::runtime::DataLoader { size_t offset, size_t size, const DataLoader::SegmentInfo& segment_info, - void* buffer) const override; - - ET_NODISCARD executorch::runtime::Result size() const override { - return size_; - } - - ET_NODISCARD executorch::runtime::Error SharedPtrDataLoader::load_into( - size_t offset, - size_t size, - const DataLoader::SegmentInfo& segment_info, - void* buffer) const { + void* buffer) const override { ET_CHECK_OR_RETURN_ERROR( offset + size <= size_, executorch::runtime::Error::OutOfBounds, @@ -70,8 +60,11 @@ class SharedPtrDataLoader final : public executorch::runtime::DataLoader { std::memcpy(buffer, static_cast(data_.get()) + offset, size); return executorch::runtime::Error::Ok; -} + } + ET_NODISCARD executorch::runtime::Result size() const override { + return size_; + } private: const std::shared_ptr data_; From 29ce3dc7cb98b92754b8e546445758ce4b208e9b Mon Sep 17 00:00:00 2001 From: Tanish Jadhav <91683678+Tanish2101@users.noreply.github.com> Date: Tue, 17 Jun 2025 20:05:05 +0530 Subject: [PATCH 3/4] Update shared_ptr_data_loader.h --- .../data_loader/shared_ptr_data_loader.h | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/extension/data_loader/shared_ptr_data_loader.h b/extension/data_loader/shared_ptr_data_loader.h index 8675a501db7..1ad721dffdc 100644 --- a/extension/data_loader/shared_ptr_data_loader.h +++ b/extension/data_loader/shared_ptr_data_loader.h @@ -8,12 +8,12 @@ #pragma once +#include #include #include #include #include #include -#include namespace executorch { namespace extension { @@ -45,18 +45,19 @@ class SharedPtrDataLoader final : public executorch::runtime::DataLoader { static_cast(data_.get()) + offset, size, /*free_fn=*/nullptr); } - ET_NODISCARD executorch::runtime::Error load_into( - size_t offset, - size_t size, - const DataLoader::SegmentInfo& segment_info, - void* buffer) const override { + ET_NODISCARD + executorch::runtime::Error load_into( + size_t offset, + size_t size, + const DataLoader::SegmentInfo& segment_info, + void* buffer) const override { ET_CHECK_OR_RETURN_ERROR( - offset + size <= size_, - executorch::runtime::Error::OutOfBounds, - "offset %zu + size %zu exceeds buffer size %zu", - offset, - size, - size_); + offset + size <= size_, + executorch::runtime::Error::OutOfBounds, + "offset %zu + size %zu exceeds buffer size %zu", + offset, + size, + size_); std::memcpy(buffer, static_cast(data_.get()) + offset, size); return executorch::runtime::Error::Ok; From 2c3eee143078e768104dd22003c98c1f43aabcd7 Mon Sep 17 00:00:00 2001 From: Tanish Jadhav <91683678+Tanish2101@users.noreply.github.com> Date: Sat, 28 Jun 2025 08:09:56 +0000 Subject: [PATCH 4/4] Fix: Use valid Error enum in SharedPtrDataLoader and update test - Replaced non-existent with in implementation. - Updated the corresponding unit test to match the corrected error. - Ensures correctness and prevents enum resolution error during compilation. --- extension/data_loader/shared_ptr_data_loader.h | 2 +- extension/data_loader/test/shared_ptr_data_loader_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/data_loader/shared_ptr_data_loader.h b/extension/data_loader/shared_ptr_data_loader.h index 1ad721dffdc..d791c132803 100644 --- a/extension/data_loader/shared_ptr_data_loader.h +++ b/extension/data_loader/shared_ptr_data_loader.h @@ -53,7 +53,7 @@ class SharedPtrDataLoader final : public executorch::runtime::DataLoader { void* buffer) const override { ET_CHECK_OR_RETURN_ERROR( offset + size <= size_, - executorch::runtime::Error::OutOfBounds, + InvalidArgument, "offset %zu + size %zu exceeds buffer size %zu", offset, size, diff --git a/extension/data_loader/test/shared_ptr_data_loader_test.cpp b/extension/data_loader/test/shared_ptr_data_loader_test.cpp index 7b933d4dd5f..af735fadf04 100644 --- a/extension/data_loader/test/shared_ptr_data_loader_test.cpp +++ b/extension/data_loader/test/shared_ptr_data_loader_test.cpp @@ -171,5 +171,5 @@ TEST(SharedPtrDataLoaderTest, LoadIntoRejectsOutOfBoundsAccess) { // This should fail because offset + size = 4 + 3 = 7 > 5 (size of data). auto err = loader.load_into(4, 3, DataLoader::SegmentInfo{}, buffer); - EXPECT_EQ(err, Error::OutOfBounds); + EXPECT_EQ(err, Error::InvalidArgument); } \ No newline at end of file