diff --git a/score/containers/test/allocator_test_type_helpers.h b/score/containers/test/allocator_test_type_helpers.h index 375c5f38..0a32a9bd 100644 --- a/score/containers/test/allocator_test_type_helpers.h +++ b/score/containers/test/allocator_test_type_helpers.h @@ -44,7 +44,7 @@ template < bool>::type = true> Allocator GetAllocator(memory::shared::ManagedMemoryResource& memory_resource) { - return memory::shared::PolymorphicOffsetPtrAllocator{memory_resource.getMemoryResourceProxy()}; + return memory::shared::PolymorphicOffsetPtrAllocator{memory_resource}; } namespace test_types diff --git a/score/memory/design/shared_memory/memory_allocation.puml b/score/memory/design/shared_memory/memory_allocation.puml index 63ba5368..0e474aa3 100644 --- a/score/memory/design/shared_memory/memory_allocation.puml +++ b/score/memory/design/shared_memory/memory_allocation.puml @@ -121,11 +121,11 @@ class "score::memory::shared::PolymorphicOffsetPtrAllocator" as Polymor { - proxy: offset_ptr -- - + PolymorphicOffsetPtrAllocator(MemoryResourceProxy* proxy) + + PolymorphicOffsetPtrAllocator(ManagedMemoryResource&) + PolymorphicOffsetPtrAllocator() + allocate( std::size_t n ): offset_ptr + deallocate(offset_ptr p, std::size_t n ): void - + getMemoryResourceProxy(): offset_ptr + - getMemoryResourceProxy(): offset_ptr -- Notes: This data type shall apply requirements diff --git a/score/memory/design/shared_memory/memory_allocation_workflow.puml b/score/memory/design/shared_memory/memory_allocation_workflow.puml index dde0bc62..2f757bdb 100644 --- a/score/memory/design/shared_memory/memory_allocation_workflow.puml +++ b/score/memory/design/shared_memory/memory_allocation_workflow.puml @@ -54,12 +54,8 @@ deactivate Resource Factory --> Instance: instance -Instance -> Resource: getMemoryResourceProxy() -activate Resource -Resource --> Instance: MemoryResourceProxy* -deactivate Resource - -Instance -> Allocator: instantiate(MemoryResourceProxy*) +Instance -> Allocator: instantiate(Resource) +activate Allocator activate Allocator Instance -> Allocator: allocate(Bytes) Allocator -> Proxy: allocate(Bytes) diff --git a/score/memory/shared/BUILD b/score/memory/shared/BUILD index 9987b33a..9441640f 100644 --- a/score/memory/shared/BUILD +++ b/score/memory/shared/BUILD @@ -698,6 +698,7 @@ cc_gtest_unit_test( ], deps = [ ":memory_resource_proxy", + ":shared_memory_test_resources", "@score_baselibs//score/memory/shared/fake:fake_memory_resources", "@score_baselibs//score/mw/log:backend_stub_testutil", ], diff --git a/score/memory/shared/managed_memory_resource.h b/score/memory/shared/managed_memory_resource.h index 2bbeba3e..0170c032 100644 --- a/score/memory/shared/managed_memory_resource.h +++ b/score/memory/shared/managed_memory_resource.h @@ -29,6 +29,9 @@ namespace test class ManagedMemoryResourceTestAttorney; } // namespace test +template +class PolymorphicOffsetPtrAllocator; + // Suppress "AUTOSAR C++14 M3-2-3" rule finding: "A type, object or function that is used in multiple translation units // shall be declared in one and only one file.". // The forward declaration of MemoryResourceProxy is necessary to avoid cyclic dependencies and to ensure that the @@ -61,16 +64,6 @@ class ManagedMemoryResource : public ::score::cpp::pmr::memory_resource ManagedMemoryResource() noexcept = default; ~ManagedMemoryResource() noexcept override = default; - /** - * We need to return a raw pointer, since we need to convert this - * pointer into an OffsetPtr if it shall be stored in shared memory. - * @return MemoryResourceProxy* that identifies _this_ memory_resource. - */ - - /// \todo: getMemoryResourceProxy should not return a non const pointer and the method should also be marked const. - /// This issue will be investigated and fixed in Ticket-146625" - virtual const MemoryResourceProxy* getMemoryResourceProxy() noexcept = 0; - /** * @brief Construct T allocating underlying MemoryResource * @tparam T The type that shall be constructed @@ -144,6 +137,12 @@ class ManagedMemoryResource : public ::score::cpp::pmr::memory_resource // This is for testing only // coverity[autosar_cpp14_a11_3_1_violation] friend class test::ManagedMemoryResourceTestAttorney; + + // PolymorphicOffsetPtrAllocator template is a friend to access getMemoryResourceProxy() in its constructor + // that accepts a ManagedMemoryResource reference. + // coverity[autosar_cpp14_a11_3_1_violation] + template friend class PolymorphicOffsetPtrAllocator; + // We make MemoryResourceRegistry a friend since it needs to access private internals of ManagedMemoryResource which // we do not want to expose to the user via the public interface of ManagedMemoryResource. // coverity[autosar_cpp14_a11_3_1_violation] @@ -156,6 +155,16 @@ class ManagedMemoryResource : public ::score::cpp::pmr::memory_resource * @return void* past-the-end address of memory resource */ virtual const void* getEndAddress() const noexcept = 0; + + /** + * We need to return a raw pointer, since we need to convert this + * pointer into an OffsetPtr if it shall be stored in shared memory. + * @return MemoryResourceProxy* that identifies _this_ memory_resource. + */ + + /// \todo: getMemoryResourceProxy should not return a non const pointer and the method should also be marked const. + /// This issue will be investigated and fixed in Ticket-146625" + virtual const MemoryResourceProxy* getMemoryResourceProxy() noexcept = 0; }; } // namespace score::memory::shared diff --git a/score/memory/shared/managed_memory_resource_test.cpp b/score/memory/shared/managed_memory_resource_test.cpp index 86e52a42..09d49d21 100644 --- a/score/memory/shared/managed_memory_resource_test.cpp +++ b/score/memory/shared/managed_memory_resource_test.cpp @@ -13,6 +13,7 @@ #include "managed_memory_resource.h" #include "fake/my_memory_resource.h" +#include "shared_memory_test_resources.h" #include "gtest/gtest.h" @@ -24,7 +25,8 @@ namespace score::memory::shared::test TEST(ManagedMemoryResource, offersToGetMemoryResourceManager) { std::unique_ptr unit = std::make_unique(); - EXPECT_NE(unit->getMemoryResourceProxy(), nullptr); + ManagedMemoryResourceTestAttorney attorney(*unit); + EXPECT_NE(attorney.getMemoryResourceProxy(), nullptr); } TEST(ManagedMemoryResource, CanDestructImplByParentClass) diff --git a/score/memory/shared/map_test.cpp b/score/memory/shared/map_test.cpp index dfecac05..ef98c585 100644 --- a/score/memory/shared/map_test.cpp +++ b/score/memory/shared/map_test.cpp @@ -26,7 +26,7 @@ TEST(Map, AllocatesMemoryOnProvidedResource) { // Given a Map of int to int, ensure that the Map uses the allocator to get its memory test::MyMemoryResource memory{}; - Map unit{memory.getMemoryResourceProxy()}; + Map unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When inserting an element @@ -41,7 +41,7 @@ TEST(Map, InnerVectorAllocatesMemoryOnProvidedResource) { // Given a Map of a Vector test::MyMemoryResource memory{}; - Map> unit{memory.getMemoryResourceProxy()}; + Map> unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When constructing a Vector within the Map @@ -55,7 +55,7 @@ TEST(Map, InnerVectorAllocatesMemoryOnProvidedResourceByDefaultConstruction) { // Given a Map of a Vector test::MyMemoryResource memory{}; - Map> unit{memory.getMemoryResourceProxy()}; + Map> unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When default constructing the Vector @@ -69,7 +69,7 @@ TEST(Map, UserConstructedVectorCanBeUsed) { // Given a Map of a Vector test::MyMemoryResource memory{}; - Map> unit{memory.getMemoryResourceProxy()}; + Map> unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When the user constructs a Vector by using the same allocator @@ -83,7 +83,7 @@ TEST(Map, MapInMapAllocatesMemoryFromCorrectRessource) { // Given a Map of a Map test::MyMemoryResource memory{}; - Map> unit{memory.getMemoryResourceProxy()}; + Map> unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When implicit constructing the inner map diff --git a/score/memory/shared/memory_resource_proxy_test.cpp b/score/memory/shared/memory_resource_proxy_test.cpp index 42d5dda8..6bb248c2 100644 --- a/score/memory/shared/memory_resource_proxy_test.cpp +++ b/score/memory/shared/memory_resource_proxy_test.cpp @@ -16,6 +16,7 @@ #include "fake/my_memory_resource.h" #include "memory_resource_registry.h" #include "score/memory/shared/pointer_arithmetic_util.h" +#include "score/memory/shared/shared_memory_test_resources.h" #include "gtest/gtest.h" #include @@ -147,7 +148,8 @@ TEST_F(BoundCheckedMemoryResourceProxyTest, AllocationIsPossibleWhenProxyInBound { // Given a registered memory resource and its respective MemoryResourceProxy // When allocating memory on the MemoryResourceProxy - MemoryResourceProxy* proxy = memoryResource.getMemoryResourceProxy(); + ManagedMemoryResourceTestAttorney attorney(memoryResource); + const MemoryResourceProxy* proxy = attorney.getMemoryResourceProxy(); // Align the memory since that will be done before allocating the new memory const auto initial_number_allocated_bytes = memoryResource.getAllocatedMemory(); diff --git a/score/memory/shared/polymorphic_offset_ptr_allocator.h b/score/memory/shared/polymorphic_offset_ptr_allocator.h index 664630a3..3612e022 100644 --- a/score/memory/shared/polymorphic_offset_ptr_allocator.h +++ b/score/memory/shared/polymorphic_offset_ptr_allocator.h @@ -45,7 +45,10 @@ class PolymorphicOffsetPtrAllocator // Non-explicit constructor is good enough for maintaining required implicit conversion // NOLINTNEXTLINE(google-explicit-constructor): Tolerated, discard explicit. - PolymorphicOffsetPtrAllocator(const MemoryResourceProxy* const proxy) noexcept : proxy_{proxy} {} + PolymorphicOffsetPtrAllocator(ManagedMemoryResource& resource) noexcept + : proxy_{resource.getMemoryResourceProxy()} + { + } template // Non-explicit constructor is good enough for maintaining required implicit conversion. diff --git a/score/memory/shared/polymorphic_offset_ptr_allocator_test.cpp b/score/memory/shared/polymorphic_offset_ptr_allocator_test.cpp index 06f9c243..efdc6345 100644 --- a/score/memory/shared/polymorphic_offset_ptr_allocator_test.cpp +++ b/score/memory/shared/polymorphic_offset_ptr_allocator_test.cpp @@ -32,7 +32,7 @@ TEST(PolymorphicOffsetPtrAllocator, AllocatesAndDeallocatesMemory) { MyMemoryResource resource{}; - std::vector> unit(resource.getMemoryResourceProxy()); + std::vector> unit(resource); unit.emplace_back(42U); unit.emplace_back(43U); } @@ -46,49 +46,11 @@ TEST(PolymorphicOffsetPtrAllocator, SupportsRebinding) std::hash, std::equal_to, PolymorphicOffsetPtrAllocator>> - unit(resource.getMemoryResourceProxy()); + unit(resource); unit.insert({42U, 0U}); } -TEST(PolymorphicOffsetPtrAllocator, AllocatorsPointingToMemoryResourceProxiesWithSameIdsComparisonOperators) -{ - MemoryResourceProxy proxy1{0U}; - MemoryResourceProxy proxy2{0U}; - - PolymorphicOffsetPtrAllocator allocator1{&proxy1}; - PolymorphicOffsetPtrAllocator allocator2{&proxy2}; - - EXPECT_TRUE(allocator1 == allocator1); - EXPECT_TRUE(allocator1 == allocator2); - EXPECT_FALSE(allocator1 != allocator2); -} - -TEST(PolymorphicOffsetPtrAllocator, AllocatorsPointingToMemoryResourceProxiesWithDifferentIdsComparisonOperators) -{ - MemoryResourceProxy proxy1{0U}; - MemoryResourceProxy proxy2{1U}; - - PolymorphicOffsetPtrAllocator allocator1{&proxy1}; - PolymorphicOffsetPtrAllocator allocator2{&proxy2}; - - EXPECT_FALSE(allocator1 == allocator2); - EXPECT_TRUE(allocator1 != allocator2); -} - -TEST(PolymorphicOffsetPtrAllocator, AllocatorsWithOneNullPtrMemoryResourceProxiesComparisonOperators) -{ - MemoryResourceProxy proxy1{0U}; - - PolymorphicOffsetPtrAllocator allocator1{&proxy1}; - PolymorphicOffsetPtrAllocator allocator2; - - EXPECT_FALSE(allocator1 == allocator2); - EXPECT_FALSE(allocator2 == allocator1); - EXPECT_TRUE(allocator1 != allocator2); - EXPECT_TRUE(allocator2 != allocator1); -} - TEST(PolymorphicOffsetPtrAllocator, AllocatorsWithNullPtrMemoryResourceProxiesComparisonOperators) { PolymorphicOffsetPtrAllocator allocator1; diff --git a/score/memory/shared/shared_memory_resource_allocate_test.cpp b/score/memory/shared/shared_memory_resource_allocate_test.cpp index d48c4233..ab6485f5 100644 --- a/score/memory/shared/shared_memory_resource_allocate_test.cpp +++ b/score/memory/shared/shared_memory_resource_allocate_test.cpp @@ -66,7 +66,8 @@ TEST_F(SharedMemoryResourceAllocateTest, AssociatedMemoryResourceProxyForwardsCa // When allocating memory through its associated MemoryResourceProxy // That we don't receive a nullptr - EXPECT_NE(resource->getMemoryResourceProxy()->allocate(5U, 1U), nullptr); + ManagedMemoryResourceTestAttorney attorney(*resource); + EXPECT_NE(attorney.getMemoryResourceProxy()->allocate(5U, 1U), nullptr); EXPECT_EQ(controlBlock->alreadyAllocatedBytes, 5U); } @@ -239,7 +240,8 @@ TEST_F(SharedMemoryResourceAllocateDeathTest, AllocatingBlockLargerThanAllocated // When allocating a memory block that is larger than the allocated shared memory segment // Then the program terminates - EXPECT_DEATH(resource->getMemoryResourceProxy()->allocate(TestValues::some_share_memory_size + 1), ".*"); + ManagedMemoryResourceTestAttorney attorney(*resource); + EXPECT_DEATH(attorney.getMemoryResourceProxy()->allocate(TestValues::some_share_memory_size + 1), ".*"); } TEST_F(SharedMemoryResourceAllocateDeathTest, AllocatingMultipleBlocksLargerThanAllocatedSharedMemoryCausesTermination) @@ -273,14 +275,16 @@ TEST_F(SharedMemoryResourceAllocateDeathTest, AllocatingMultipleBlocksLargerThan // When allocating a memory block smaller than the allocated shared memory segment const auto memory_to_allocate = (TestValues::some_share_memory_size / 2); - EXPECT_NE(resource->getMemoryResourceProxy()->allocate(TestValues::some_share_memory_size / 2), nullptr); + ManagedMemoryResourceTestAttorney attorney1(*resource); + EXPECT_NE(attorney1.getMemoryResourceProxy()->allocate(TestValues::some_share_memory_size / 2), nullptr); // and then allocating another memory block such that the total memory block allocated is larger than the allocated // shared memory segment const auto remaining_memory = TestValues::some_share_memory_size - memory_to_allocate; // Then the program terminates - EXPECT_DEATH(resource->getMemoryResourceProxy()->allocate(remaining_memory + 1), ".*"); + ManagedMemoryResourceTestAttorney attorney2(*resource); + EXPECT_DEATH(attorney2.getMemoryResourceProxy()->allocate(remaining_memory + 1), ".*"); } } // namespace score::memory::shared::test diff --git a/score/memory/shared/shared_memory_test_resources.cpp b/score/memory/shared/shared_memory_test_resources.cpp index 18183b06..18aea0fd 100644 --- a/score/memory/shared/shared_memory_test_resources.cpp +++ b/score/memory/shared/shared_memory_test_resources.cpp @@ -88,6 +88,11 @@ const void* ManagedMemoryResourceTestAttorney::getEndAddress() const noexcept return resource_.getEndAddress(); } +const MemoryResourceProxy* ManagedMemoryResourceTestAttorney::getMemoryResourceProxy() const noexcept +{ + return resource_.getMemoryResourceProxy(); +} + SharedMemoryResourceTestAttorney::SharedMemoryResourceTestAttorney(SharedMemoryResource& resource) noexcept : resource_{resource} { diff --git a/score/memory/shared/shared_memory_test_resources.h b/score/memory/shared/shared_memory_test_resources.h index d11e0fed..5f92873d 100644 --- a/score/memory/shared/shared_memory_test_resources.h +++ b/score/memory/shared/shared_memory_test_resources.h @@ -65,6 +65,8 @@ class ManagedMemoryResourceTestAttorney const void* getEndAddress() const noexcept; + const MemoryResourceProxy* getMemoryResourceProxy() const noexcept; + private: ManagedMemoryResource& resource_; }; diff --git a/score/memory/shared/string_test.cpp b/score/memory/shared/string_test.cpp index 9428f3ab..64ca93fd 100644 --- a/score/memory/shared/string_test.cpp +++ b/score/memory/shared/string_test.cpp @@ -34,7 +34,7 @@ TEST(StringTest, StringUsesProvidedMemoryResource) { // Given a string that is associated with our memory resource test::MyMemoryResource memory{}; - PolymorphicOffsetPtrAllocator allocator{memory.getMemoryResourceProxy()}; + PolymorphicOffsetPtrAllocator allocator{memory}; String unit{allocator}; EXPECT_EQ(memory.getAllocatedMemory(), 0U); // A default-constructed string shall not allocate any data yet. @@ -48,7 +48,7 @@ TEST(StringTest, StringUsesProvidedMemoryResource) TEST(StringTest, CompareStringToStdString) { test::MyMemoryResource memory{}; - PolymorphicOffsetPtrAllocator allocator{memory.getMemoryResourceProxy()}; + PolymorphicOffsetPtrAllocator allocator{memory}; String my_string{"OÖKuzidaskjiksoaddszfkjdfdskjkjdskmlkjdnfmgbjhtknfgbiuhte", allocator}; const std::size_t after_first_allocation{memory.getAllocatedMemory()}; EXPECT_GT(after_first_allocation, 0U); @@ -70,7 +70,7 @@ TEST(StringTest, CompareStringToStdString) TEST(StringTest, OutputOperatorOverload) { test::MyMemoryResource memory{}; - PolymorphicOffsetPtrAllocator allocator{memory.getMemoryResourceProxy()}; + PolymorphicOffsetPtrAllocator allocator{memory}; String my_string{"OÖKuzidaskjiksoaddszfkjdfdskjkjdskmlkjdnfmgbjhtknfgbiuhte", allocator}; std::ostringstream out_stream; @@ -82,7 +82,7 @@ TEST(StringTest, OutputOperatorOverload) TEST(StringTest, InputOperatorOverload) { test::MyMemoryResource memory{}; - PolymorphicOffsetPtrAllocator allocator{memory.getMemoryResourceProxy()}; + PolymorphicOffsetPtrAllocator allocator{memory}; String my_string{"", allocator}; const char* const test_string{"OÖKuzidaskjiksoaddszfkjdfdskjkjdskmlkjdnfmgbjhtknfgbiuhte"}; diff --git a/score/memory/shared/vector_test.cpp b/score/memory/shared/vector_test.cpp index 74ff3c47..ed7c141c 100644 --- a/score/memory/shared/vector_test.cpp +++ b/score/memory/shared/vector_test.cpp @@ -25,7 +25,7 @@ TEST(Vector, OuterVectorAllocatesMemoryOnProvidedResource) { // Given a Vector of Vectors including integers test::MyMemoryResource memory{}; - score::memory::shared::Vector> unit{memory.getMemoryResourceProxy()}; + score::memory::shared::Vector> unit{memory}; auto before_allocating_vector = memory.getAllocatedMemory(); // When allocating a new inner vector @@ -40,7 +40,7 @@ TEST(Vector, InnerVectorAllocatesMemoryOnProvidedResource) { // Given a Vector of Vectors including integers test::MyMemoryResource memory{}; - score::memory::shared::Vector> unit{memory.getMemoryResourceProxy()}; + score::memory::shared::Vector> unit{memory}; unit.resize(1); auto before_allocating_integer = memory.getAllocatedMemory(); @@ -55,7 +55,7 @@ TEST(Vector, PositiveComparisonOfStdVector) { // Given a pmr::vector and a std::vector with the same content test::MyMemoryResource memory{}; - score::memory::shared::Vector unit{{1U, 2U, 3U}, memory.getMemoryResourceProxy()}; + score::memory::shared::Vector unit{{1U, 2U, 3U}, memory}; std::vector other{1U, 2U, 3U}; // When comparing them @@ -69,7 +69,7 @@ TEST(Vector, NegativeComparisonOfStdVector) { // Given a pmr::vector and a std::vector with different content test::MyMemoryResource memory{}; - score::memory::shared::Vector unit{{1U, 2U, 3U}, memory.getMemoryResourceProxy()}; + score::memory::shared::Vector unit{{1U, 2U, 3U}, memory}; std::vector other{1U, 3U, 3U}; // When comparing them @@ -83,7 +83,7 @@ TEST(Vector, PositiveComparisonOfStdVectorReverse) { // Given a pmr::vector and a std::vector with the same content test::MyMemoryResource memory{}; - score::memory::shared::Vector unit{{1U, 2U, 3U}, memory.getMemoryResourceProxy()}; + score::memory::shared::Vector unit{{1U, 2U, 3U}, memory}; std::vector other{1U, 2U, 3U}; // When comparing them @@ -97,7 +97,7 @@ TEST(Vector, NegativeComparisonOfStdVectorReverse) { // Given a pmr::vector and a std::vector with different content test::MyMemoryResource memory{}; - score::memory::shared::Vector unit{{1U, 2U, 3U}, memory.getMemoryResourceProxy()}; + score::memory::shared::Vector unit{{1U, 2U, 3U}, memory}; std::vector other{1U, 3U, 3U}; // When comparing them