From 23cc994255d9d24e9dcfa667834d2aa0ed936a3f Mon Sep 17 00:00:00 2001
From: alyekypo <alyekypo@gmail.com>
Date: Mon, 21 Sep 2026 13:09:25 +0000
Subject: [PATCH] fix(checker): keep entity iteration valid across a scope map
 rehash

`check_scope_decls` iterates a scope's entity map while
`check_entity_decl` can insert into that same scope: a procedure alias
(`f :: e`) goes through `override_entity_in_scope`, which calls
`scope_map_insert`. The map grows at 75% load (12 of 16 slots, then 24
of 32), and `scope_map_insert` grows before it probes for the key, so
even an alias that only replaces an existing value rehashes and
reallocates the table in the middle of the iteration.

`ScopeMapIterator` re-read its map on every step while `end` had
captured the capacity at loop start, so the walk continued over the new
table under the old sentinel: it ran past the end of the reallocated
table and dereferenced whatever followed it as an `Entity *` - a nullptr
dereference for a procedure scope holding exactly 12 or 24 entities - or
it stopped at the stale end index and silently skipped every declaration
that the rehash had moved behind it, which left unused declarations
without any diagnostic at all.

The iterator now snapshots the table pointer and capacity when it is
constructed, so iteration is invariant under any insert or grow and
cannot leave the table it began on. A scope that does not grow behaves
exactly as before: the snapshot points at the same table.

Refs #7598
---
 src/check_expr.cpp                            |   3 +
 src/checker.hpp                               |  28 +-
 tests/issues/run.sh                           |   9 +
 tests/issues/test_issue_7598.odin             | 307 ++++++++++++++++++
 .../test_issue_7598_all_entities_checked.odin |  55 ++++
 5 files changed, 393 insertions(+), 9 deletions(-)
 create mode 100644 tests/issues/test_issue_7598.odin
 create mode 100644 tests/issues/test_issue_7598_all_entities_checked.odin

