Modified: trunk/Source/bmalloc/ChangeLog (280667 => 280668)
--- trunk/Source/bmalloc/ChangeLog 2021-08-04 22:24:44 UTC (rev 280667)
+++ trunk/Source/bmalloc/ChangeLog 2021-08-04 22:53:54 UTC (rev 280668)
@@ -1,3 +1,33 @@
+2021-08-04 Filip Pizlo <fpi...@apple.com>
+
+ [libpas] medium size class lookup needs to correctly fence the counting lock read path
+ https://bugs.webkit.org/show_bug.cgi?id=228799
+
+ Reviewed by Tadeu Zagallo.
+
+ The medium size class lookup does a binary search on a data structure that may mutate; we
+ catch that using a counting lock. But the algorithm wasn't fencing the tail end; it's supposed
+ to reread the count at the end but that read was not fenced.
+
+ This adds the fencing using pas_depend. I confirmed that the disassembly does the right thing.
+ It adds very little code.
+
+ Also rebased a test. Libpas tests are very specific about memory usage in some cases, and so
+ sometimes you will encounter a test run that requires limits to be adjusted. This happens
+ because some tests can sometimes create very complex heap layouts that really do use more
+ memory than we asserted, but the assertion had always worked because the test never ran with
+ the "wrong" kind of layout. This fixes a one-off test failure I saw when debugging this fix.
+
+ * libpas/src/libpas/pas_mutation_count.h:
+ (pas_mutation_count_matches_with_dependency):
+ (pas_mutation_count_matches): Deleted.
+ * libpas/src/libpas/pas_segregated_heap.c:
+ (medium_directory_tuple_for_index_impl):
+ (medium_directory_tuple_for_index_with_lock):
+ (pas_segregated_heap_medium_directory_tuple_for_index):
+ * libpas/src/test/ThingyAndUtilityHeapAllocationTests.cpp:
+ (std::addLargeHeapTests):
+
2021-08-03 Filip Pizlo <fpi...@apple.com>
pas_segmented_vector's iterate functions should handle memory ordering correctly
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_mutation_count.h (280667 => 280668)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_mutation_count.h 2021-08-04 22:24:44 UTC (rev 280667)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_mutation_count.h 2021-08-04 22:53:54 UTC (rev 280668)
@@ -66,11 +66,12 @@
return saved_mutation_count.count & PAS_MUTATION_COUNT_MUTATING_BIT;
}
-static inline bool pas_mutation_count_matches(pas_mutation_count* mutation_count,
- pas_mutation_count saved_mutation_count)
+static inline bool pas_mutation_count_matches_with_dependency(pas_mutation_count* mutation_count,
+ pas_mutation_count saved_mutation_count,
+ uintptr_t dependency)
{
pas_compiler_fence();
- return mutation_count->count == saved_mutation_count.count;
+ return mutation_count[pas_depend(dependency)].count == saved_mutation_count.count;
}
static inline unsigned pas_mutation_count_depend(pas_mutation_count saved_mutation_count)
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_heap.c (280667 => 280668)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_heap.c 2021-08-04 22:24:44 UTC (rev 280667)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_heap.c 2021-08-04 22:53:54 UTC (rev 280668)
@@ -234,7 +234,12 @@
return result;
}
-static pas_segregated_heap_medium_directory_tuple*
+typedef struct {
+ pas_segregated_heap_medium_directory_tuple* tuple;
+ uintptr_t dependency;
+} medium_directory_tuple_for_index_impl_result;
+
+static PAS_ALWAYS_INLINE medium_directory_tuple_for_index_impl_result
medium_directory_tuple_for_index_impl(
pas_segregated_heap_rare_data* rare_data,
pas_segregated_heap_medium_directory_tuple* medium_directories,
@@ -247,6 +252,7 @@
unsigned begin;
unsigned end;
pas_segregated_heap_medium_directory_tuple* best;
+ medium_directory_tuple_for_index_impl_result result;
PAS_ASSERT(rare_data);
@@ -256,10 +262,14 @@
begin = 0;
end = num_medium_directories;
best = NULL;
+
+ result.dependency = (uintptr_t)medium_directories;
while (end > begin) {
unsigned middle;
pas_segregated_heap_medium_directory_tuple* directory;
+ unsigned begin_index;
+ unsigned end_index;
middle = (begin + end) >> 1;
@@ -269,31 +279,40 @@
pas_log("begin = %u, end = %u, middle = %u, begin_index = %u, end_index = %u\n",
begin, end, middle, directory->begin_index, directory->end_index);
}
+
+ begin_index = directory->begin_index;
+ end_index = directory->end_index;
- if (index < directory->begin_index) {
+ result.dependency += begin_index + end_index;
+
+ if (index < begin_index) {
end = middle;
best = directory;
continue;
}
- if (index > directory->end_index) {
+ if (index > end_index) {
begin = middle + 1;
continue;
}
-
- return directory;
+
+ result.tuple = directory;
+ return result;
}
switch (search_mode) {
case pas_segregated_heap_medium_size_directory_search_within_size_class_progression:
- return NULL;
+ result.tuple = NULL;
+ return result;
case pas_segregated_heap_medium_size_directory_search_least_greater_equal:
- return best;
+ result.tuple = best;
+ return result;
}
PAS_ASSERT(!"Should not be reached");
- return NULL;
+ result.tuple = NULL;
+ return result;
}
static pas_segregated_heap_medium_directory_tuple*
@@ -318,7 +337,7 @@
medium_directories,
rare_data->num_medium_directories,
index,
- search_mode);
+ search_mode).tuple;
pas_heap_lock_unlock_conditionally(heap_lock_hold_mode);
@@ -336,7 +355,7 @@
pas_mutation_count saved_count;
pas_segregated_heap_medium_directory_tuple* medium_directories;
unsigned num_medium_directories;
- pas_segregated_heap_medium_directory_tuple* result;
+ medium_directory_tuple_for_index_impl_result result;
rare_data = pas_segregated_heap_rare_data_ptr_load(&heap->rare_data);
if (!rare_data)
@@ -361,8 +380,9 @@
result = medium_directory_tuple_for_index_impl(
rare_data, medium_directories, num_medium_directories, index, search_mode);
- if (pas_mutation_count_matches(&rare_data->mutation_count, saved_count))
- return result;
+ if (pas_mutation_count_matches_with_dependency(
+ &rare_data->mutation_count, saved_count, result.dependency))
+ return result.tuple;
return medium_directory_tuple_for_index_with_lock(
heap, index, search_mode, pas_lock_is_not_held);
Modified: trunk/Source/bmalloc/libpas/src/test/ThingyAndUtilityHeapAllocationTests.cpp (280667 => 280668)
--- trunk/Source/bmalloc/libpas/src/test/ThingyAndUtilityHeapAllocationTests.cpp 2021-08-04 22:24:44 UTC (rev 280667)
+++ trunk/Source/bmalloc/libpas/src/test/ThingyAndUtilityHeapAllocationTests.cpp 2021-08-04 22:53:54 UTC (rev 280668)
@@ -2720,7 +2720,7 @@
AllocationProgram::free("firstHalf"),
AllocationProgram::free("secondHalf")));
ADD_TEST(testComplexLargeAllocation(IsolatedComplexAllocator(7, 1),
- ExpectedBytes::upperBound(417792),
+ ExpectedBytes::upperBound(638976),
AllocationProgram::allocate("boot", 40960, 1),
AllocationProgram::free("boot"),
AllocationProgram::allocate("big", 9362, 65536),
