Fix a double-free: a heap-string field taken from a borrowed local was freed

CHANGELOG.md

@@ -11,6 +11,36 @@ version number before tagging the release.
 
 ## [current]
 
+### Fixed
+
+- **A heap-string struct field assigned from a borrowed value was freed at
+  teardown, a double free / free of rodata.** When a heap-string field is
+  assigned from a bare heap-tracked local (`a.value = v`), the field's
+  `_heap_<field>` tracker was hard-coded to 1. But a local classified as a
+  heap-string var can hold a *borrowed* value at runtime, e.g. one returned by
+  a function that passes a parameter or a literal straight through, leaving its
+  `_heap_<var>` at 0. The struct destructor then freed a pointer the program
+  never owned: a literal (free of read-only data) or a value still owned
+  elsewhere. It surfaced downstream as a callback/FFI crash: a hook returns a
+  literal `string`, the engine threads it through `-> string` helpers into a
+  heap-boxed struct field, and teardown aborted with `invalid pointer` /
+  SIGSEGV. The field store now *moves* the source var's runtime ownership
+  (`_heap_<field> = _heap_<var>; _heap_<var> = 0`) instead of asserting 1, so a
+  borrowed value is not freed and a genuinely-owned one is still freed exactly
+  once. Regression follow-up to #1866/#1879.
+
+  Reading the tracker only works if the tracker is maintained, and for one
+  shape it was not. When a heap-string var's value escapes into a container or
+  a struct field, its own frees are suppressed (the recipient may have kept the
+  pointer), and on that path the assignment left `_heap_<var>` at its stale 0
+  on the reasoning that nothing would read it. The field store now reads it, so
+  a genuinely owned value (`s = strbuilder.finish(b)` then `n.text = s`) moved
+  a 0 into the field tracker and the destructor never reclaimed the buffer.
+  `std.message` leaked 60 allocations exactly this way, caught by the macOS
+  leaks gate. The flag is the ownership token, so it is now recorded whether or
+  not this scope is the one that acts on it; the escape-suppressed frees are
+  unchanged.
+
 ## [0.648.0]
 
 ### Fixed

tests/integration/heap_field_borrowed_var_no_double_free/prog.ae

@@ -0,0 +1,83 @@
+// A heap-string struct field assigned from a heap-tracked LOCAL that at
+// runtime holds a BORROWED value must not be freed by the struct destructor.
+//
+// `passthru` returns its parameter straight through, so it is classified
+// non-heap: `borrowed` is a heap-string var (it is reassignable to owned
+// strings elsewhere in a real program) but its runtime `_heap_borrowed` is 0.
+// Before the fix, `a.value = borrowed` hard-coded `_heap_value = 1`, so
+// `heap.free(a)` freed a value the program never owned — a literal string
+// (free of rodata) — aborting with SIGSEGV / "invalid pointer" at teardown.
+// The store must instead MOVE the source's runtime ownership:
+//   a->value = borrowed; a->_heap_value = _heap_borrowed; _heap_borrowed = 0;
+//
+// This is the FFI/closure-callback shape a downstream sanitizer hit: a hook
+// returns a literal, the engine threads it through `-> string` helpers into a
+// heap-boxed attr's string field, and teardown double-freed it.
+//
+// The `owned` half proves the fix does not regress the leak it protects:
+// a genuinely-heap value is still freed exactly once.
+//
+// `owned` alone is not enough, and the reason is worth stating: the compiler
+// constant-folds `string.concat("https://", "cdn.example/style.css")` into a
+// literal, so that half runs with `_heap_owned == 0` and exercises the
+// borrowed path a second time rather than the owned one. `runtime_owned` is
+// built through a call the folder cannot see through, so it is genuinely heap
+// at runtime -- and it is the shape that leaked: for a var whose value escapes
+// into a struct field, the assignment used to leave `_heap_<var>` at its stale
+// 0, and moving that 0 into the field tracker told the destructor the buffer
+// was borrowed. std.message leaked 60 allocations exactly this way.
+
+import std.io
+import std.string
+import std.strbuilder
+
+struct Attr {
+    name: string
+    value: string
+}
+
+fn passthru(u: string) -> string {
+    return u
+}
+
+// Built through a call, so the constant folder cannot reduce it to a literal:
+// the result is genuinely heap-allocated at runtime.
+fn build_owned(n: int) -> string {
+    b = strbuilder.new(16)
+    strbuilder.append(b, "https://cdn.example/")
+    strbuilder.append_int(b, n)
+    return strbuilder.finish(b)
+}
+
+main() {
+    // Borrowed: value the program does not own. Must NOT be freed at teardown.
+    a = heap.new(Attr)
+    a.name = "src"
+    borrowed = passthru("https://cdn.example/logo.png")
+    ap = a as *Attr
+    ap.value = borrowed
+    println("borrowed: ${a.value}")
+    heap.free(a)
+
+    // Owned: genuinely-heap value. Must be freed exactly once (no leak).
+    b = heap.new(Attr)
+    b.name = "href"
+    owned = string.concat("https://", "cdn.example/style.css")
+    bp = b as *Attr
+    bp.value = owned
+    println("owned: ${b.value}")
+    heap.free(b)
+
+    // Genuinely heap at runtime, and escaping into a struct field. The field
+    // tracker must receive 1 so the destructor reclaims it exactly once:
+    // freeing nothing here is the leak, freeing twice is the crash above.
+    c = heap.new(Attr)
+    c.name = "src"
+    runtime_owned = build_owned(7)
+    cp = c as *Attr
+    cp.value = runtime_owned
+    println("runtime_owned: ${c.value}")
+    heap.free(c)
+
+    println("done")
+}

