From 5bfdc7b20d2f06f81004756400761726383f108a Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Wed, 13 May 2026 19:21:43 +0900 Subject: [PATCH] w6c+selfhost: cgen && and || short-circuit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both stages were eagerly evaluating RHS regardless of LHS (eager ANDQ/ORQ on the two results). Now: eval LHS into AX, CMPQ $0 + JE/JNE to a per-call-site label, eval RHS into AX, fall through. AX holds the LHS sentinel on the skipped path — typechecker already enforces bool operands. Surfaced by lib/getopt's nil-argv guard segfault. Six new rows in test/wcc/700_e2e.c, three of which segfault pre-fix. lib/getopt test comment relaxed; nested-if kept as regression marker. --- cmd/w6c/cgen.c | 26 +++++++++----- lib/getopt/getopttest.ww | 8 ++--- selfhost/cmd/w6c/main.combined.ww | 24 +++++++++++-- selfhost/cmd/wcc/cgenexpr.ww | 24 +++++++++++-- selfhost/cmd/wwdump/main.combined.ww | 24 +++++++++++-- test/wcc/700_e2e.c | 53 ++++++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 18 deletions(-) diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index ea676826..38bafcfa 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -1380,6 +1380,22 @@ cgexpr(Cg *c, Node *n, Local *locals) } break; case N_BIN: { + /* Short-circuit `&&` / `||`. Operands are bool (0/1); the + * type checker enforces it. Eval LHS into AX, branch over + * RHS on the short-circuit polarity, otherwise eval RHS + * into AX. The surviving AX is the result. Must precede + * any eager-eval path below — `if (p != nil && p.x > 0)` + * would segfault on a nil deref otherwise. */ + if (n->op == TK_AND || n->op == TK_OR) { + char *end = mklabel(c, n->op == TK_AND ? "andend" : "orend"); + int jshrt = (n->op == TK_AND) ? A_JE : A_JNE; + cgexpr(c, n->lhs, locals); + ins2(c, A_CMPQ, aimm(0), areg(D_AX)); + ins1(c, jshrt, abranch(end)); + cgexpr(c, n->rhs, locals); + label(c, end); + break; + } /* str == str / str != str — delegate to rt_streq, which * does the byte-by-byte compare. */ if ((n->op == TK_EQ || n->op == TK_NEQ) && @@ -1550,14 +1566,8 @@ cgexpr(Cg *c, Node *n, Local *locals) label(c, e); break; } - case TK_AND: case TK_OR: { - /* short-circuit not yet — eager evaluation. */ - if (n->op == TK_AND) - ins2(c, A_ANDQ, areg(D_BX), areg(D_AX)); - else - ins2(c, A_ORQ, areg(D_BX), areg(D_AX)); - break; - } + /* TK_AND / TK_OR handled with short-circuit codegen at the + * top of N_BIN — they never reach this eager-eval switch. */ default: break; } break; diff --git a/lib/getopt/getopttest.ww b/lib/getopt/getopttest.ww index 5a05aa05..9e165323 100644 --- a/lib/getopt/getopttest.ww +++ b/lib/getopt/getopttest.ww @@ -14,10 +14,10 @@ // path uses MOVL and avoids the over-read. // // Guards on a possibly-nil [[command.argsptr]] are split into nested -// `if`s rather than `argslen > 0 && !streq(argsptr[0], ...)`: the -// cstage cgen evaluates the RHS of `&&` even when the LHS is false, -// segfaulting on the nil deref when tryparse exits with no positional -// args (e.g. `["ls", "--"]`). +// `if`s rather than `argslen > 0 && !streq(argsptr[0], ...)`. The +// flattened `&&` form is now legal — both cgen stages short-circuit +// per task #15 — but the nested-if shape was the original workaround +// and is preserved here as a regression marker. use getopt; use strings; diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index f8771749..3a9bbd42 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -9668,6 +9668,26 @@ fn cgun(c: *cgen, n: *node) void = { }; fn cgbin(c: *cgen, n: *node) void = { + // Short-circuit `&&` / `||`. Operands are bool (0/1); the type + // checker enforces it. Eval LHS into AX, branch over RHS on the + // short-circuit polarity, otherwise eval RHS into AX. The + // surviving AX is the result. Must precede any eager-eval path + // below — `if (p != nil && p.x > 0)` would segfault on a nil + // deref otherwise. Byte-identical to cmd/w6c/cgen.c N_BIN. + if (n.op == tkind.TK_AND || n.op == tkind.TK_OR) { + let prefix: str = "andend"; + let jshrt: str = "JE"; + if (n.op == tkind.TK_OR) { prefix = "orend"; jshrt = "JNE"; }; + let end: str = mklabel(c, prefix); + cgexpr(c, n.lhs); + emitline("\tCMPQ\t$0, AX\n"); + emitline("\t"); emitline(jshrt); emitline("\t"); + emitline(end); emitline("\n"); + cgexpr(c, n.rhs); + emitlabel(end); + return; + }; + let unsignd: bool = nodeisunsigned(c, n.lhs); if (!unsignd) { unsignd = nodeisunsigned(c, n.rhs); }; @@ -9773,8 +9793,8 @@ fn cgbin(c: *cgen, n: *node) void = { emitline("\tSHRQ\tCX, AX\n"); return; }; - if (n.op == tkind.TK_AND) { emitline("\tANDQ\tBX, AX\n"); return; }; - if (n.op == tkind.TK_OR) { emitline("\tORQ\tBX, AX\n"); return; }; + // TK_AND / TK_OR handled with short-circuit codegen at the top of + // cgbin — they never reach this eager-eval tail. // Comparison: emit CMPQ, jump on signed/unsigned variant, // materialise 0/1 in AX. Same shape as the C cgen. diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index cd63a005..7f6f284f 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -1887,6 +1887,26 @@ fn cgun(c: *cgen, n: *node) void = { }; fn cgbin(c: *cgen, n: *node) void = { + // Short-circuit `&&` / `||`. Operands are bool (0/1); the type + // checker enforces it. Eval LHS into AX, branch over RHS on the + // short-circuit polarity, otherwise eval RHS into AX. The + // surviving AX is the result. Must precede any eager-eval path + // below — `if (p != nil && p.x > 0)` would segfault on a nil + // deref otherwise. Byte-identical to cmd/w6c/cgen.c N_BIN. + if (n.op == tkind.TK_AND || n.op == tkind.TK_OR) { + let prefix: str = "andend"; + let jshrt: str = "JE"; + if (n.op == tkind.TK_OR) { prefix = "orend"; jshrt = "JNE"; }; + let end: str = mklabel(c, prefix); + cgexpr(c, n.lhs); + emitline("\tCMPQ\t$0, AX\n"); + emitline("\t"); emitline(jshrt); emitline("\t"); + emitline(end); emitline("\n"); + cgexpr(c, n.rhs); + emitlabel(end); + return; + }; + let unsignd: bool = nodeisunsigned(c, n.lhs); if (!unsignd) { unsignd = nodeisunsigned(c, n.rhs); }; @@ -1992,8 +2012,8 @@ fn cgbin(c: *cgen, n: *node) void = { emitline("\tSHRQ\tCX, AX\n"); return; }; - if (n.op == tkind.TK_AND) { emitline("\tANDQ\tBX, AX\n"); return; }; - if (n.op == tkind.TK_OR) { emitline("\tORQ\tBX, AX\n"); return; }; + // TK_AND / TK_OR handled with short-circuit codegen at the top of + // cgbin — they never reach this eager-eval tail. // Comparison: emit CMPQ, jump on signed/unsigned variant, // materialise 0/1 in AX. Same shape as the C cgen. diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index b382a864..50b8f7bf 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -9668,6 +9668,26 @@ fn cgun(c: *cgen, n: *node) void = { }; fn cgbin(c: *cgen, n: *node) void = { + // Short-circuit `&&` / `||`. Operands are bool (0/1); the type + // checker enforces it. Eval LHS into AX, branch over RHS on the + // short-circuit polarity, otherwise eval RHS into AX. The + // surviving AX is the result. Must precede any eager-eval path + // below — `if (p != nil && p.x > 0)` would segfault on a nil + // deref otherwise. Byte-identical to cmd/w6c/cgen.c N_BIN. + if (n.op == tkind.TK_AND || n.op == tkind.TK_OR) { + let prefix: str = "andend"; + let jshrt: str = "JE"; + if (n.op == tkind.TK_OR) { prefix = "orend"; jshrt = "JNE"; }; + let end: str = mklabel(c, prefix); + cgexpr(c, n.lhs); + emitline("\tCMPQ\t$0, AX\n"); + emitline("\t"); emitline(jshrt); emitline("\t"); + emitline(end); emitline("\n"); + cgexpr(c, n.rhs); + emitlabel(end); + return; + }; + let unsignd: bool = nodeisunsigned(c, n.lhs); if (!unsignd) { unsignd = nodeisunsigned(c, n.rhs); }; @@ -9773,8 +9793,8 @@ fn cgbin(c: *cgen, n: *node) void = { emitline("\tSHRQ\tCX, AX\n"); return; }; - if (n.op == tkind.TK_AND) { emitline("\tANDQ\tBX, AX\n"); return; }; - if (n.op == tkind.TK_OR) { emitline("\tORQ\tBX, AX\n"); return; }; + // TK_AND / TK_OR handled with short-circuit codegen at the top of + // cgbin — they never reach this eager-eval tail. // Comparison: emit CMPQ, jump on signed/unsigned variant, // materialise 0/1 in AX. Same shape as the C cgen. diff --git a/test/wcc/700_e2e.c b/test/wcc/700_e2e.c index 0ecb6171..b5035afb 100644 --- a/test/wcc/700_e2e.c +++ b/test/wcc/700_e2e.c @@ -1627,6 +1627,59 @@ static const struct row rows[] = { "fn main() i32 = {\n" " return fmt.println(\"hello\", 7i64): i32;\n" "};", 8 }, + /* short-circuit `&&`: RHS skipped when LHS is false. Without + * short-circuit the `p.x` deref on a nil pointer segfaults. + * Pinned the cgen bug surfaced by lib/getopt's argv guards + * (`argslen > 0 && !streq(argsptr[0], "--")`) on a nil argsptr. */ + { "type point = struct { x: i32, y: i32 };\n" + "fn main() i32 = {\n" + " let p: *point = nil;\n" + " if (p != nil && p.x > 0) { return 1; };\n" + " return 42;\n" + "};", 42 }, + /* short-circuit `||`: RHS skipped when LHS is true. */ + { "type point = struct { x: i32, y: i32 };\n" + "fn main() i32 = {\n" + " let p: *point = nil;\n" + " if (p == nil || p.x > 0) { return 42; };\n" + " return 1;\n" + "};", 42 }, + /* `&&` LHS true: RHS evaluated, expression yields its boolean. */ + { "type point = struct { x: i32, y: i32 };\n" + "fn main() i32 = {\n" + " let pt: point = point { x = 5, y = 10 };\n" + " let p: *point = &pt;\n" + " if (p != nil && p.x > 0) { return 42; };\n" + " return 1;\n" + "};", 42 }, + /* `||` LHS false: RHS evaluated, expression yields its boolean. */ + { "type point = struct { x: i32, y: i32 };\n" + "fn main() i32 = {\n" + " let pt: point = point { x = 7, y = 0 };\n" + " let p: *point = &pt;\n" + " if (p == nil || p.x > 0) { return 42; };\n" + " return 1;\n" + "};", 42 }, + /* mixed `&&` / `||` precedence — `&&` binds tighter than `||`, + * so `(p != nil && p.x > 0) || p == nil`. With short-circuit at + * each level: AND skips `p.x` (LHS false), OR keeps the true. */ + { "type point = struct { x: i32, y: i32 };\n" + "fn main() i32 = {\n" + " let p: *point = nil;\n" + " if (p != nil && p.x > 0 || p == nil) { return 42; };\n" + " return 1;\n" + "};", 42 }, + /* short-circuit must still yield a clean boolean in the + * expression context (not just inside `if`). `true && false` + * stored into a bool and re-checked. */ + { "fn main() i32 = {\n" + " let a: bool = (1 > 0) && (2 < 1);\n" + " let b: bool = (1 < 0) || (2 > 1);\n" + " let n: i32 = 0;\n" + " if (!a) { n += 10; };\n" + " if (b) { n += 32; };\n" + " return n;\n" + "};", 42 }, { NULL, 0 } };