- Revision
- 207062
- Author
- carlo...@webkit.org
- Date
- 2016-10-11 01:05:04 -0700 (Tue, 11 Oct 2016)
Log Message
Merge r205694 - [WTF] HashTable's rehash is not compatible to Ref<T> and ASan
https://bugs.webkit.org/show_bug.cgi?id=161763
Reviewed by Mark Lam.
Source/WebCore:
Include wtf/text/StringHash.h to avoid linking errors in EFL port.
* loader/ResourceLoadStatistics.h:
Source/WTF:
If we move an object, the location which the moved object used should not be touched anymore.
HashTable::rehash performs WTFMove for the object that resides in the old table.
However, after moving it, we accidentally touch this location by using `!isEmptyOrDeletedBucket(table[i])`
in HashTable::deallocateTable. And it causes ASan crashing if we use Ref<> for HashTable's key or value.
In this patch, we call the destructor right after moving the object. And HashTable::rehash just calls
fastFree since all the objects in the old table are already moved and destructed.
And we also change HashTable::deallocate to destruct only live objects. Calling destructors for empty objects
is meaningless. And according to the Ref<>'s comment, empty object is not designed to be destructed.
* wtf/HashTable.h:
(WTF::KeyTraits>::deallocateTable):
Tools:
Add tests that inserts many Ref<>s. It incurs HashTable::rehash, and we can ensure
that ASan crash does not occur with this patch.
* TestWebKitAPI/Tests/WTF/HashMap.cpp:
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WTF/HashSet.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog 2016-10-11 08:05:04 UTC (rev 207062)
@@ -1,3 +1,23 @@
+2016-09-08 Yusuke Suzuki <utatane....@gmail.com>
+
+ [WTF] HashTable's rehash is not compatible to Ref<T> and ASan
+ https://bugs.webkit.org/show_bug.cgi?id=161763
+
+ Reviewed by Mark Lam.
+
+ If we move an object, the location which the moved object used should not be touched anymore.
+ HashTable::rehash performs WTFMove for the object that resides in the old table.
+ However, after moving it, we accidentally touch this location by using `!isEmptyOrDeletedBucket(table[i])`
+ in HashTable::deallocateTable. And it causes ASan crashing if we use Ref<> for HashTable's key or value.
+
+ In this patch, we call the destructor right after moving the object. And HashTable::rehash just calls
+ fastFree since all the objects in the old table are already moved and destructed.
+ And we also change HashTable::deallocate to destruct only live objects. Calling destructors for empty objects
+ is meaningless. And according to the Ref<>'s comment, empty object is not designed to be destructed.
+
+ * wtf/HashTable.h:
+ (WTF::KeyTraits>::deallocateTable):
+
2016-09-08 Filip Pizlo <fpi...@apple.com>
Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
Modified: releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/HashTable.h (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/HashTable.h 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/HashTable.h 2016-10-11 08:05:04 UTC (rev 207062)
@@ -1153,7 +1153,7 @@
void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::deallocateTable(ValueType* table, unsigned size)
{
for (unsigned i = 0; i < size; ++i) {
- if (!isDeletedBucket(table[i]))
+ if (!isEmptyOrDeletedBucket(table[i]))
table[i].~ValueType();
}
fastFree(table);
@@ -1203,6 +1203,7 @@
}
Value* reinsertedEntry = reinsert(WTFMove(oldTable[i]));
+ oldTable[i].~ValueType();
if (std::addressof(oldTable[i]) == entry) {
ASSERT(!newEntry);
newEntry = reinsertedEntry;
@@ -1211,7 +1212,7 @@
m_deletedCount = 0;
- deallocateTable(oldTable, oldTableSize);
+ fastFree(oldTable);
internalCheckTableConsistency();
return newEntry;
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-10-11 08:05:04 UTC (rev 207062)
@@ -1,3 +1,14 @@
+2016-09-08 Yusuke Suzuki <utatane....@gmail.com>
+
+ [WTF] HashTable's rehash is not compatible to Ref<T> and ASan
+ https://bugs.webkit.org/show_bug.cgi?id=161763
+
+ Reviewed by Mark Lam.
+
+ Include wtf/text/StringHash.h to avoid linking errors in EFL port.
+
+ * loader/ResourceLoadStatistics.h:
+
2016-09-08 Simon Fraser <simon.fra...@apple.com>
Don't run transitions to or from undefined Lengths
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/ResourceLoadStatistics.h (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/ResourceLoadStatistics.h 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/ResourceLoadStatistics.h 2016-10-11 08:05:04 UTC (rev 207062)
@@ -27,6 +27,7 @@
#define ResourceLoadStatistics_h
#include <wtf/HashCountedSet.h>
+#include <wtf/text/StringHash.h>
#include <wtf/text/WTFString.h>
namespace WebCore {
Modified: releases/WebKitGTK/webkit-2.14/Tools/ChangeLog (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Tools/ChangeLog 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Tools/ChangeLog 2016-10-11 08:05:04 UTC (rev 207062)
@@ -1,3 +1,18 @@
+2016-09-08 Yusuke Suzuki <utatane....@gmail.com>
+
+ [WTF] HashTable's rehash is not compatible to Ref<T> and ASan
+ https://bugs.webkit.org/show_bug.cgi?id=161763
+
+ Reviewed by Mark Lam.
+
+ Add tests that inserts many Ref<>s. It incurs HashTable::rehash, and we can ensure
+ that ASan crash does not occur with this patch.
+
+ * TestWebKitAPI/Tests/WTF/HashMap.cpp:
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/Tests/WTF/HashSet.cpp:
+ (TestWebKitAPI::TEST):
+
2016-08-31 Filip Pizlo <fpi...@apple.com>
Butterflies should be allocated in Auxiliary MarkedSpace instead of CopiedSpace and we should rewrite as much of the GC as needed to make this not a regression
Modified: releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp 2016-10-11 08:05:04 UTC (rev 207062)
@@ -789,6 +789,18 @@
}
ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+
+ {
+ HashMap<Ref<RefLogger>, int> map;
+ for (int i = 0; i < 64; ++i) {
+ Ref<RefLogger> ref = adoptRef(*new RefLogger("a"));
+ auto* pointer = ref.ptr();
+ map.add(WTFMove(ref), i + 1);
+ ASSERT_TRUE(map.contains(pointer));
+ }
+ }
+
+ ASSERT_STREQ("deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) ", takeLogStr().c_str());
}
TEST(WTF_HashMap, Ref_Value)
@@ -890,6 +902,17 @@
}
ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+
+ {
+ HashMap<int, Ref<RefLogger>> map;
+ for (int i = 0; i < 64; ++i) {
+ Ref<RefLogger> ref = adoptRef(*new RefLogger("a"));
+ map.add(i + 1, WTFMove(ref));
+ ASSERT_TRUE(map.contains(i + 1));
+ }
+ }
+
+ ASSERT_STREQ("deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) ", takeLogStr().c_str());
}
TEST(WTF_HashMap, DeletedAddressOfOperator)
Modified: releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashSet.cpp (207061 => 207062)
--- releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashSet.cpp 2016-10-11 07:38:03 UTC (rev 207061)
+++ releases/WebKitGTK/webkit-2.14/Tools/TestWebKitAPI/Tests/WTF/HashSet.cpp 2016-10-11 08:05:04 UTC (rev 207062)
@@ -428,6 +428,17 @@
}
ASSERT_STREQ("ref(a) deref(a) ", takeLogStr().c_str());
+
+ {
+ HashSet<Ref<RefLogger>> set;
+ for (int i = 0; i < 64; ++i) {
+ Ref<RefLogger> ref = adoptRef(*new RefLogger("a"));
+ auto* pointer = ref.ptr();
+ set.add(WTFMove(ref));
+ ASSERT_TRUE(set.contains(pointer));
+ }
+ }
+ ASSERT_STREQ("deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) deref(a) ", takeLogStr().c_str());
}
TEST(WTF_HashSet, DeletedAddressOfOperator)
