From bcc23933695e1cec4e7dcd2e2b8bde0ef3c298c9 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Fri, 30 Jul 2010 05:49:32 +0000 Subject: [PATCH] Fix the ValueMap copy constructor. The issue is that the map keys are value handles with a pointer to the containing map. When a map is copied, these pointers need to be corrected to point to the new map. If not, then consider the case of a map M1 which maps a value V to something. Create a copy M2 of M1. At this point there are two value handles on V, one representing V as a key in M1, the other representing V as a key in M2. But both value handles point to M1 as the containing map. Now delete V. The value handles remove themselves from their containing map (which destroys them), but only the first value handle is successful: the second one cannot remove itself from M1 as (once the first one has removed itself) there is nothing there to remove; it is therefore not destroyed. This causes an assertion failure "All references to V were not removed?". git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@109851 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/ValueMap.h | 7 ++++++- unittests/ADT/ValueMapTest.cpp | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/include/llvm/ADT/ValueMap.h b/include/llvm/ADT/ValueMap.h index 98e43773258..f7560681097 100644 --- a/include/llvm/ADT/ValueMap.h +++ b/include/llvm/ADT/ValueMap.h @@ -87,7 +87,12 @@ public: typedef ValueT mapped_type; typedef std::pair value_type; - ValueMap(const ValueMap& Other) : Map(Other.Map), Data(Other.Data) {} + ValueMap(const ValueMap& Other) : Map(Other.Map), Data(Other.Data) { + // Each ValueMapCVH key contains a pointer to the containing ValueMap. + // The keys in the new map need to point to the new map, not Other. + for (typename MapT::iterator I = Map.begin(), E = Map.end(); I != E; ++I) + I->first.Map = this; + } explicit ValueMap(unsigned NumInitBuckets = 64) : Map(NumInitBuckets), Data() {} diff --git a/unittests/ADT/ValueMapTest.cpp b/unittests/ADT/ValueMapTest.cpp index 152e8eaaf1f..ff7c3b55b70 100644 --- a/unittests/ADT/ValueMapTest.cpp +++ b/unittests/ADT/ValueMapTest.cpp @@ -39,6 +39,15 @@ protected: typedef ::testing::Types KeyTypes; TYPED_TEST_CASE(ValueMapTest, KeyTypes); +TYPED_TEST(ValueMapTest, CopyConstructor) { + ValueMap VM1; + VM1[this->AddV.get()] = 7; + ValueMap VM2(VM1); + this->AddV.reset(); + EXPECT_TRUE(VM1.empty()); + EXPECT_TRUE(VM2.empty()); +} + TYPED_TEST(ValueMapTest, Null) { ValueMap VM1; VM1[NULL] = 7; -- 2.34.1