tests/integration/heap_field_borrowed_var_no_double_free/test_heap_field_borrowed_var_no_double_free.sh

@@ -0,0 +1,94 @@
+#!/bin/sh
+# A heap-string struct field assigned from a heap-tracked local that holds a
+# BORROWED value at runtime must move the source's runtime ownership into the
+# field tracker, not hard-code it to 1. Hard-coding 1 made the destructor free
+# a value the program never owned (a literal -> free of rodata), aborting at
+# teardown. This is the closure/FFI-callback shape a downstream HTML sanitizer
+# hit: a hook returns a literal, threaded through `-> string` helpers into a
+# heap-boxed attr field, double-freed on free.
+#
+# Running to completion is half the assertion (a double free / rodata free
+# aborts with SIGSEGV or SIGABRT). The emitted ownership move is checked
+# directly too, so this means something on CI platforms with no leak checker.
+
+set -e
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)"
+AE="$ROOT/build/ae"
+AETHERC="$ROOT/build/aetherc"
+[ -x "$AE" ] || { echo "  [SKIP] heap_field_borrowed_var_no_double_free: build/ae missing"; exit 0; }
+
+TMP="$(mktemp -d)"
+cleanup() { rm -rf "$TMP"; }
+trap cleanup EXIT
+
+# 1. Generated C moves the source var's runtime tracker into the field, rather
+#    than writing a literal 1. The borrowed store must NOT read `= 1`.
+if ! "$AETHERC" "$SCRIPT_DIR/prog.ae" "$TMP/out.c" > "$TMP/gen.log" 2>&1; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: codegen failed"
+    sed 's/^/        /' "$TMP/gen.log" | head -8
+    exit 1
+fi
+if ! grep -q "_heap_value = _heap_borrowed" "$TMP/out.c"; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: borrowed store did not move the runtime tracker"
+    echo "         expected '..._heap_value = _heap_borrowed; _heap_borrowed = 0;'"
+    grep -n "value = borrowed\|_heap_value" "$TMP/out.c" | sed 's/^/        /' | head -6
+    exit 1
+fi
+if grep -q "value = borrowed; ap->_heap_value = 1" "$TMP/out.c"; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: borrowed store still hard-codes _heap_value = 1"
+    exit 1
+fi
+
+# 1b. The mirror image: a var holding a GENUINELY owned value must reach the
+#     field tracker as 1, or the destructor frees nothing and the buffer leaks.
+#     For a var whose value escapes into a struct field the assignment used to
+#     leave `_heap_<var>` at its stale 0, so the move carried a 0 and the leak
+#     was silent on every platform without a leak checker. Asserted on the
+#     generated C so it fails on those platforms too.
+if ! grep -q "runtime_owned = build_owned(7); _heap_runtime_owned = 1" "$TMP/out.c"; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: an owned value that escapes"
+    echo "         into a field left its tracker stale, so the field tracker takes 0"
+    echo "         and the destructor never frees it"
+    grep -n "runtime_owned" "$TMP/out.c" | sed 's/^/        /' | head -6
+    exit 1
+fi
+if ! grep -q "_heap_value = _heap_runtime_owned" "$TMP/out.c"; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: owned store did not move the runtime tracker"
+    grep -n "value = runtime_owned\|_heap_value" "$TMP/out.c" | sed 's/^/        /' | head -6
+    exit 1
+fi
+
+# 2. It runs to completion — the actual bug was a teardown abort. Build a real
+#    binary and run it; a double free / rodata free crashes here.
+if ! "$AE" build "$SCRIPT_DIR/prog.ae" -o "$TMP/prog" > "$TMP/build.log" 2>&1; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: build failed"
+    sed 's/^/        /' "$TMP/build.log" | head -8
+    exit 1
+fi
+if ! "$TMP/prog" > "$TMP/run.out" 2>&1; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: program aborted at runtime (rc $?)"
+    sed 's/^/        /' "$TMP/run.out" | head -8
+    exit 1
+fi
+if ! grep -q "^done$" "$TMP/run.out"; then
+    echo "  [FAIL] heap_field_borrowed_var_no_double_free: did not reach 'done'"
+    sed 's/^/        /' "$TMP/run.out" | head -8
+    exit 1
+fi
+
+# 3. Freed exactly once, where a leak checker is available. leaks(1) is the one
+#    that works on macOS; Linux CI covers the same program under Valgrind.
+if [ "$(uname -s)" = Darwin ] && command -v leaks >/dev/null 2>&1; then
+    if ! MallocStackLogging=1 leaks --atExit -- "$TMP/prog" > "$TMP/leaks.out" 2>&1; then
+        if grep -qE "[1-9][0-9]* leak(s)? for" "$TMP/leaks.out"; then
+            echo "  [FAIL] heap_field_borrowed_var_no_double_free: the owned value leaked"
+            grep -E "leaks? for" "$TMP/leaks.out" | sed 's/^/        /' | head -3
+            exit 1
+        fi
+    fi
+fi
+
+echo "  [PASS] heap_field_borrowed_var_no_double_free: borrowed field-store moves runtime ownership; no teardown double-free"
+exit 0