From dca7bb22e5c4f69278486b517715e6bceceea2b4 Mon Sep 17 00:00:00 2001 From: CharlesKWON Date: Mon, 4 May 2026 07:05:22 +0900 Subject: [PATCH] fix(gengo): count nested LOCALs into the function frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Function-entry Frame() allocation counted only top-level LOCAL declarations from fn.Body. Mid-function LOCALs hidden inside an IF / FOR / WHILE / DO CASE / SWITCH / SEQUENCE block weren't included, so the runtime allocated a frame too small to hold them. Subsequent reads/writes via PopLocalFast / PushLocalFast / LocalAdd to those slot indices then either silently scribbled past the frame (read-back saw NIL) or panicked with "local variable index out of range" once the index exceeded the underlying slice. This is the underlying bug behind frb_demo Section 4 — the `LOCAL ch := Channel(1)` declared inside `IF pAsync != NIL` got slot N+1 from the codegen but the runtime only allocated N. The Channel value was scribbled past the frame, ChReceive then read NIL from a non-existent slot, and the goroutine's ChSend(49) had nowhere to land. New helper gen_util.go::countLocalsInStmts walks every nested body (IF + ElseIfs + ElseBody, ForStmt, ForEachStmt, DoWhileStmt, SeqStmt's Body + RecoverBody, SwitchStmt's Cases + Otherwise) and totals every ScopeLocal VarDecl. The function-emit caller adds this to the top-level count before sizing the Frame. Test fixture (tests/frb/test_frb_goroutine.prg) reproduces the demo Section 4 shape — `LOCAL ch := Channel(1)` inside IF, then `Go("WORKER", ch, 7)`, then ChReceive(ch). Wired into the FRB runner so it stands at 6/6. Other gates green: go test ./... : PASS FiveSql2 SQL:1999 : 43/43 Harbour compat : 56/56 std.ch suite : 15/15 FRB suite : 6/6 Co-Authored-By: Claude Opus 4.7 (1M context) --- compiler/gengo/gen_util.go | 41 ++++++++++++++++++++++++++++++++ compiler/gengo/gengo.go | 15 ++++++------ tests/frb/run.sh | 3 +++ tests/frb/test_frb_goroutine.prg | 14 +++++++++++ 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 tests/frb/test_frb_goroutine.prg diff --git a/compiler/gengo/gen_util.go b/compiler/gengo/gen_util.go index f91a669..3405bd0 100644 --- a/compiler/gengo/gen_util.go +++ b/compiler/gengo/gen_util.go @@ -69,6 +69,47 @@ func scanExprForXBase(expr ast.Expr) bool { return false } +// countLocalsInStmts walks a statement list (recursively into IF / +// FOR / WHILE / DO CASE / SWITCH / SEQUENCE / WITH bodies) and totals +// every `LOCAL` declaration found. Function-entry frame allocation +// must include these so a mid-function or nested LOCAL doesn't get +// assigned a slot index past the runtime's allocated count — that +// previously surfaced as silent NIL reads / out-of-range writes +// (e.g. frb_demo Section 4 with `LOCAL ch := Channel(1)` inside an +// IF block returning NIL on receive). +func countLocalsInStmts(stmts []ast.Stmt) int { + n := 0 + for _, s := range stmts { + switch v := s.(type) { + case *ast.VarDecl: + if v.Scope == ast.ScopeLocal { + n += len(v.Vars) + } + case *ast.IfStmt: + n += countLocalsInStmts(v.Body) + for _, ei := range v.ElseIfs { + n += countLocalsInStmts(ei.Body) + } + n += countLocalsInStmts(v.ElseBody) + case *ast.ForStmt: + n += countLocalsInStmts(v.Body) + case *ast.ForEachStmt: + n += countLocalsInStmts(v.Body) + case *ast.DoWhileStmt: + n += countLocalsInStmts(v.Body) + case *ast.SeqStmt: + n += countLocalsInStmts(v.Body) + n += countLocalsInStmts(v.RecoverBody) + case *ast.SwitchStmt: + for _, c := range v.Cases { + n += countLocalsInStmts(c.Body) + } + n += countLocalsInStmts(v.Otherwise) + } + } + return n +} + func scanStmtsForXBase(stmts []ast.Stmt) bool { for _, s := range stmts { switch v := s.(type) { diff --git a/compiler/gengo/gengo.go b/compiler/gengo/gengo.go index bc61389..6051d05 100644 --- a/compiler/gengo/gengo.go +++ b/compiler/gengo/gengo.go @@ -553,7 +553,13 @@ func (g *Generator) emitFuncDecl(fn *ast.FuncDecl) { g.writeln(fmt.Sprintf("func %s(t *hbrt.Thread) {", goName)) g.indent++ - // Count params and locals (including mid-function LOCALs in Body) + // Count params and locals (including mid-function LOCALs in Body — + // also the ones nested inside IF / FOR / WHILE / DO CASE / BEGIN + // SEQUENCE / etc.). Without recursion the function frame was sized + // to only the top-level LOCALs, so a nested `LOCAL ch := + // Channel(1)` inside `IF ...` got slot N+1 from the codegen but + // the runtime allocated N slots — silent miscompile when the + // nested local was later read (see frb_demo Section 4). nParams := len(fn.Params) nLocals := 0 for _, d := range fn.Decls { @@ -561,12 +567,7 @@ func (g *Generator) emitFuncDecl(fn *ast.FuncDecl) { nLocals += len(vd.Vars) } } - // Count mid-function LOCAL declarations in Body - for _, s := range fn.Body { - if vd, ok := s.(*ast.VarDecl); ok && vd.Scope == ast.ScopeLocal { - nLocals += len(vd.Vars) - } - } + nLocals += countLocalsInStmts(fn.Body) g.writeln(fmt.Sprintf("t.Frame(%d, %d)", nParams, nLocals)) g.writeln("defer t.EndProc()") g.writeln("") diff --git a/tests/frb/run.sh b/tests/frb/run.sh index 2507789..d33dfc1 100755 --- a/tests/frb/run.sh +++ b/tests/frb/run.sh @@ -9,6 +9,8 @@ # test_frb_compile — FrbCompile / FrbExec — in-memory compile. # test_frb_loop — FOR loop accumulators (`+=` and `:=`). # test_frb_step — FOR ... STEP -1 / STEP 2 in pcode mode. +# test_frb_goroutine — Go("WORKER", ...) launches dynamic FRB +# function in a goroutine + Channel rendezvous. # # Builds frb_simple.frb (and mathlib_pc.frb if needed) into the # scratch dir before running the loaders. @@ -34,6 +36,7 @@ TESTS=( test_frb_compile test_frb_loop test_frb_step + test_frb_goroutine ) pass=0 diff --git a/tests/frb/test_frb_goroutine.prg b/tests/frb/test_frb_goroutine.prg new file mode 100644 index 0000000..e742a5d --- /dev/null +++ b/tests/frb/test_frb_goroutine.prg @@ -0,0 +1,14 @@ +FUNCTION Main() + LOCAL cAsync := ; + 'FUNCTION Worker(ch, n)' + Chr(10) + ; + ' ChSend(ch, n * n)' + Chr(10) + ; + ' RETURN NIL' + Chr(10) + + LOCAL pAsync := FrbCompile(cAsync) + IF pAsync != NIL + LOCAL ch := Channel(1) + Go("WORKER", ch, 7) + ? " 7^2 from dynamic goroutine =", ChReceive(ch) + FrbUnload(pAsync) + ENDIF + RETURN NIL