diff --git a/src/check_expr.cpp b/src/check_expr.cpp
index aa62a6114..1a4229475 100644
--- a/src/check_expr.cpp
+++ b/src/check_expr.cpp
@@ -366,6 +366,9 @@ gb_internal void check_scope_decls(CheckerContext *c, Slice<Ast *> const &nodes,
 
 	check_collect_entities(c, nodes);
 
+	// NOTE: checking a declaration can insert entities into this very scope - a procedure alias
+	// goes through `override_entity_in_scope`, which inserts into the scope being iterated - so
+	// the iteration must not be invalidated by `s->elements` growing; see ScopeMapIterator.
 	for (auto const &entry : s->elements) {
 		Entity *e = entry.value;\
 		switch (e->kind) {
diff --git a/src/checker.hpp b/src/checker.hpp
index 89b6318d7..81bcc8e25 100644
--- a/src/checker.hpp
+++ b/src/checker.hpp
@@ -465,17 +465,27 @@ gb_internal void scope_map_clear(ScopeMap *m) {
 	m->count = 0;
 }
 
+// IMPORTANT NOTE: an iteration snapshots the table pointer and capacity when it is constructed,
+// so that inserting into the map while iterating it cannot invalidate the iteration.
+// `scope_map_insert` may grow the map - rehashing into a newly allocated table - at any time, and
+// `begin`/`end` are both created before the loop body runs: without the snapshot, `end` would
+// keep the capacity that `begin` saw while `operator++`/`operator*` read the reallocated table,
+// walking past the end of it. An iteration therefore walks the table it began on: a slot written
+// in place (an insert that does not rehash) is seen when the walk reaches it, while the entries
+// written into a table that a rehash replaced are not.
 struct ScopeMapIterator {
 	ScopeMap const *map;
-	u32 index;
+	ScopeMapSlot *  slots;
+	u32             cap;
+	u32             index;
 
 	ScopeMapIterator &operator++() noexcept {
 		for (;;) {
 			++index;
-			if (map->cap == index) {
+			if (cap == index) {
 				return *this;
 			}
-			ScopeMapSlot *s = map->slots+index;
+			ScopeMapSlot *s = slots+index;
 			if (s->hash) {
 				return *this;
 			}
@@ -483,20 +493,20 @@ struct ScopeMapIterator {
 	}
 
 	bool operator==(ScopeMapIterator const &other) const noexcept {
-		return this->map == other.map && this->index == other.index;
+		return this->map == other.map && this->slots == other.slots && this->index == other.index;
 	}
 
 	operator ScopeMapSlot *() const {
-		return map->slots+index;
+		return slots+index;
 	}
 };
 
 
 gb_internal ScopeMapIterator end(ScopeMap &m) noexcept {
-	return ScopeMapIterator{&m, m.cap};
+	return ScopeMapIterator{&m, m.slots, m.cap, m.cap};
 }
 gb_internal ScopeMapIterator const end(ScopeMap const &m) noexcept {
-	return ScopeMapIterator{&m, m.cap};
+	return ScopeMapIterator{&m, m.slots, m.cap, m.cap};
 }
 gb_internal ScopeMapIterator begin(ScopeMap &m) noexcept {
 	if (m.count == 0) {
@@ -510,7 +520,7 @@ gb_internal ScopeMapIterator begin(ScopeMap &m) noexcept {
 		}
 		index++;
 	}
-	return ScopeMapIterator{&m, index};
+	return ScopeMapIterator{&m, m.slots, m.cap, index};
 }
 gb_internal ScopeMapIterator const begin(ScopeMap const &m) noexcept {
 	if (m.count == 0) {
@@ -524,7 +534,7 @@ gb_internal ScopeMapIterator const begin(ScopeMap const &m) noexcept {
 		}
 		index++;
 	}
-	return ScopeMapIterator{&m, index};
+	return ScopeMapIterator{&m, m.slots, m.cap, index};
 }
 
 enum ScopeFlag : i32 {
diff --git a/tests/issues/run.sh b/tests/issues/run.sh
index 565528ed8..7cd74fcda 100755
--- a/tests/issues/run.sh
+++ b/tests/issues/run.sh
@@ -155,6 +155,15 @@ else
 	exit 1
 fi
 
+$ODIN test ../test_issue_7598.odin $COMMON
+
+if [[ $($ODIN build ../test_issue_7598_all_entities_checked.odin $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 4 ]]; then
+	echo "SUCCESSFUL 1/1"
+else
+	echo "SUCCESSFUL 0/1"
+	exit 1
+fi
+
 clang -c ../test_issue_7010.c -o test_issue_7010_c.o
 $ODIN test ../test_issue_7010.odin $COMMON
 
diff --git a/tests/issues/test_issue_7598.odin b/tests/issues/test_issue_7598.odin
new file mode 100644
index 000000000..a57c9ae37
--- /dev/null
+++ b/tests/issues/test_issue_7598.odin
@@ -0,0 +1,307 @@
+// Tests issue #7598: the compiler crashed with a null pointer dereference when a procedure
+// scope held exactly 12 (and, at the next load factor, exactly 24) entities.
+// https://github.com/odin-lang/Odin/issues/7598
+//
+// `check_scope_decls` iterates the scope's entity map while `check_entity_decl` can insert an
+// entity into that very scope: a procedure alias such as `f :: e` goes through
+// `override_entity_in_scope`, which inserts into the scope being iterated. The scope map is
+// grown at 75% load (12 of 16 slots, then 24 of 32), so the insert rehashes and reallocates the
+// table *inside* the loop. The range-for's end sentinel had captured the old capacity, so the
+// iteration kept walking the new table past the slot that sentinel named, off the end of the
+// table, dereferencing whatever followed it as an `Entity *`.
+//
+// Both sides of the count are exact, which is what made one declaration more or less compile:
+// one entity fewer and the alias insert does not reach the load factor, one entity more and the
+// map has already grown while collecting declarations, before the loop starts.
+package test_issues
+
+import "core:testing"
+
+
+// The reported program: 12 entities in one procedure scope, three of them procedure aliases.
+// The scope map sits at 75% load (12/16), so the alias insert inside the entity loop rehashes it.
+issue_repro :: proc() -> int {
+	n :: 11
+	a :: proc() -> int { return 1 }
+	b :: proc() -> int { return 2 }
+	C :: bit_set[1..=9]
+	d :: proc() -> int { return 4 }
+	e :: proc() -> int { return 5 }
+	f :: e
+	g :: e
+	h :: e
+	i :: proc() -> int { return 9 }
+	j :: proc() -> int { return 10 }
+	k :: proc() -> int { return 11 }
+
+	std :: size_of(C)
+	return n + a() + b() + std + d() + e() + f() + g() + h() + i() + j() + k()
+}
+
+// 11 entities: one below the 12/16 load boundary, the map never grows inside the loop.
+boundary_11 :: proc() -> int {
+	n :: 11
+	b11_1 :: proc() -> int { return 1 }
+	b11_2 :: b11_1
+	b11_3 :: b11_1
+	b11_4 :: proc() -> int { return 4 }
+	b11_5 :: proc() -> int { return 5 }
+	b11_6 :: proc() -> int { return 6 }
+	b11_7 :: proc() -> int { return 7 }
+	b11_8 :: proc() -> int { return 8 }
+	b11_9 :: proc() -> int { return 9 }
+	b11_10 :: proc() -> int { return 10 }
+
+	return n + b11_1() + b11_2() + b11_3() + b11_4() + b11_5() + b11_6() + b11_7() + b11_8() + b11_9() + b11_10()
+}
+
+// 12 entities again, with names that rehash into different slots: a second layout at the boundary.
+boundary_12_alt :: proc() -> int {
+	n :: 11
+	c12_1 :: proc() -> int { return 1 }
+	c12_2 :: proc() -> int { return 2 }
+	c12_3 :: proc() -> int { return 3 }
+	c12_4 :: c12_1
+	c12_5 :: c12_1
+	c12_6 :: proc() -> int { return 6 }
+	c12_7 :: proc() -> int { return 7 }
+	c12_8 :: proc() -> int { return 8 }
+	c12_9 :: proc() -> int { return 9 }
+	c12_10 :: proc() -> int { return 10 }
+	c12_11 :: proc() -> int { return 11 }
+
+	return n + c12_1() + c12_2() + c12_3() + c12_4() + c12_5() + c12_6() + c12_7() + c12_8() + c12_9() + c12_10() + c12_11()
+}
+
+// 12 entities, third layout, with the aliases visited later in the iteration.
+boundary_12_alt2 :: proc() -> int {
+	n :: 11
+	d12_1 :: proc() -> int { return 1 }
+	d12_2 :: proc() -> int { return 2 }
+	d12_3 :: proc() -> int { return 3 }
+	d12_4 :: proc() -> int { return 4 }
+	d12_5 :: proc() -> int { return 5 }
+	d12_6 :: d12_1
+	d12_7 :: d12_1
+	d12_8 :: d12_1
+	d12_9 :: proc() -> int { return 9 }
+	d12_10 :: proc() -> int { return 10 }
+	d12_11 :: proc() -> int { return 11 }
+
+	return n + d12_1() + d12_2() + d12_3() + d12_4() + d12_5() + d12_6() + d12_7() + d12_8() + d12_9() + d12_10() + d12_11()
+}
+
+// 13 entities: the map has already grown while collecting, so it does not grow inside the loop.
+boundary_13 :: proc() -> int {
+	n :: 11
+	e13_1 :: proc() -> int { return 1 }
+	e13_2 :: e13_1
+	e13_3 :: e13_1
+	e13_4 :: proc() -> int { return 4 }
+	e13_5 :: proc() -> int { return 5 }
+	e13_6 :: proc() -> int { return 6 }
+	e13_7 :: proc() -> int { return 7 }
+	e13_8 :: proc() -> int { return 8 }
+	e13_9 :: proc() -> int { return 9 }
+	e13_10 :: proc() -> int { return 10 }
+	e13_11 :: proc() -> int { return 11 }
+	e13_12 :: proc() -> int { return 12 }
+
+	return n + e13_1() + e13_2() + e13_3() + e13_4() + e13_5() + e13_6() + e13_7() + e13_8() + e13_9() + e13_10() + e13_11() + e13_12()
+}
+
+// 23 entities: the second grow point (24/32) is not reached.
+boundary_23 :: proc() -> int {
+	n :: 11
+	f23_1 :: proc() -> int { return 1 }
+	f23_2 :: f23_1
+	f23_3 :: f23_1
+	f23_4 :: proc() -> int { return 4 }
+	f23_5 :: proc() -> int { return 5 }
+	f23_6 :: proc() -> int { return 6 }
+	f23_7 :: proc() -> int { return 7 }
+	f23_8 :: proc() -> int { return 8 }
+	f23_9 :: proc() -> int { return 9 }
+	f23_10 :: proc() -> int { return 10 }
+	f23_11 :: proc() -> int { return 11 }
+	f23_12 :: proc() -> int { return 12 }
+	f23_13 :: proc() -> int { return 13 }
+	f23_14 :: proc() -> int { return 14 }
+	f23_15 :: proc() -> int { return 15 }
+	f23_16 :: proc() -> int { return 16 }
+	f23_17 :: proc() -> int { return 17 }
+	f23_18 :: proc() -> int { return 18 }
+	f23_19 :: proc() -> int { return 19 }
+	f23_20 :: proc() -> int { return 20 }
+	f23_21 :: proc() -> int { return 21 }
+	f23_22 :: proc() -> int { return 22 }
+
+	return n + f23_1() + f23_2() + f23_3() + f23_4() + f23_5() + f23_6() + f23_7() + f23_8() + f23_9() + f23_10() + f23_11() + f23_12() + f23_13() + f23_14() + f23_15() + f23_16() + f23_17() + f23_18() + f23_19() + f23_20() + f23_21() + f23_22()
+}
+
+// 24 entities: the scope map is at 75% load again (24/32) and grows 32 -> 64 inside the loop.
+boundary_24 :: proc() -> int {
+	n :: 11
+	g24_1 :: proc() -> int { return 1 }
+	g24_2 :: g24_1
+	g24_3 :: g24_1
+	g24_4 :: proc() -> int { return 4 }
+	g24_5 :: proc() -> int { return 5 }
+	g24_6 :: proc() -> int { return 6 }
+	g24_7 :: proc() -> int { return 7 }
+	g24_8 :: proc() -> int { return 8 }
+	g24_9 :: proc() -> int { return 9 }
+	g24_10 :: proc() -> int { return 10 }
+	g24_11 :: proc() -> int { return 11 }
+	g24_12 :: proc() -> int { return 12 }
+	g24_13 :: proc() -> int { return 13 }
+	g24_14 :: proc() -> int { return 14 }
+	g24_15 :: proc() -> int { return 15 }
+	g24_16 :: proc() -> int { return 16 }
+	g24_17 :: proc() -> int { return 17 }
+	g24_18 :: proc() -> int { return 18 }
+	g24_19 :: proc() -> int { return 19 }
+	g24_20 :: proc() -> int { return 20 }
+	g24_21 :: proc() -> int { return 21 }
+	g24_22 :: proc() -> int { return 22 }
+	g24_23 :: proc() -> int { return 23 }
+
+	return n + g24_1() + g24_2() + g24_3() + g24_4() + g24_5() + g24_6() + g24_7() + g24_8() + g24_9() + g24_10() + g24_11() + g24_12() + g24_13() + g24_14() + g24_15() + g24_16() + g24_17() + g24_18() + g24_19() + g24_20() + g24_21() + g24_22() + g24_23()
+}
+
+// 24 entities, second layout at the second grow point.
+boundary_24_alt :: proc() -> int {
+	n :: 11
+	h24_1 :: proc() -> int { return 1 }
+	h24_2 :: proc() -> int { return 2 }
+	h24_3 :: proc() -> int { return 3 }
+	h24_4 :: proc() -> int { return 4 }
+	h24_5 :: proc() -> int { return 5 }
+	h24_6 :: proc() -> int { return 6 }
+	h24_7 :: h24_1
+	h24_8 :: h24_1
+	h24_9 :: proc() -> int { return 9 }
+	h24_10 :: proc() -> int { return 10 }
+	h24_11 :: proc() -> int { return 11 }
+	h24_12 :: proc() -> int { return 12 }
+	h24_13 :: proc() -> int { return 13 }
+	h24_14 :: proc() -> int { return 14 }
+	h24_15 :: proc() -> int { return 15 }
+	h24_16 :: proc() -> int { return 16 }
+	h24_17 :: proc() -> int { return 17 }
+	h24_18 :: proc() -> int { return 18 }
+	h24_19 :: proc() -> int { return 19 }
+	h24_20 :: proc() -> int { return 20 }
+	h24_21 :: proc() -> int { return 21 }
+	h24_22 :: proc() -> int { return 22 }
+	h24_23 :: proc() -> int { return 23 }
+
+	return n + h24_1() + h24_2() + h24_3() + h24_4() + h24_5() + h24_6() + h24_7() + h24_8() + h24_9() + h24_10() + h24_11() + h24_12() + h24_13() + h24_14() + h24_15() + h24_16() + h24_17() + h24_18() + h24_19() + h24_20() + h24_21() + h24_22() + h24_23()
+}
+
+// 25 entities: the map grew while collecting, so it does not grow inside the loop.
+boundary_25 :: proc() -> int {
+	n :: 11
+	i25_1 :: proc() -> int { return 1 }
+	i25_2 :: i25_1
+	i25_3 :: i25_1
+	i25_4 :: proc() -> int { return 4 }
+	i25_5 :: proc() -> int { return 5 }
+	i25_6 :: proc() -> int { return 6 }
+	i25_7 :: proc() -> int { return 7 }
+	i25_8 :: proc() -> int { return 8 }
+	i25_9 :: proc() -> int { return 9 }
+	i25_10 :: proc() -> int { return 10 }
+	i25_11 :: proc() -> int { return 11 }
+	i25_12 :: proc() -> int { return 12 }
+	i25_13 :: proc() -> int { return 13 }
+	i25_14 :: proc() -> int { return 14 }
+	i25_15 :: proc() -> int { return 15 }
+	i25_16 :: proc() -> int { return 16 }
+	i25_17 :: proc() -> int { return 17 }
+	i25_18 :: proc() -> int { return 18 }
+	i25_19 :: proc() -> int { return 19 }
+	i25_20 :: proc() -> int { return 20 }
+	i25_21 :: proc() -> int { return 21 }
+	i25_22 :: proc() -> int { return 22 }
+	i25_23 :: proc() -> int { return 23 }
+	i25_24 :: proc() -> int { return 24 }
+
+	return n + i25_1() + i25_2() + i25_3() + i25_4() + i25_5() + i25_6() + i25_7() + i25_8() + i25_9() + i25_10() + i25_11() + i25_12() + i25_13() + i25_14() + i25_15() + i25_16() + i25_17() + i25_18() + i25_19() + i25_20() + i25_21() + i25_22() + i25_23() + i25_24()
+}
+
+// The same boundary in a nested block scope: the declaration-checking flag reaches nested blocks
+// as well, so the block's own scope is collected and iterated exactly like a procedure body's.
+boundary_12_block :: proc() -> int {
+	total := 0
+	{
+		n :: 11
+		blk_1 :: proc() -> int { return 1 }
+		blk_2 :: blk_1
+		blk_3 :: blk_1
+		blk_4 :: proc() -> int { return 4 }
+		blk_5 :: proc() -> int { return 5 }
+		blk_6 :: proc() -> int { return 6 }
+		blk_7 :: proc() -> int { return 7 }
+		blk_8 :: proc() -> int { return 8 }
+		blk_9 :: proc() -> int { return 9 }
+		blk_10 :: proc() -> int { return 10 }
+		blk_11 :: proc() -> int { return 11 }
+
+		total = n + blk_1() + blk_2() + blk_3() + blk_4() + blk_5() + blk_6() + blk_7() + blk_8() + blk_9() + blk_10() + blk_11()
+	}
+	return total
+}
+
+// Every case is called and its result checked: an entity whose declaration the entity loop
+// dropped has to fail here, and a compiler that cannot check the scope has to fail before it
+// gets this far.
+@(test)
+test_issue_7598_issue_repro :: proc(t: ^testing.T) {
+	testing.expect_value(t, issue_repro(), 70)
+}
+
+@(test)
+test_issue_7598_boundary_11 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_11(), 63)
+}
+
+@(test)
+test_issue_7598_boundary_12_alt :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_12_alt(), 70)
+}
+
+@(test)
+test_issue_7598_boundary_12_alt2 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_12_alt2(), 59)
+}
+
+@(test)
+test_issue_7598_boundary_12_block :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_12_block(), 74)
+}
+
+@(test)
+test_issue_7598_boundary_13 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_13(), 86)
+}
+
+@(test)
+test_issue_7598_boundary_23 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_23(), 261)
+}
+
+@(test)
+test_issue_7598_boundary_24 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_24(), 284)
+}
+
+@(test)
+test_issue_7598_boundary_24_alt :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_24_alt(), 274)
+}
+
+@(test)
+test_issue_7598_boundary_25 :: proc(t: ^testing.T) {
+	testing.expect_value(t, boundary_25(), 308)
+}
\ No newline at end of file
diff --git a/tests/issues/test_issue_7598_all_entities_checked.odin b/tests/issues/test_issue_7598_all_entities_checked.odin
new file mode 100644
index 000000000..3b043b76f
--- /dev/null
+++ b/tests/issues/test_issue_7598_all_entities_checked.odin
@@ -0,0 +1,55 @@
+// Companion to test_issue_7598.odin, covering the other half of issue #7598.
+// https://github.com/odin-lang/Odin/issues/7598
+//
+// When the scope map is rehashed from inside `check_scope_decls`' iteration, the iteration
+// either runs off the end of the reallocated table (the crash in test_issue_7598.odin) or it
+// stops at the slot the stale end sentinel named and silently drops every entity whose new slot
+// lies behind it. A dropped entity is never checked, so an invalid declaration simply produces
+// no diagnostic at all.
+//
+// Every invalid declaration here must be reported: 2 declarations x 2 procedure scopes = 4
+// errors. `boundary_errors_13` holds 13 entities, one above the 12/16 load boundary, so its map
+// has already grown by the time the loop starts and it reports its own 2 errors even without the
+// fix; `boundary_errors_12` holds exactly 12 entities and its map grows *inside* the loop, so
+// without the fix it swallows one of its 2.
+package test_issues
+
+boundary_errors_13 :: proc() -> int {
+	n :: 11
+	a0 :: proc() -> int { return 1 }
+	b1 :: a0
+	c2 :: a0
+	d3 :: proc() -> int { return 4 }
+	e4 :: undeclared_name_5
+	f5 :: proc() -> int { return 6 }
+	g6 :: proc() -> int { return 7 }
+	h7 :: undeclared_name_8
+	i8 :: proc() -> int { return 9 }
+	j9 :: proc() -> int { return 10 }
+	k10 :: proc() -> int { return 11 }
+	l11 :: proc() -> int { return 12 }
+
+	return n + a0() + b1() + c2() + d3() + f5() + g6() + i8() + j9() + k10() + l11()
+}
+
+boundary_errors_12 :: proc() -> int {
+	n :: 11
+	a0 :: proc() -> int { return 1 }
+	b1 :: a0
+	c2 :: a0
+	d3 :: proc() -> int { return 4 }
+	e4 :: undeclared_name_5
+	f5 :: proc() -> int { return 6 }
+	g6 :: proc() -> int { return 7 }
+	h7 :: undeclared_name_8
+	i8 :: proc() -> int { return 9 }
+	j9 :: proc() -> int { return 10 }
+	k10 :: proc() -> int { return 11 }
+
+	return n + a0() + b1() + c2() + d3() + f5() + g6() + i8() + j9() + k10()
+}
+
+main :: proc() {
+	_ = boundary_errors_13()
+	_ = boundary_errors_12()
+}
\ No newline at end of file