From eda657becb10a05459f9f396e0c04ace41f85225 Mon Sep 17 00:00:00 2001 From: John Ky Date: Fri, 18 Sep 2026 18:45:04 +1000 Subject: [PATCH 1/2] fix(jq): carry the fold register into its UPDATE/EXTRACT nested pipes (#3145) #3133 gave a pipe stage's `Frame` the register's value, so a pipe nested under `try`/`if`/`,` could seed itself with it. A fold's UPDATE/EXTRACT takes its own route -- `FoldRegister::resolve` passes the register to `resolve_seq` explicitly, under a frame built by `unknown()`/`clone()` -- so the same nesting was still register-less there: `try ($v | .b)` could not re-establish `$v`, raised the resolver's own refusal, and `try` swallowed it. `del(foreach .a as $v (.; try ($v | .b); .))` on `{"a":{"b":1}}` echoed the document where jq writes `{"a":{}}`. The fold's update frame now carries `self.value`, gated on `self.trackable && !tr` in jq mode. The `self.trackable` half is load-bearing and was found by the fuzz, not the sweep: an untracked `FoldRegister`'s `value` is not a register at all (`enter`'s untracked arm seeds it from the ambient), and handing that to a nested pipe fabricated three writes in 18,000 programs -- none of which the sweep rows or the unit matrix reached. Not reached: a fold whose INIT is untracked *after a literal stage* re-seeds its register from the ambient, so the marker is unrecognised there (the pre-existing `literal-then-fold-untracked-init` / `carried-register-passthrough` class) and a `try` around it turns that refusal into a no-op write -- recorded in limitations.md and pinned. --- docs/compliance/jq/limitations.md | 16 +++-- scripts/jq-bind-origin-oracle-sweep.sh | 4 ++ src/jq/eval.rs | 88 +++++++++++++++++++++++++- 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 03f38db0c..04e6b1435 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -1262,12 +1262,16 @@ is the revert that established what the other one costs. now reaching nested pipes `... \| $x \| .b \| .c` would have navigated through it too. Such a bind is `Origin::Untracked` now (`identity_bind_position`); a marker source keeps its own origin, and a `null`/`bool` `.` loses nothing since `jv_identical` admits those by - value. The same register does **not** yet reach a fold's UPDATE/EXTRACT body: that route - (`FoldRegister::resolve`) passes its register to `resolve_seq` explicitly under a frame that - carries none, so `del(foreach .a as $v (.; try ($v \| .b); .))` on `{"a":{"b":1}}` still - echoes the document where jq writes `{"a":{}}`, as does `del(.a as $y \| .a \| 5 \| foreach - range(1) as $i (0; .; try ($y \| .b)))` — pre-existing, filed as - [#3145](https://github.com/rust-works/succinctly/issues/3145). Two refuse-only residuals, + value. [#3145](https://github.com/rust-works/succinctly/issues/3145) extends the same + carrying to a fold's UPDATE/EXTRACT body, whose own route (`FoldRegister::resolve`) passed + its register to `resolve_seq` explicitly under a frame that carried none: `del(foreach .a as + $v (.; try ($v \| .b); .))` on `{"a":{"b":1}}` echoed the document and now writes + `{"a":{}}`, as jq does. It does not reach a fold whose INIT is untracked *after a literal + stage* — that fold re-seeds its register from the ambient value, so the marker is not + recognised at all (the pre-existing `literal-then-fold-untracked-init` / + `carried-register-passthrough` class), and with a generator source the enclosing `try` then + swallows that refusal into a no-op write: `del(.a as $y \| .a \| 5 \| foreach range(1) as $i + (0; .; try ($y \| .b)))` is `{"a":{}}` in jq and echoes here. Two refuse-only residuals, both in the sweep: `path(.a \| try error(.) catch .)` — `error(.)` raises the register node itself and jq answers `["a"]`, but a payload equal to the register by value cannot be told from a rebuilt copy (`error({"a":1,"b":2})` refuses in diff --git a/scripts/jq-bind-origin-oracle-sweep.sh b/scripts/jq-bind-origin-oracle-sweep.sh index 34c1bcdad..f2c6f016d 100755 --- a/scripts/jq-bind-origin-oracle-sweep.sh +++ b/scripts/jq-bind-origin-oracle-sweep.sh @@ -454,6 +454,10 @@ computed-identity-bind-marker-source {"a":{"b":{"c":1}}} path(. as $x | 5 | $x a computed-identity-bind-mixed-if {"a":{"b":{"c":1}}} path(. as $x | 5 | (if true then $x else . end) as $y | $y) catch-break-payload-not-register {"a":null} (.a | label $out | try (break $out) catch .b) = 1 catch-break-restores-register {"a":null} path(.a as $y | .a | label $out | try (break $out) catch $y) +fold-body-nested-try {"a":{"b":1}} del(foreach .a as $v (.; try ($v | .b); .)) +fold-body-nested-if {"a":{"b":1}} path(foreach .a as $v (.; if true then ($v | .b) else . end; .)) +fold-extract-nested-try {"a":{"b":1}} path(.a as $y | .a | foreach range(1) as $i (0; .; try ($y | .b))) +fold-body-nested-try-sibling-control {"a":{"b":1},"c":{"b":1}} del(.a as $y | foreach .c as $v (.; try ($y | .b); .)) CASES_EOF # Known refuse-only rows (jq answers, succinctly refuses), each with the diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 6b4c0552d..83d69aed6 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -33785,11 +33785,31 @@ impl FoldRegister { // accumulator itself currently sits there (the seed below carries // the register for exactly that case). A lost register has no // position to offer. + // + // #3145: the frame carries the register's *value* too, for the same + // reason `resolve_seq_stage` puts it on a pipe stage's frame + // (#3133): a pipe nested in UPDATE/EXTRACT under `try`/`if`/`,` is + // reached through `resolve_node_sink`, which has no `PathBranch` to + // take the register from, so `try ($v | .b)` could not re-establish + // `$v` and `try` swallowed its refusal -- `del(foreach .a as $v (.; + // try ($v | .b); .))` echoed the document where jq writes + // `{"a":{}}`. Only while the register is **live** (`self.trackable`) + // and only in jq mode, exactly as the pipe-stage site gates it: an + // untracked `FoldRegister`'s `value` is not a register at all + // (`enter`'s own untracked arm seeds it from the ambient), and + // handing that to a nested pipe let markers re-establish where jq + // refuses -- three fabricated writes in 18,000 fuzzed programs, + // none of which the sweep or the unit rows reached. let update_frame = if self.trackable { self.frame.clone() } else { self.frame.unknown() - }; + } + .with_register(if S::TAG == EvalTag::Jq && self.trackable && !tr { + Some(&self.value) + } else { + None + }); let resolved = if let Expr::Pipe(exprs) = unwrap_paren(expr) { resolve_seq::( exprs, @@ -99970,6 +99990,72 @@ mod tests { } } + /// #3145: a pipe nested under `try`/`if`/`,` inside a fold's + /// UPDATE/EXTRACT reaches `resolve_node_sink` with no `PathBranch` to + /// take the register from, so `try ($v | .b)` could not re-establish + /// `$v`, raised the resolver's own refusal, and `try` swallowed it -- + /// `del(foreach .a as $v (.; try ($v | .b); .))` echoed the document + /// where jq writes `{"a":{}}` (#3133 fixed the pipe-stage half; the + /// fold's own `FoldRegister::resolve` frame carried no register). + /// Every row confirmed live against jq 1.7.1. + #[test] + fn test_fold_body_nested_pipe_keeps_the_register_3145() { + for (doc, filter, want) in [ + ( + &br#"{"a":{"b":1}}"#[..], + r"path(foreach .a as $v (.; try ($v | .b); .))", + r#"["a","b"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"del(foreach .a as $v (.; try ($v | .b); .))", + r#"{"a":{}}"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(foreach .a as $v (.; if true then ($v | .b) else . end; .))", + r#"["a","b"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(foreach .a as $v (.; ($v | .b), 1; .))", + r#"["a","b"]"#, + ), + // EXTRACT, not just UPDATE. + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a as $y | .a | foreach range(1) as $i (0; .; try ($y | .b)))", + r#"["a","b"]"#, + ), + // The sibling-copy control: `$v` is the register here, `$y` is + // not, and jq writes nothing through the latter either. + ( + &br#"{"a":{"b":1},"c":{"b":1}}"#[..], + r"del(foreach .c as $v (.; try ($v | .b); .))", + r#"{"a":{"b":1},"c":{}}"#, + ), + ( + &br#"{"a":{"b":1},"c":{"b":1}}"#[..], + r"del(.a as $y | foreach .c as $v (.; try ($y | .b); .))", + r#"{"a":{"b":1},"c":{"b":1}}"#, + ), + ] { + assert_eq!(outputs(doc, filter), [want], "{filter}"); + } + // Unchanged: a fold whose INIT is untracked *after a literal stage* + // re-seeds its register from the ambient, so the marker is not + // recognised at all -- pre-existing (`literal-then-fold-untracked- + // init`, `carried-register-passthrough`), and with a generator + // source the `try` then swallows that refusal into a no-op write. + assert_eq!( + outputs( + br#"{"a":{"b":1}}"#, + r"del(.a as $y | .a | 5 | foreach range(1) as $i (0; .; try ($y | .b)))" + ), + [r#"{"a":{"b":1}}"#] + ); + } + /// #2676: a fold's own destructuring pattern is tracked exactly the way /// a plain `. as PATTERN` bind is (#2649's walk, `PathPatternMode`), whenever the /// fold's SOURCE resolves through the path register. Every row here is From 6b212071b77388446c2126ee0edf171277f2e7fc Mon Sep 17 00:00:00 2001 From: John Ky Date: Fri, 18 Sep 2026 19:39:44 +1000 Subject: [PATCH 2/2] fix(jq): decline the fold register for a fan-out body; add a --fold-p fuzz knob (#3145 review) The register reached a fold body's nested pipe but not a *sibling* branch of the same multi-output body (those go through `resolve_node`, whose non-pipe arms this frame does not reach -- #2046's documented scope limit), so an UPDATE that used to refuse wholesale could half-succeed: `(foreach .a as {a:$v} ?// {c:$v} (0; ($v[0]?, $v))) = 9` wrote `.a.c`, a key jq never names, because only the second output refused and drove a `?//` retry jq does not perform; `del(foreach .a as $v (.; (($v | .b?), 1); try ($v | .b?)))` turned a loud refusal into a silent no-op write. A body that cannot fan out (`fans_out`) cannot split that way, so the register is carried only there, and the frame stops carrying it on the `Expr::Pipe` arm at all, where the explicit argument already wins and the copy was never read. The fuzz gains `--fold-p`: the changed code is reachable only from a fold, and at the stock 0.2 weight three 6000-program runs drew too few folds to see this class -- the review found it immediately at `--fold-p 1.0`. Both weightings are now at zero fabricate / zero mismatch (3x6000 stock, 3x4000 fold-only including the review's own seed), and both fabricating shapes are pinned in the sweep. --- scripts/jq-bind-origin-fuzz.py | 18 +++++-- scripts/jq-bind-origin-oracle-sweep.sh | 4 ++ src/jq/eval.rs | 71 +++++++++++++++++++++----- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/scripts/jq-bind-origin-fuzz.py b/scripts/jq-bind-origin-fuzz.py index 5b495841e..241d4c0ed 100755 --- a/scripts/jq-bind-origin-fuzz.py +++ b/scripts/jq-bind-origin-fuzz.py @@ -415,6 +415,11 @@ def main(): ap.add_argument("-n", type=int, default=500) ap.add_argument("--seed", type=int, default=1) ap.add_argument("--show", type=int, default=8) + ap.add_argument("--fold-p", type=float, default=None, + help="probability a program is a fold (default FOLD_P). A fold-only fix's " + "changed code is reachable only from a fold, so a stock run draws too " + "few to see its rare shapes -- #3145's review found a fabricated write " + "at --fold-p 1.0 that 18,000 stock programs had missed") ap.add_argument("--self-test", action="store_true") a = ap.parse_args() pin = open("tests/data/jq-golden/JQ_VERSION").read().strip() @@ -430,6 +435,11 @@ def main(): print(f"{name} ({len(pool)}): " + " ; ".join(pool)) return 0 rng = random.Random(a.seed) + # `--fold-p 1.0` drives every program through `fold_program`; the other + # routes keep their own shares of what is left. + fold_p = FOLD_P if a.fold_p is None else a.fold_p + route_p = ROUTE_P * (1.0 - fold_p) / max(1.0 - FOLD_P, 1e-9) + value_bind_p = VALUE_BIND_P * (1.0 - fold_p) / max(1.0 - FOLD_P, 1e-9) kinds = ["agree", "fabricate", "mismatch", "refuse-only", "refuse-early", "both-reject", "fabricate-baseline", "mismatch-baseline", "timeout"] counts = {k: 0 for k in kinds} @@ -438,13 +448,13 @@ def main(): dv = doc(rng) d = json.dumps(dv) r = rng.random() - if r < ROUTE_P: + if r < route_p: # #3036: the document twice, so `input` reads a second copy. f, d = route_program(rng, dv), d + "\n" + d - elif r < ROUTE_P + VALUE_BIND_P: + elif r < route_p + value_bind_p: # #3037: same ROUTES, so `input` needs its second copy too. f, d = value_bind_program(rng, dv), d + "\n" + d - elif r < ROUTE_P + VALUE_BIND_P + FOLD_P: + elif r < route_p + value_bind_p + fold_p: f = fold_program(rng) else: f = program(rng) @@ -458,7 +468,7 @@ def main(): counts[c] += 1 if c != "agree" and len(examples[c]) < a.show: examples[c].append((d, f, j, s)) - print(f"seed={a.seed} n={a.n} " + " ".join(f"{k}={v}" for k, v in counts.items() if v or k in kinds[:4])) + print(f"seed={a.seed} n={a.n} fold_p={fold_p:g} " + " ".join(f"{k}={v}" for k, v in counts.items() if v or k in kinds[:4])) for c in kinds[1:]: for d, f, j, s in examples[c]: print(f"[{c}] {f}\n on {d}\n jq[{j[0]}]: {j[1].strip()[:120]}\n sc[{s[0]}]: {s[1].strip()[:120]}") diff --git a/scripts/jq-bind-origin-oracle-sweep.sh b/scripts/jq-bind-origin-oracle-sweep.sh index f2c6f016d..11d1f55a9 100755 --- a/scripts/jq-bind-origin-oracle-sweep.sh +++ b/scripts/jq-bind-origin-oracle-sweep.sh @@ -458,6 +458,8 @@ fold-body-nested-try {"a":{"b":1}} del(foreach .a as $v (.; try ($v | .b); .)) fold-body-nested-if {"a":{"b":1}} path(foreach .a as $v (.; if true then ($v | .b) else . end; .)) fold-extract-nested-try {"a":{"b":1}} path(.a as $y | .a | foreach range(1) as $i (0; .; try ($y | .b))) fold-body-nested-try-sibling-control {"a":{"b":1},"c":{"b":1}} del(.a as $y | foreach .c as $v (.; try ($y | .b); .)) +fold-body-fanout-declines {"a":{"b":1}} (foreach .a as {a:$v} ?// {c:$v} (0; ($v[0]?, $v))) = 9 +fold-body-fanout-declines-del {"a":{"b":1}} del(foreach .a as $v (.; (($v | .b?), 1); try ($v | .b?))) CASES_EOF # Known refuse-only rows (jq answers, succinctly refuses), each with the @@ -504,6 +506,8 @@ untracked-opaque-stage-lost-register:#3120 review -- an opaque stage (reduce, a untracked-later-step-refusal-no-retry:#3120 review -- refusal_is_exact is decided per source, and a marker that is the register is value-equal to it, so a later-step refusal after a certified first step is treated as a guess and does not retry; jq retries onto $w. The trackable twin retries and agrees catch-payload-own-node-refuse-only:#3133 -- error(.) raises the register node itself and jq answers ["a"]; the payload equals the register by value but is not null/bool and carries no marker, so it cannot be told from a rebuilt copy (catch-rebuilt-payload-refuses) and the handler stays untracked computed-identity-bind-mixed-if:#3133 -- an if source with one arm a computed `.` and the other a marker binds Untracked on an untracked stage (the condition is not evaluated); jq evaluates it and binds the marker +fold-body-fanout-declines:#3145 review -- the register reaches a fold body only when that body cannot fan out: a nested pipe sees it while a sibling branch of the same multi-output body does not, so an UPDATE that refused wholesale could half-succeed and drive a ?// retry jq never performs, writing a key jq never names. Refusing the whole body is the safe side of that asymmetry +fold-body-fanout-declines-del:#3145 review -- the del twin: jq writes {"a":{}}, the half-success wrote nothing at exit 0, and declining refuses loudly instead REFUSE_EOF if [[ "${1:-}" == "--list-cases" ]]; then diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 83d69aed6..8201bb1cc 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -33456,6 +33456,28 @@ fn getpath_preserves_register( .is_some_and(|(invocation, path)| !stage_frame.names(invocation, path)) } +/// Whether `expr` can produce more than one output -- a syntactic +/// over-approximation, used by [`FoldRegister::resolve`] to decline +/// carrying the register into a body whose branches would then see it +/// unevenly (#3145 review). `Comma` and the iterating/recursing shapes +/// count; so does anything this predicate cannot see inside, since the +/// answer must be "maybe" for those. +fn fans_out(expr: &Expr) -> bool { + any_subexpr(expr, &mut |e| { + matches!( + e, + Expr::Comma(_) + | Expr::Iterate + | Expr::RecursiveDescent + | Expr::AsPattern { .. } + | Expr::Reduce { .. } + | Expr::Foreach { .. } + | Expr::FuncCall { .. } + | Expr::NamespacedCall { .. } + ) + }) +} + /// jq's own `jv_identical(v, jq->value_at_path)`, modeled for a value type /// that has no pointer to compare — the single definition shared by every /// site that has to answer "is this branch still sitting *on* the path @@ -33793,24 +33815,36 @@ impl FoldRegister { // take the register from, so `try ($v | .b)` could not re-establish // `$v` and `try` swallowed its refusal -- `del(foreach .a as $v (.; // try ($v | .b); .))` echoed the document where jq writes - // `{"a":{}}`. Only while the register is **live** (`self.trackable`) + // `{"a":{}}`. Only for a body that cannot fan out (`fans_out`), + // only while the register is **live** (`self.trackable`) // and only in jq mode, exactly as the pipe-stage site gates it: an // untracked `FoldRegister`'s `value` is not a register at all // (`enter`'s own untracked arm seeds it from the ambient), and // handing that to a nested pipe let markers re-establish where jq // refuses -- three fabricated writes in 18,000 fuzzed programs, // none of which the sweep or the unit rows reached. + // + // The `fans_out` half is the review's finding, and the same shape + // one level up: a nested pipe sees the register while a *sibling* + // branch of the same multi-output body does not (it goes through + // `resolve_node`, whose non-pipe arms this frame does not reach), + // so an UPDATE that used to refuse wholesale could half-succeed -- + // `(foreach .a as {a:$v} ?// {c:$v} (0; ($v[0]?, $v))) = 9` wrote + // `.a.c`, a key jq never names, because only the second output + // refused and drove a `?//` retry jq does not perform. A body with + // one output path cannot split that way. Widening the other + // direction (re-establishing in `resolve_node`'s own arms) is + // #2046's documented scope limit, not this fix's. let update_frame = if self.trackable { self.frame.clone() } else { self.frame.unknown() - } - .with_register(if S::TAG == EvalTag::Jq && self.trackable && !tr { - Some(&self.value) - } else { - None - }); + }; let resolved = if let Expr::Pipe(exprs) = unwrap_paren(expr) { + // The register goes in as the explicit argument here, which + // `resolve_seq_sink` prefers over the frame's and every later + // stage overwrites from `carried_register` -- so the frame does + // not carry it too (that copy would never be read). resolve_seq::( exprs, &input, @@ -33821,6 +33855,13 @@ impl FoldRegister { self.trackable.then_some(&self.value), ) } else { + let update_frame = update_frame.with_register( + if S::TAG == EvalTag::Jq && self.trackable && !tr && !fans_out(expr) { + Some(&self.value) + } else { + None + }, + ); resolve_node::(expr, &input, tr, snapshot, &update_frame, keep) }; let owned = match resolved { @@ -100016,11 +100057,6 @@ mod tests { r"path(foreach .a as $v (.; if true then ($v | .b) else . end; .))", r#"["a","b"]"#, ), - ( - &br#"{"a":{"b":1}}"#[..], - r"path(foreach .a as $v (.; ($v | .b), 1; .))", - r#"["a","b"]"#, - ), // EXTRACT, not just UPDATE. ( &br#"{"a":{"b":1}}"#[..], @@ -100042,6 +100078,17 @@ mod tests { ] { assert_eq!(outputs(doc, filter), [want], "{filter}"); } + // A *fan-out* body declines the register entirely (review): a + // nested pipe would see it while a sibling branch of the same body + // does not, and that half-success fabricated a write. jq emits + // `["a","b"]` here and then raises; succinctly refuses outright, + // with no prefix -- the safe side (`fold-body-fanout-declines` in + // the sweep). + query!(br#"{"a":{"b":1}}"#, r"path(foreach .a as $v (.; ($v | .b), 1; .))", + QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { + assert!(is_resolver_refusal(&e), "{}", e.message); + } + ); // Unchanged: a fold whose INIT is untracked *after a literal stage* // re-seeds its register from the ambient, so the marker is not // recognised at all -- pre-existing (`literal-then-fold-untracked-