Skip to content

Commit 3d2f5f7

Browse files
I've made some changes to help diagnose a segmentation fault that's been happening on ARM platforms. Here's what I did:
1. I enabled a `debug` feature in the project's configuration for the `gcmodule`. According to the library's documentation, this should add more checks that can turn undefined behavior or segfaults into panics with clearer messages. 2. I added some assertions in `src/collect.rs` to ensure that certain pointers (`GcHeader` pointers) are aligned correctly. These checks are in places where these pointers are used for storing extra information. If these pointers aren't aligned properly on ARM, it could corrupt the data and lead to segfaults. These new assertions will cause a panic if any misalignments are found, which should help us track down such problems. The goal of these changes is to give us more detailed information when the code runs on an ARM environment, making it easier to find the cause of the segfault.
1 parent b95d96b commit 3d2f5f7

File tree

2 files changed

+33
-4
lines changed

2 files changed

+33
-4
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ gcmodule_derive = { version = "=0.3.3", optional = true, path = "gcmodule_derive
1818
parking_lot = { version = "0.10", optional = true }
1919

2020
[features]
21-
default = ["derive", "sync"]
21+
default = ["derive", "sync", "debug"]
2222
debug = []
2323
derive = ["gcmodule_derive"]
2424
nightly = []

src/collect.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ impl AbstractObjectSpace for ObjectSpace {
8787
type Header = GcHeader;
8888

8989
fn insert(&self, header: &mut Self::Header, value: &dyn CcDyn) {
90+
// Assert GcHeader alignment during insertion
91+
assert_eq!((header as *const _ as usize) % 4, 0, "Inserted GcHeader pointer is not 4-byte aligned");
9092
let prev: &GcHeader = &self.list.borrow();
9193
debug_assert!(header.next.get().is_null());
9294
let next = prev.next.get();
@@ -283,6 +285,11 @@ const PREV_SHIFT: u32 = 2;
283285
/// Idea comes from https://bugs.python.org/issue33597.
284286
fn update_refs<L: Linked>(list: &L) {
285287
visit_list(list, |header| {
288+
// Assert GcHeader alignment before modifying prev
289+
assert_eq!((header as *const _ as usize) % 4, 0, "GcHeader pointer in update_refs is not 4-byte aligned");
290+
let original_prev = header.prev();
291+
assert_eq!((original_prev as usize) % 4, 0, "GcHeader.prev pointer in update_refs is not 4-byte aligned before modification");
292+
286293
let ref_count = header.value().gc_ref_count();
287294
// It's possible that the ref_count becomes 0 in a multi-thread context:
288295
// thread 1> drop()
@@ -426,11 +433,33 @@ fn release_unreachable<L: Linked, K>(list: &L, lock: K) -> usize {
426433

427434
/// Restore `GcHeader.prev` as a pointer used in the linked list.
428435
fn restore_prev<L: Linked>(list: &L) {
429-
let mut prev = list;
436+
let mut prev_node_in_list = list;
430437
visit_list(list, |header| {
431-
header.set_prev(prev);
432-
prev = header;
438+
// Assert GcHeader alignment before modifying prev
439+
assert_eq!((header as *const _ as usize) % 4, 0, "GcHeader pointer in restore_prev is not 4-byte aligned");
440+
441+
header.set_prev(prev_node_in_list);
442+
443+
// Assert GcHeader alignment after restoring prev
444+
let restored_prev_ptr = header.prev();
445+
assert_eq!((restored_prev_ptr as usize) % 4, 0, "Restored GcHeader.prev pointer in restore_prev is not 4-byte aligned");
446+
447+
prev_node_in_list = header;
433448
});
449+
450+
// Final check for the list head itself if it's part of the restoration logic,
451+
// ensuring its prev points to the last element which should also be aligned.
452+
// The list head's prev pointer should point to the last actual element in the list.
453+
// And the last element's next pointer should point to the list head.
454+
// visit_list does not yield the list head itself, so we check list.prev()
455+
// which should be the last element processed by visit_list's `prev = header`
456+
// then that last element's prev should be the one before it.
457+
// The `list` itself (sentinel) should also be aligned.
458+
assert_eq!((list as *const _ as usize) % 4, 0, "List sentinel pointer is not 4-byte aligned in restore_prev");
459+
if list.next() as *const _ != list as *const _ { // if not empty list
460+
let last_elem_ptr = list.prev(); // This is what `prev_node_in_list` became after the loop
461+
assert_eq!((last_elem_ptr as usize) % 4, 0, "List sentinel's prev pointer (last element) is not 4-byte aligned in restore_prev");
462+
}
434463
}
435464

436465
fn is_unreachable<L: Linked>(header: &L) -> bool {

0 commit comments

Comments
 (0)