diff --git a/CHANGELOG.md b/CHANGELOG.md index 3446a6d3..40ac260a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,25 @@ version number before tagging the release. ## [current] +### Fixed + +- **A closure that returned a string on one path and a `-> ptr` builtin's + result on another handed a string literal to a caller that freed it.** + `resolve_closure_return_type` typed a closure by its FIRST return site. + `string.from_int(x)` is declared `-> ptr` (an AetherString behind a raw + pointer), so a closure returning it on one path and a struct's string + field on another was emitted `void*`, was not a string closure, and none + of its returns got the uniform-heap wrap of #2054 — while its caller, a + `-> string` function returning `cb(...)`, took ownership of the result and + freed it. The field path returned a literal; the caller `free()`d it: + STATUS_HEAP_CORRUPTION on Windows, SIGABRT on macOS, silently absorbed by + glibc. aether-ui's table cell callback has exactly that shape, so every + table in that tree aborted at startup from 0.682 on. Any string-typed + return site now makes the closure `const char*`, so every path is + wrapped: the literal is copied out, the AetherString is copied out by its + header, and the caller owns both. `tests/regression/ + test_closure_mixed_string_returns.ae` drives both sites fifty times. + ## [0.683.0] ### Fixed diff --git a/compiler/codegen/codegen_expr.c b/compiler/codegen/codegen_expr.c index f8128f54..45ed741b 100644 --- a/compiler/codegen/codegen_expr.c +++ b/compiler/codegen/codegen_expr.c @@ -815,6 +815,35 @@ static ASTNode* find_first_return_expr(ASTNode* node) { return NULL; } +/* Does ANY return site under `node` (not descending into nested closures) + * carry a `string`-typed expression? + * + * resolve_closure_return_type picks the closure's C return type from its + * FIRST return, which is the right type for the signature only when every + * return agrees. A closure that returns `string.from_int(x)` on one path and + * a struct's string field on another is typed by the first: `void*` (the + * builtin is declared `-> ptr`), so it is not a "string closure" and none of + * its returns get the uniform-heap wrap of #2054 — while its caller, a + * `-> string` function returning `cb(...)`, takes ownership of whatever + * comes back and frees it. The field-returning path handed over a literal, + * and the caller free()d it (aether-ui's table cell callback, every row; + * a heap-corruption abort on macOS and Windows). Any string-typed return + * makes the closure a string closure, so every path is wrapped. */ +static int any_return_is_string(ASTNode* node) { + if (!node) return 0; + if (node->type == AST_CLOSURE) return 0; + if (node->type == AST_RETURN_STATEMENT && node->child_count > 0 && + node->children[0] && node->children[0]->type != AST_PRINT_STATEMENT) { + ASTNode* e = node->children[0]; + if (e->node_type && e->node_type->kind == TYPE_STRING) return 1; + return 0; + } + for (int i = 0; i < node->child_count; i++) { + if (any_return_is_string(node->children[i])) return 1; + } + return 0; +} + // Return 1 if any AST_VARIABLE_DECLARATION node under `node` assigns to // `name` (i.e., appears as its `value`). Used by closure codegen to detect // which captures are mutated inside the body — those captures cannot use @@ -1689,6 +1718,15 @@ static const char* resolve_closure_return_type(CodeGenerator* gen, int ci) { } } } + /* Mixed returns: one string-typed return site makes this a string + * closure whatever the first site was typed (see any_return_is_string). + * A `void*` first site is an AetherString from a `-> ptr` builtin, which + * the uniform-heap wrap copies out by its header, so `const char*` is + * correct for both. */ + if (strcmp(ret_type, "const char*") != 0 && body_check && + any_return_is_string(body_check)) { + ret_type = "const char*"; + } return ret_type; } diff --git a/tests/regression/test_closure_mixed_string_returns.ae b/tests/regression/test_closure_mixed_string_returns.ae new file mode 100644 index 00000000..9bbd5437 --- /dev/null +++ b/tests/regression/test_closure_mixed_string_returns.ae @@ -0,0 +1,71 @@ +// Regression: a closure with one string-typed return site and one +// `-> ptr` builtin return site is a string closure on every path. +// +// The bug: resolve_closure_return_type typed a closure by its FIRST +// return. `string.from_int(x)` is declared `-> ptr` (an AetherString +// behind a raw pointer), so a closure that returned it on one path and +// a struct's string field on another was emitted `void*`, was not a +// "string closure", and none of its returns got the uniform-heap wrap +// of #2054. Its caller — a `-> string` function that returns `cb(...)` +// — still took ownership of whatever came back and freed it. The +// field path handed over a string literal, and the caller free()d it: +// STATUS_HEAP_CORRUPTION on Windows, SIGABRT on macOS, glibc quietly +// absorbed it. This was aether-ui's table cell callback, on every row +// of every table, the moment that tree moved past 0.681. +// +// Pre-fix this crashed in the first `invoke`. Post-fix any string-typed +// return site makes the closure `const char*`, so the literal is copied +// on the way out and the AetherString is copied out by its header; the +// caller owns both and frees both. + +import std.string + +extern exit(code: int) +extern malloc(size: int) -> ptr + +struct Person { name: string, kb: int } + +// The shape of aether-ui's `_table_invoke_cell`: a `-> string` function +// whose whole body is a call through a bare fn value. +invoke(cb: fn, item: ptr, c: int) -> string { + return cb(item, c) +} + +fail(msg: string) { + println("FAIL: ${msg}") + exit(1) +} + +main() { + print("=== closure with mixed string / ptr-builtin returns ===\n\n") + + p = malloc(sizeof(Person)) as *Person + p.name = "zoe" + p.kb = 30 + + // First return site is the `-> ptr` builtin; the second is a string + // field that holds a literal. Pre-fix: typed void* by the first site, + // the literal reached the caller unwrapped and was freed. + cell = | item: ptr, c: int | { + q = item as *Person + if c == 1 { return string.from_int(q.kb) } + return q.name + } + + i = 0 + while i < 50 { + name = invoke(cell, p as ptr, 0) + if string.equals(name, "zoe") != 1 { fail("column 0 -> '${name}', want 'zoe'") } + size = invoke(cell, p as ptr, 1) + if string.equals(size, "30") != 1 { fail("column 1 -> '${size}', want '30'") } + i = i + 1 + } + print(" PASS: 50 rounds through both return sites, no corruption\n") + + // The literal itself is untouched: the struct still reads it after + // every copy the caller freed. + if string.equals(p.name, "zoe") != 1 { fail("the field's literal was damaged: '${p.name}'") } + print(" PASS: the field's literal survived\n") + + print("\n=== mixed-return closure test passed ===\n") +}