clist: make debugging better

arch/x86/include/arch/poison.h

@@ -0,0 +1,9 @@
+/* Copyright (C) 2021 fef <owo@fef.moe>.  All rights reserved. */
+
+#pragma once
+
+#ifdef __x86_64__
+#define POISON_BASE 0xdead000000000000ul
+#else
+#define POISON_BASE 0x00000000ul
+#endif

include/gay/clist.h

@@ -238,6 +238,6 @@ struct clist *clist_del_last(struct clist *head);
 #define clist_foreach_entry_safe(head, cursor, tmp, member)			\
 	for ((cursor) = clist_first_entry(head, typeof(*(cursor)), member),	\
 		(tmp) = clist_next_entry(cursor, member);			\
-	     (tmp)->member.prev != (head);					\
+	     &(cursor)->member != (head);					\
 	     (cursor) = (tmp),							\
 		(tmp) = clist_next_entry(tmp, member))

include/gay/poison.h

@@ -0,0 +1,30 @@
+/* Copyright (C) 2021 fef <owo@fef.moe>.  All rights reserved. */
+
+#pragma once
+
+#include <arch/poison.h>
+
+/*
+ * If possible, arch should define this value as an unmappable base address.
+ * For example, on the amd64, this is set to 0xdead000000000000 because the
+ * highest 17 bits of linear addresses must always be identical there.
+ * As a result, it will *always* page fault when dereferenced, no matter
+ * the current state of the page maps.
+ */
+#ifndef POISON_BASE
+#define POISON_BASE 0ul
+#endif
+
+/*
+ * You should only use values that `sus_nil()` finds sus (i.e. values < 0x1000),
+ * and whose LSB is zero.  The latter is a little safety measure because most
+ * architectures use bit 0 of the page maps as a present flag.  So, even if the
+ * impossible happens and this *somehow* ends up in a page map, it is marked as
+ * non-present and therefore page faults rather than exposing anything.
+ */
+
+#define PAGE_POISON_ALLOC	(POISON_BASE + 0x00000010ul)
+#define PAGE_POISON_FREE	(POISON_BASE + 0x00000020ul)
+
+#define CLIST_POISON_PREV	(POISON_BASE + 0x000000c4ul)
+#define CLIST_POISON_NEXT	(POISON_BASE + 0x000000c8ul)

kernel/clist.c

@@ -3,13 +3,23 @@
 #include <gay/clist.h>
 #include <gay/config.h>
 #include <gay/kprintf.h>
+#include <gay/poison.h>
+#include <gay/systm.h>
+
+#if CFG_DEBUG_CLIST
+#define CLIST_DEBUG_BLOCK
+#define CLIST_ASSERT(x) KASSERT(x)
+#else
+#define CLIST_DEBUG_BLOCK if (0)
+#define CLIST_ASSERT(x) ({})
+#endif
 
 void clist_init(struct clist *head)
 {
-#	if CFG_DEBUG_CLIST
+	CLIST_DEBUG_BLOCK {
 		if (head->next == head && head->prev == head)
 			kprintf("clist_init(%p) called multiple times\n", head);
-#	endif
+	}
 
 	head->next = head;
 	head->prev = head;
@@ -17,16 +27,8 @@ void clist_init(struct clist *head)
 
 void clist_add(struct clist *head, struct clist *new)
 {
-#	if CFG_DEBUG_CLIST
-		if (sus_nil(head->next) || sus_nil(head->prev)) {
-			kprintf("clist_add(%p, %p): head seems uninitialized\n", head, new);
-			return;
-		}
-		if (sus_nil(new)) {
-			kprintf("clist_add(%p, %p): new node is NULL!\n", head, new);
-			return;
-		}
-#	endif
+	CLIST_ASSERT(!sus_nil(head->next));
+	CLIST_ASSERT(!sus_nil(new));
 
 	head->prev->next = new;
 	new->next = head;
@@ -49,20 +51,15 @@ void clist_del(struct clist *node)
 	node->next->prev = node->prev;
 	node->prev->next = node->next;
 
-#	if CFG_DEBUG_CLIST
-		node->next = nil;
-		node->prev = nil;
-#	endif
+	CLIST_DEBUG_BLOCK {
+		node->next = (struct clist *)CLIST_POISON_NEXT;
+		node->prev = (struct clist *)CLIST_POISON_PREV;
+	}
 }
 
 struct clist *clist_del_first(struct clist *head)
 {
-#	if CFG_DEBUG_CLIST
-		if (clist_is_empty(head)) {
-			kprintf("clist_del_first(%p): empty list!\n", head);
-			return nil;
-		}
-#	endif
+	CLIST_ASSERT(!clist_is_empty(head));
 
 	struct clist *first = head->next;
 	clist_del(first);
@@ -71,12 +68,7 @@ struct clist *clist_del_first(struct clist *head)
 
 struct clist *clist_del_last(struct clist *head)
 {
-#	ifdef CFG_DEBUG_CLIST
-		if (clist_is_empty(head)) {
-			kprintf("clist_del_last(%p): empty list!\n", head);
-			return nil;
-		}
-#	endif
+	CLIST_ASSERT(!clist_is_empty(head));
 
 	struct clist *last = head->prev;
 	clist_del(last);