From 147a7a0f607aa6e98c242091f0555bfbd957b5e4 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Mon, 12 Oct 2026 13:51:27 -0400 Subject: [PATCH] Fix #139 fetch arity panic; empty min/max VALUE_ERROR; Go/JS targets reject core-library sugar and nz --- .../2026-10-12_issue139_corelib_hardening.md | 91 +++++++++++++ .../token-efficiency-heldout-v1/REPORT.md | 11 +- howlframe.go | 32 +++++ internal/ast/desugar_corelib.go | 15 ++- internal/bytecode/bytecode.go | 7 + issue139_corelib_hardening_cli_test.go | 126 ++++++++++++++++++ 6 files changed, 277 insertions(+), 5 deletions(-) create mode 100644 docs/journals/2026-10-12_issue139_corelib_hardening.md create mode 100644 issue139_corelib_hardening_cli_test.go diff --git a/docs/journals/2026-10-12_issue139_corelib_hardening.md b/docs/journals/2026-10-12_issue139_corelib_hardening.md new file mode 100644 index 00000000..11b32ce1 --- /dev/null +++ b/docs/journals/2026-10-12_issue139_corelib_hardening.md @@ -0,0 +1,91 @@ +# 2026-10-12: #139 fix and core-library hardening + +Base: main `facdd332` (#138 merged). No new language features, no token +claims, no production `-compile-bc`/HFIR flip, #90 untouched. + +## 1. #139: governed `(fetch "http://x")` crashed the bytecode compiler + +**Root cause:** +- `fetch` takes `(fetch url method [body])`. The AST bytecode compiler's + `fetch` case (`internal/bytecode/bytecode.go`) compiled `Children[1]` and + `Children[2]` without checking arity. +- A one-argument `(fetch url)` therefore indexed past the end of the child + list and panicked with `index out of range [2] with length 2`. +- Nothing upstream stopped it. The governed profile allows `fetch`, since + network is grantable at run time, so the profile scan accepted it. The + checker's `fetch` arity check only runs on the JS statement path. +- The interpreter (`InterpErr`) and the HFIR lowering already reject the wrong + arity cleanly. Only the AST bytecode compiler crashed. +- The panic affected both profiles. It surfaced in the governed build reported + in #139. + +**Fix:** the compiler now checks arity first. On a bad arity it reports the +diagnostic `fetch expects (fetch url method [body])` (same wording as the +interpreter), exits 1, prints no panic and writes no artifact. The well-formed +`(fetch url method)` still builds under governed. Network stays +grant-at-run-time and the capability rules are unchanged. + +**Regression test:** `TestIssue139FetchArityFailsClosed`. + +**Not changed:** a body argument is still compiled away by AST bytecode. This +is pre-existing; `OpFetch` has no body operand. + +## 2. Empty `min` / `max` + +**Before:** `(list_get L 0)` on an empty list silently returned an +empty/absent value. + +**Now:** it is a runtime error on every path: the interpreter, governed AST +bytecode and HFIR bytecode. It writes `VALUE_ERROR: min of empty list` (or +`max`) to stderr and exits 1. Output printed before the error stays. +- This is the same stderr-plus-`exit 1` shape that `assert` already desugars + to. +- It is still pure sugar: no new opcode, and `inspect --caps` still reports + nothing. +- **Caveat:** this is a coded stderr message, not a structured `RuntimeError` + JSON code. Making it structured would need a new opcode, which is out of + scope. + +**Test:** `TestEmptyMinMaxIsValueError`. + +## 3. Go/JS target guard (Lain) + +`build --target go|js` now fails closed when the program uses a #138 +core-library built-in or `nz`. The error is +`UNSUPPORTED_ON_TARGET: … build for the bytecode target instead`. The exit is +nonzero, there is no "Built" line and no file is written. The built-ins are +not implemented on these targets. + +- **Detection:** it runs after desugar and looks for the desugar temporaries + (`__cl*`, `__nz`). Only real expansions are rejected; a user variable or + defun named `sum` or `sort` still builds. +- **Why nz is included:** it lowers to a let-expression in value position, + which both backends emit as a statement. The output did not compile, and + this was confirmed in #137. + +**Test:** `TestGoJSTargetsRejectCoreLibraryAndNz`, which covers 7 forms × 2 +targets plus the variable-name control. + +**Caveat:** other sugar that produces let-expressions (`all`/`any`, +`ver_gt`/`ver_ge`, the `list_get`/`parse_json` operand binding) is not covered +by this guard. Lain's guard was scoped to #138 and `nz`. + +## Docs + +- heldout-v1 REPORT: + - Total-token ratio: HowlFrame uses 1.076× Python's total on cl100k_base + (2132 vs 1982) and 1.078× on o200k_base. + - `str_len`/`str_slice` count bytes. + - `sort` is O(n²) insertion. + - The Go/JS guard. +- The `desugar_corelib.go` header now documents the same points. + +## Verification + +- `gofmt -l` is empty. +- `go build/vet/test ./...` and `go test ./experiments/...` pass. +- All `run.sh` scripts pass: + - v0: 60/60 per language. + - heldout-v0: 126/126. + - heldout-v1: 136/136. + - The results files are unchanged, so no reference changed. diff --git a/experiments/token-efficiency-heldout-v1/REPORT.md b/experiments/token-efficiency-heldout-v1/REPORT.md index 6ef305a5..b88ba9e7 100644 --- a/experiments/token-efficiency-heldout-v1/REPORT.md +++ b/experiments/token-efficiency-heldout-v1/REPORT.md @@ -15,6 +15,10 @@ per task. It does not win.** | HowlFrame vs JS | 0.922 | 26/2/6 | | HowlFrame vs Go | 0.339 | 34/0/0 | +- Total tokens across all 34 tasks: HowlFrame uses **1.076×** Python's total + (2132 vs 1982 on cl100k_base; 1.078× on o200k_base), 0.948× JS's and + 0.394× Go's. The per-task median hides that the losses are larger than the + wins. - Median tokens are 52.5 for HowlFrame and 53.0 for Python, so the ratio of medians is 0.991. - HowlFrame is clearly shorter than Go and somewhat shorter than JS. @@ -142,6 +146,11 @@ The method is unchanged from heldout-v0: field and is unique. With duplicate skus, ties would be broken differently. - **v18 distinct.** It uses `map_keys` of `count_by`, which is documented to return keys in byte order. +- **Library semantics.** `str_len` and `str_slice` count bytes, not + characters (`str_split s ""` splits bytes). `sort` is a stable insertion + sort, O(n²). Empty `min`/`max` is a runtime `VALUE_ERROR` (since the #139 + hardening slice). - **Go/JS targets.** The core-library sugar does not compile on the Go/JS - backends. Their let-in-expression lowering is a known pre-existing gap. All + backends, because their let-in-expression lowering is a known pre-existing + gap. `build --target go|js` now rejects it with `UNSUPPORTED_ON_TARGET`. All HowlFrame references run on governed `-run-bc`. diff --git a/howlframe.go b/howlframe.go index 46b25eac..edb14b64 100644 --- a/howlframe.go +++ b/howlframe.go @@ -793,6 +793,7 @@ func buildSource() { if diags := hfir.VerifyProfile(root, profile, inputFile, ""); len(diags) > 0 { reportHFIRDiagnostics(diags) } + rejectUnsupportedTargetSugar(root, strings.ToLower(strings.TrimSpace(*target))) analysis := checker.Check(root) hfirModule := filepath.Base(inputFile) @@ -1115,3 +1116,34 @@ func writeExecutionReceipt(path string, receipt vm.ExecutionReceipt) error { } return os.Rename(f.Name(), path) } + +// rejectUnsupportedTargetSugar fails closed when a Go or JS build would emit +// broken code. The core-library built-ins (#138: list_slice, str_slice, +// str_len, index_of, map_list, filter, reduce, sum, min, max, reverse, +// count_by, sort) and nz desugar into let-expressions in value position, which +// the Go and JS backends emit as statements, so the output does not compile. +// Their expansions are recognized by the desugar temporaries (__cl*, __nz), +// so a user variable or defun with the same name is not rejected. It runs +// before any file is written: nonzero exit, no "Built", no output. +func rejectUnsupportedTargetSugar(root *ast.Node, target string) { + if target != "go" && target != "js" && target != "javascript" { + return + } + var walk func(n *ast.Node) + walk = func(n *ast.Node) { + if n == nil { + return + } + if n.Type == "SYMBOL" && (strings.HasPrefix(n.Value, "__cl") || n.Value == "__nz") { + what := "a core-library built-in (list_slice, str_slice, str_len, index_of, map_list, filter, reduce, sum, min, max, reverse, count_by, sort)" + if n.Value == "__nz" { + what = "nz" + } + ast.ReportError(fmt.Sprintf("UNSUPPORTED_ON_TARGET: %s is not supported on --target=%s (it lowers to a let-expression this backend cannot emit); build for the bytecode target instead", what, target), n.Line, n.Column) + } + for _, c := range n.Children { + walk(c) + } + } + walk(root) +} diff --git a/internal/ast/desugar_corelib.go b/internal/ast/desugar_corelib.go index a3fce9d1..36e39f78 100644 --- a/internal/ast/desugar_corelib.go +++ b/internal/ast/desugar_corelib.go @@ -10,16 +10,20 @@ package ast // out-of-range indices are clamped by the scan. // (list_slice lst a) runs to the end. // (str_slice s a b) same over the bytes of s; (str_slice s a) too. -// (str_len s) byte length of s. +// (str_len s) byte length of s (bytes, not characters). // (index_of lst v) first 0-based index of v, or -1. // (map_list x lst expr) list of expr for each x. // (filter x lst pred) elements x where pred holds. // (reduce acc init x lst expr) fold: acc starts at init, set to expr per x. // (sum lst) numeric sum (0 for empty). -// (min lst) / (max lst) smallest / largest element (first on ties). +// (min lst) / (max lst) smallest / largest element (first on ties). An +// empty list is a runtime error: stderr +// "VALUE_ERROR: min of empty list" and exit 1, the +// same stderr+exit shape as assert, on every path. // (reverse lst) list in reverse order. // (count_by x lst key) dict mapping each key value to its integer count. // (sort lst) ascending, stable; numbers or strings (byte order). +// Insertion sort: O(n^2) comparisons and copies. var corelibTemplates = map[string]struct { arity int @@ -46,9 +50,9 @@ var corelibTemplates = map[string]struct { "sum": {1, []string{"lst"}, []string{"L", "S", "X"}, "(let ($L $lst $S 0) (do (for $X $L (set $S (+ $S $X))) $S))", -1}, "min": {1, []string{"lst"}, []string{"L", "M", "X"}, - "(let ($L $lst $M (list_get $L 0)) (do (for $X $L (when (< $X $M) (set $M $X))) $M))", -1}, + "(let ($L $lst $M (list_get $L 0)) (do (when (= (list_len $L) 0) (do (stderr $msg) (exit 1))) (for $X $L (when (< $X $M) (set $M $X))) $M))", -1}, "max": {1, []string{"lst"}, []string{"L", "M", "X"}, - "(let ($L $lst $M (list_get $L 0)) (do (for $X $L (when (> $X $M) (set $M $X))) $M))", -1}, + "(let ($L $lst $M (list_get $L 0)) (do (when (= (list_len $L) 0) (do (stderr $msg) (exit 1))) (for $X $L (when (> $X $M) (set $M $X))) $M))", -1}, "reverse": {1, []string{"lst"}, []string{"L", "R", "N"}, "(let ($L $lst $R (list) $N (list_len $L)) (do (while (> $N 0) (do (set $N (- $N 1)) (append $R (list_get $L $N)))) $R))", -1}, "count_by": {3, []string{"x", "lst", "key"}, []string{"L", "D", "K"}, @@ -88,6 +92,9 @@ func desugarCorelib(node *Node) bool { for i, v := range spec.vars { args[v] = node.Children[i+1] } + if head == "min" || head == "max" { + args["msg"] = &Node{Type: "STRING", Value: "VALUE_ERROR: " + head + " of empty list\n", Line: node.Line, Column: node.Column, Filename: node.Filename} + } for _, tmp := range spec.temps { args[tmp] = symNode(compactTemp("cl"+tmp), node) } diff --git a/internal/bytecode/bytecode.go b/internal/bytecode/bytecode.go index d208fd81..79a3f9c3 100644 --- a/internal/bytecode/bytecode.go +++ b/internal/bytecode/bytecode.go @@ -483,6 +483,13 @@ func (c *BCCompiler) compileNode(node *ast.Node) []BCInstruction { } insts = append(insts, BCInstruction{OpString: "STORE_KEYS", Op: OpStoreKeys, StringOperand: handleNode.Value}) case "fetch": + // (fetch url method [body]). Arity is checked before any child is + // indexed: a one-argument (fetch url) used to index past the end + // and panic the compiler (#139). Fail closed with a diagnostic. + if len(node.Children) != 3 && len(node.Children) != 4 { + ast.ReportError("fetch expects (fetch url method [body])", node.Line, node.Column) + return insts + } insts = append(insts, c.compileNode(node.Children[1])...) insts = append(insts, c.compileNode(node.Children[2])...) insts = append(insts, BCInstruction{OpString: "FETCH", Op: OpFetch}) diff --git a/issue139_corelib_hardening_cli_test.go b/issue139_corelib_hardening_cli_test.go new file mode 100644 index 00000000..5654685b --- /dev/null +++ b/issue139_corelib_hardening_cli_test.go @@ -0,0 +1,126 @@ +package main + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// #139: a governed build of (fetch "http://x") panicked the AST bytecode +// compiler (index out of range on the missing method child). It must fail +// closed with a diagnostic, nonzero exit, no panic and no artifact, under +// both profiles; the well-formed (fetch url method) still builds. +func TestIssue139FetchArityFailsClosed(t *testing.T) { + bin := c1c2Binary(t) + dir := t.TempDir() + src := filepath.Join(dir, "f.howl") + art := filepath.Join(dir, "f.hfbc") + os.WriteFile(src, []byte(`(cli_app (fetch "http://x"))`), 0600) + for _, profile := range []string{"governed", "default"} { + out, err := exec.Command(bin, "build", "--profile", profile, src, "-o", art).CombinedOutput() + if err == nil { + t.Fatalf("%s: one-argument fetch built: %s", profile, out) + } + if strings.Contains(string(out), "panic") || strings.Contains(string(out), "goroutine") { + t.Fatalf("%s: compiler panicked: %s", profile, out) + } + if !strings.Contains(string(out), "fetch expects (fetch url method [body])") { + t.Fatalf("%s: missing diagnostic: %s", profile, out) + } + if _, err := os.Stat(art); err == nil { + t.Fatalf("%s: artifact written after a rejected build", profile) + } + } + good := filepath.Join(dir, "g.howl") + os.WriteFile(good, []byte(`(cli_app (print (fetch "http://127.0.0.1:1" "GET")))`), 0600) + if out, err := exec.Command(bin, "build", "--profile", "governed", good, "-o", art).CombinedOutput(); err != nil { + t.Fatalf("well-formed fetch did not build: %v %s", err, out) + } +} + +// Empty min/max is a runtime error (VALUE_ERROR on stderr, exit 1), not a +// silent nil, on the interpreter, governed AST bytecode and HFIR bytecode. +func TestEmptyMinMaxIsValueError(t *testing.T) { + bin := c1c2Binary(t) + for _, op := range []string{"min", "max"} { + t.Run(op, func(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "e.howl") + art := filepath.Join(dir, "e.hfbc") + hart := filepath.Join(dir, "h.hfbc") + os.WriteFile(src, []byte(`(do (print (`+op+` (list 2 7 5))) (print (`+op+` (list))) (print "after"))`), 0600) + if out, err := exec.Command(bin, "build", "--profile", "governed", src, "-o", art).CombinedOutput(); err != nil { + t.Fatalf("build: %v %s", err, out) + } + if out, err := exec.Command(bin, "-compile-hfir-bc", src, "-o", hart).CombinedOutput(); err != nil { + t.Fatalf("hfir: %v %s", err, out) + } + want := map[string]string{"min": "2\n", "max": "7\n"}[op] + for name, args := range map[string][]string{ + "interpreter": {"-run", src}, + "bytecode_governed": {"-run-bc", art}, + "hfir_bytecode": {"-run-bc", hart}, + } { + var stdout, stderr bytes.Buffer + cmd := exec.Command(bin, args...) + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + if err == nil { + t.Fatalf("%s: empty %s succeeded: %q", name, op, stdout.String()) + } + if stdout.String() != want { + t.Fatalf("%s: stdout %q want %q", name, stdout.String(), want) + } + if !strings.Contains(stderr.String(), "VALUE_ERROR: "+op+" of empty list") { + t.Fatalf("%s: stderr %q", name, stderr.String()) + } + } + }) + } +} + +// Lain's guard: Go/JS builds reject the #138 core-library built-ins and nz +// (both lower to let-expressions those backends cannot emit) with a clear +// error, nonzero exit, no "Built" and no output file. A user variable that +// shares a built-in name is unaffected. +func TestGoJSTargetsRejectCoreLibraryAndNz(t *testing.T) { + bin := c1c2Binary(t) + programs := map[string]string{ + "sum": `(cli_app (let (xs (list 3 1)) (print (sum xs))))`, + "sort": `(cli_app (let (xs (list "b" "a")) (print (str_join (sort xs) ","))))`, + "filter": `(cli_app (let (xs (list 1 2)) (print (list_len (filter x xs (> x 1))))))`, + "str_slice": `(cli_app (print (str_slice "hello" 1 3)))`, + "count_by": `(cli_app (let (xs (list "a")) (print (encode_json (count_by x xs x)))))`, + "nz": `(cli_app (let (m (dict ("a" "x"))) (print (nz (map_get m "b") "d"))))`, + "list_slice": `(cli_app (let (xs (list 1 2 3)) (print (list_len (list_slice xs 1)))))`, + } + for _, target := range []string{"go", "js"} { + for name, program := range programs { + t.Run(target+"/"+name, func(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "p.howl") + outDir := filepath.Join(dir, "out") + os.WriteFile(src, []byte(program), 0600) + out, err := exec.Command(bin, "build", "--target="+target, src, "-o", outDir).CombinedOutput() + if err == nil { + t.Fatalf("build succeeded: %s", out) + } + if strings.Contains(string(out), "Built") || !strings.Contains(string(out), "UNSUPPORTED_ON_TARGET") { + t.Fatalf("unexpected output: %s", out) + } + if entries, _ := os.ReadDir(outDir); len(entries) != 0 { + t.Fatalf("files written after rejection: %v", entries) + } + }) + } + } + dir := t.TempDir() + src := filepath.Join(dir, "v.howl") + os.WriteFile(src, []byte(`(cli_app (let (sum 2) (print sum)))`), 0600) + if out, err := exec.Command(bin, "build", "--target=go", src, "-o", filepath.Join(dir, "out")).CombinedOutput(); err != nil { + t.Fatalf("variable named sum rejected: %v %s", err, out) + } +}