sgraenitz created this revision.
sgraenitz added reviewers: jingham, friss, labath.
This issue came up because it caused problems in our unit tests. The StringPool
did connect counterparts only once and silently ignored the values passed in
subsequent calls.
The simplest solution for the unit tests would be silent overwrite. In
practice, however, it seems useful to assert that we never overwrite a
different mangled counterpart.
If we ever have mangled counterparts for other languages than C++, this makes
it more likely to notice collisions.
I added an assertion that allows the following cases:
- inserting a new value
- overwriting the empty string
- overwriting with an identical value
I fixed the unit tests, which used "random" strings and thus produced
collisions.
It would be even better if there was a way to reset or isolate the StringPool,
but that's a different story.
https://reviews.llvm.org/D50536
Files:
source/Utility/ConstString.cpp
unittests/Utility/ConstStringTest.cpp
Index: unittests/Utility/ConstStringTest.cpp
===================================================================
--- unittests/Utility/ConstStringTest.cpp
+++ unittests/Utility/ConstStringTest.cpp
@@ -18,40 +18,56 @@
}
TEST(ConstStringTest, MangledCounterpart) {
- ConstString foo("foo");
+ ConstString uvw("uvw");
ConstString counterpart;
- EXPECT_FALSE(foo.GetMangledCounterpart(counterpart));
+ EXPECT_FALSE(uvw.GetMangledCounterpart(counterpart));
EXPECT_EQ("", counterpart.GetStringRef());
- ConstString bar;
- bar.SetStringWithMangledCounterpart("bar", foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ ConstString xyz;
+ xyz.SetStringWithMangledCounterpart("xyz", uvw);
+ EXPECT_EQ("xyz", xyz.GetStringRef());
- EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_TRUE(xyz.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("uvw", counterpart.GetStringRef());
- EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_TRUE(uvw.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("xyz", counterpart.GetStringRef());
+}
+
+TEST(ConstStringTest, UpdateMangledCounterpart) {
+ { // Init counterpart with empty string
+ ConstString some1;
+ some1.SetStringWithMangledCounterpart("some", ConstString(""));
+ }
+ { // Update counterpart to "one"
+ ConstString some2;
+ some2.SetStringWithMangledCounterpart("some", ConstString("one"));
+ }
+ { // Check counterpart is "one"
+ ConstString counterpart;
+ EXPECT_TRUE(ConstString("some").GetMangledCounterpart(counterpart));
+ EXPECT_EQ("one", counterpart.GetStringRef());
+ }
}
TEST(ConstStringTest, FromMidOfBufferStringRef) {
// StringRef's into bigger buffer: no null termination
- const char *buffer = "foobarbaz";
+ const char *buffer = "abcdefghi";
llvm::StringRef foo_ref(buffer, 3);
llvm::StringRef bar_ref(buffer + 3, 3);
ConstString foo(foo_ref);
ConstString bar;
bar.SetStringWithMangledCounterpart(bar_ref, foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ EXPECT_EQ("def", bar.GetStringRef());
ConstString counterpart;
EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_EQ("abc", counterpart.GetStringRef());
EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_EQ("def", counterpart.GetStringRef());
}
TEST(ConstStringTest, NullAndEmptyStates) {
Index: source/Utility/ConstString.cpp
===================================================================
--- source/Utility/ConstString.cpp
+++ source/Utility/ConstString.cpp
@@ -119,11 +119,15 @@
const uint8_t h = hash(demangled);
llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
- // Make string pool entry with the mangled counterpart already set
- StringPoolEntryType &entry =
- *m_string_pools[h]
- .m_string_map.insert(std::make_pair(demangled, mangled_ccstr))
- .first;
+ StringPool &map = m_string_pools[h].m_string_map;
+ assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0
||
+ map[demangled] == mangled_ccstr) &&
+ "The demangled string must have a unique counterpart or otherwise
"
+ "it must be empty");
+
+ // Make or update string pool entry with the mangled counterpart
+ StringPoolEntryType &entry = *map.try_emplace(demangled).first;
+ entry.second = mangled_ccstr;
// Extract the const version of the demangled_cstr
demangled_ccstr = entry.getKeyData();
Index: unittests/Utility/ConstStringTest.cpp
===================================================================
--- unittests/Utility/ConstStringTest.cpp
+++ unittests/Utility/ConstStringTest.cpp
@@ -18,40 +18,56 @@
}
TEST(ConstStringTest, MangledCounterpart) {
- ConstString foo("foo");
+ ConstString uvw("uvw");
ConstString counterpart;
- EXPECT_FALSE(foo.GetMangledCounterpart(counterpart));
+ EXPECT_FALSE(uvw.GetMangledCounterpart(counterpart));
EXPECT_EQ("", counterpart.GetStringRef());
- ConstString bar;
- bar.SetStringWithMangledCounterpart("bar", foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ ConstString xyz;
+ xyz.SetStringWithMangledCounterpart("xyz", uvw);
+ EXPECT_EQ("xyz", xyz.GetStringRef());
- EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_TRUE(xyz.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("uvw", counterpart.GetStringRef());
- EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_TRUE(uvw.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("xyz", counterpart.GetStringRef());
+}
+
+TEST(ConstStringTest, UpdateMangledCounterpart) {
+ { // Init counterpart with empty string
+ ConstString some1;
+ some1.SetStringWithMangledCounterpart("some", ConstString(""));
+ }
+ { // Update counterpart to "one"
+ ConstString some2;
+ some2.SetStringWithMangledCounterpart("some", ConstString("one"));
+ }
+ { // Check counterpart is "one"
+ ConstString counterpart;
+ EXPECT_TRUE(ConstString("some").GetMangledCounterpart(counterpart));
+ EXPECT_EQ("one", counterpart.GetStringRef());
+ }
}
TEST(ConstStringTest, FromMidOfBufferStringRef) {
// StringRef's into bigger buffer: no null termination
- const char *buffer = "foobarbaz";
+ const char *buffer = "abcdefghi";
llvm::StringRef foo_ref(buffer, 3);
llvm::StringRef bar_ref(buffer + 3, 3);
ConstString foo(foo_ref);
ConstString bar;
bar.SetStringWithMangledCounterpart(bar_ref, foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ EXPECT_EQ("def", bar.GetStringRef());
ConstString counterpart;
EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_EQ("abc", counterpart.GetStringRef());
EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_EQ("def", counterpart.GetStringRef());
}
TEST(ConstStringTest, NullAndEmptyStates) {
Index: source/Utility/ConstString.cpp
===================================================================
--- source/Utility/ConstString.cpp
+++ source/Utility/ConstString.cpp
@@ -119,11 +119,15 @@
const uint8_t h = hash(demangled);
llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
- // Make string pool entry with the mangled counterpart already set
- StringPoolEntryType &entry =
- *m_string_pools[h]
- .m_string_map.insert(std::make_pair(demangled, mangled_ccstr))
- .first;
+ StringPool &map = m_string_pools[h].m_string_map;
+ assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0 ||
+ map[demangled] == mangled_ccstr) &&
+ "The demangled string must have a unique counterpart or otherwise "
+ "it must be empty");
+
+ // Make or update string pool entry with the mangled counterpart
+ StringPoolEntryType &entry = *map.try_emplace(demangled).first;
+ entry.second = mangled_ccstr;
// Extract the const version of the demangled_cstr
demangled_ccstr = entry.getKeyData();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
