Title: [287991] trunk/Source/bmalloc
- Revision
- 287991
- Author
- fpi...@apple.com
- Date
- 2022-01-13 13:12:09 -0800 (Thu, 13 Jan 2022)
Log Message
[libpas] add assertions that we aren't switching to a NULL lock
https://bugs.webkit.org/show_bug.cgi?id=235190
Reviewed by Yusuke Suzuki.
This adds a pas_panic call when pas_local_allocator_stop sees a NULL page->lock_ptr. That's one
possible explanation of a very rare crash I'm seeing where return_memory_to_page fails its assertion
that we are holding the page lock.
This also adds TESTING asserts in a bunch of other places. The PAS_TESTING_ASSERTS about this are in
places that are perf-sensitive, so we probably cannot assert in production. The hope behind those is
that it will help to catch this issue in test_pas.
* libpas/src/libpas/pas_local_allocator.c:
(stop_impl):
* libpas/src/libpas/pas_segregated_page.c:
(pas_segregated_page_switch_lock_and_rebias_while_ineligible_impl):
* libpas/src/libpas/pas_segregated_page_inlines.h:
(pas_segregated_page_lock_with_unbias_not_utility):
(pas_segregated_page_lock_with_unbias):
(pas_segregated_page_lock):
(pas_segregated_page_switch_lock_impl):
(pas_segregated_page_switch_lock_with_mode):
Modified Paths
Diff
Modified: trunk/Source/bmalloc/ChangeLog (287990 => 287991)
--- trunk/Source/bmalloc/ChangeLog 2022-01-13 21:11:10 UTC (rev 287990)
+++ trunk/Source/bmalloc/ChangeLog 2022-01-13 21:12:09 UTC (rev 287991)
@@ -1,3 +1,29 @@
+2022-01-13 Filip Pizlo <fpi...@apple.com>
+
+ [libpas] add assertions that we aren't switching to a NULL lock
+ https://bugs.webkit.org/show_bug.cgi?id=235190
+
+ Reviewed by Yusuke Suzuki.
+
+ This adds a pas_panic call when pas_local_allocator_stop sees a NULL page->lock_ptr. That's one
+ possible explanation of a very rare crash I'm seeing where return_memory_to_page fails its assertion
+ that we are holding the page lock.
+
+ This also adds TESTING asserts in a bunch of other places. The PAS_TESTING_ASSERTS about this are in
+ places that are perf-sensitive, so we probably cannot assert in production. The hope behind those is
+ that it will help to catch this issue in test_pas.
+
+ * libpas/src/libpas/pas_local_allocator.c:
+ (stop_impl):
+ * libpas/src/libpas/pas_segregated_page.c:
+ (pas_segregated_page_switch_lock_and_rebias_while_ineligible_impl):
+ * libpas/src/libpas/pas_segregated_page_inlines.h:
+ (pas_segregated_page_lock_with_unbias_not_utility):
+ (pas_segregated_page_lock_with_unbias):
+ (pas_segregated_page_lock):
+ (pas_segregated_page_switch_lock_impl):
+ (pas_segregated_page_switch_lock_with_mode):
+
2022-01-12 Filip Pizlo <fpi...@apple.com>
[libpas] thread_local_cache should not be allocated in the compact heap (cherry pick 11afcedfb5968f6894379ff1a41dd449ba7745f6)
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c (287990 => 287991)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c 2022-01-13 21:11:10 UTC (rev 287990)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c 2022-01-13 21:12:09 UTC (rev 287991)
@@ -180,6 +180,9 @@
if (!pas_segregated_page_switch_lock_with_mode(page, &held_lock, page_lock_mode, page_config))
return false;
+
+ if (!pas_segregated_page_config_is_utility(page_config) && !held_lock)
+ pas_panic("Should be holding a lock after pas_segregated_page_switch_lock_with_mode in stop_impl\n");
page_config.specialized_local_allocator_return_memory_to_page(
allocator, view, page, directory, heap_lock_hold_mode);
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page.c (287990 => 287991)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page.c 2022-01-13 21:11:10 UTC (rev 287990)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page.c 2022-01-13 21:12:09 UTC (rev 287991)
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018-2021 Apple Inc. All rights reserved.
+ * Copyright (c) 2018-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -125,6 +125,7 @@
bool got_right_lock;
page_lock = page->lock_ptr;
+ PAS_TESTING_ASSERT(page_lock);
if (*held_lock == page_lock && *held_lock == &cache_node->page_lock) {
pas_compiler_fence();
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h (287990 => 287991)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2022-01-13 21:11:10 UTC (rev 287990)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2022-01-13 21:12:09 UTC (rev 287991)
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018-2021 Apple Inc. All rights reserved.
+ * Copyright (c) 2018-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -130,6 +130,8 @@
pas_lock** held_lock,
pas_lock* lock_ptr)
{
+ PAS_TESTING_ASSERT(lock_ptr);
+
*held_lock = lock_ptr;
if (PAS_LIKELY(pas_lock_try_lock(lock_ptr)))
@@ -151,6 +153,8 @@
return true;
}
+ PAS_TESTING_ASSERT(lock_ptr);
+
return pas_segregated_page_lock_with_unbias_not_utility(page, held_lock, lock_ptr);
}
@@ -170,7 +174,7 @@
pas_lock* held_lock_ignored;
lock_ptr = page->lock_ptr;
-
+
if (pas_segregated_page_lock_with_unbias(page, &held_lock_ignored, lock_ptr, page_config))
return;
@@ -208,6 +212,8 @@
held_lock_value = *held_lock;
page_lock = page->lock_ptr;
+
+ PAS_TESTING_ASSERT(page_lock);
if (PAS_LIKELY(held_lock_value == page_lock)) {
if (verbose)
@@ -232,8 +238,12 @@
}
switch (lock_mode) {
- case pas_lock_lock_mode_try_lock:
- return pas_lock_switch_with_mode(held_lock, page->lock_ptr, pas_lock_lock_mode_try_lock);
+ case pas_lock_lock_mode_try_lock: {
+ pas_lock* page_lock;
+ page_lock = page->lock_ptr;
+ PAS_TESTING_ASSERT(page_lock);
+ return pas_lock_switch_with_mode(held_lock, page_lock, pas_lock_lock_mode_try_lock);
+ }
case pas_lock_lock_mode_lock: {
pas_segregated_page_switch_lock_impl(page, held_lock);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
