Fix #355: bound parser recursion depth — nested input raises parse_error by petlenz · Pull Request #409 · NumSim-Stack/numsim-cas

petlenz · 2026-07-24T21:48:34Z

Summary

Fixes #355. Stacked on #408.

~20k-deep nested input ((((...1...))), nested sin(, or a unary-minus chain) segfaulted the process — PEGTL's recursive descent has no depth control, and the crash also fired on the error path (20k unclosed parens crashed while reporting them).

Design

A pre-scan in parse() bounds every recursion driver — bracket nesting depth and unary-minus run length — at 512, throwing syntax_error with the offending position. This is the cheap-and-total variant from the issue; a PEGTL custom control could count actual rule depth but adds plumbing for no practical gain (both reproduced crash shapes are covered, and 512 is far beyond hand-written or generated physics expressions). Whitespace doesn't reset a minus run (- - -x recurses per minus). The domain wrappers route through parse() and inherit the guard.

Tests

Lock-in: 20k parens (balanced + unclosed) and 20k minuses all raise parse_error; 200-deep still parses. Full suite: 2450/2450 pass.

int64 arithmetic in scalar_number wrapped silently (UB): pow(10,30) folded to 5076944270305263616 (10^30 mod 2^64), rat_add/rat_sub cross products were signed-overflow UB at ~2^40 magnitudes (UBSan-confirmed), rat_div could construct an invariant-violating 1/0 rational, and normalize_rational/negation hit UB at INT64_MIN. All int64 paths are now overflow-checked (builtin overflow intrinsics on GCC/Clang, manual range checks on MSVC) and demote to double when the exact value does not fit — value stays correct, exactness is lost only at the extremes. rat_div routes zero denominators to +-inf double like the int/int path, and INT64_MIN operands demote instead of negating. pow needs no change: it composes operator*, so the checked multiply fixes the wraparound. Exact behavior in range is unchanged and lock-in tested (1/3 + 1/6 stays the exact rational 1/2). Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

to_rational promotes int64 operands without normalization, so scalar_number(INT64_MIN) * scalar_number(1,2) reached std::abs(INT64_MIN) in rat_mul's gcd cross-cancel (UBSan-confirmed abort) - the exact class #349 eliminates elsewhere. rat_mul (and rat_div through it) now demotes to double when any component is INT64_MIN, matching the existing normalize_rational guard. Regression test covers mul both ways and div. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

pow's repeated-squaring negated the exponent with -n before the uint64 cast - signed-negation UB for INT64_MIN (round-2 review; UBSan abort on pow(1/2, INT64_MIN)). |n| is now computed as 0u - unsigned(n). Also repairs conflict markers committed during the stack rebase. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…nstants The generic hash_combine did static_cast<std::size_t>(value): formal UB for any negative double (every negative constant hashes on first comparison) and gross truncation - all fractions in (0,1) collided with 0.0. Doubles now hash via std::bit_cast<uint64_t> (with -0.0 normalized), and scalar_number gets a value-normalizing hash_combine overload so numerically equal constants keep hashing equal across variant alternatives (int64 2 == double 2.0 == rational 2/1 - whole doubles hash as their integer). scalar_constant::update_hash_value routes through it; without the normalization, expressions built from int and double spellings of the same constant stop canceling (caught by ScalarDifferentiationAudit). Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…malizer The value-normalizing hash cast the double to int64 in the FIRST conjunct, before the range checks - UB for NaN, inf, and |x| >= 2^63 (UBSan float-cast-overflow abort at scalar_number.h:117, the very class this PR removes elsewhere). NaN/inf now short-circuit via the range comparisons (false for NaN) before any cast. Regression test hashes 1e300/NaN/inf constants. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

I4_ijkl = delta_ik*delta_jl has major symmetry only: swapping the pairs (ij)<->(kl) gives delta_ki*delta_lj (equal), but swapping i<->j gives delta_jk*delta_il (a different tensor - the minor-symmetric identity is P_sym). The MinorMajor tag propagated through negation and scalar-mul and routed inv() evaluation through the symmetric 6x6 Voigt path, so inv(-I4) evaluated to -0.25 at component (0,1,0,1) instead of -1; is_symmetric/is_minor_major also misreported, mis-selecting the D4 symmetrizer in the rank-4 inv-diff kernels (cf. #283, #299). space_for_rank now tags rank 4 as Major. The three tests pinning the MinorMajor annotation are updated (Rank4IdentityIsMajorOnly plus the move/copy preservation pair); a numeric lock-in evaluates inv(-I4) through the Major path. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

Updates the set_symmetric no-op comment (the MinorMajor early-return it cited is gone; the rank gate is the actual reason at rank 4) and repairs a conflict-resolution artifact that glued a test's closing brace into a trailing comment, leaving the test namespace open. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

tensor_rebuild_visitor::apply restored the source expression's space() onto any rebuilt result lacking one (the #93 fix for variadic-ctor reconstruction dropping post-construction annotations). Sound for pure rebuilds, wrong when a subclass swapped children: substitute( trans(A)-A, trans(A), C) returned C-A tagged Skew, after which sym() folded it to zero and skew() returned it unchanged - every consumer of the tag (projector guards, trans, inv, diff kernels) trusted it. The restore is now gated on structural equality (m_result == expr, honest since #339), with one deliberate exception: projector contractions (skew(X), sym(X), ...) restore across argument substitution because their space is derived from the projector, not the argument - substituting inside skew(A) keeps Skew (the original deterministic #93 reproducer stays green). Structure-derived tags (trans(X)-X) are re-derived by construction when the substituted form still qualifies. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

The projector-restore exception compared only the projectors; a substitution that changed the argument's rank or dim could restore a rank-2-style tag onto a shape-inconsistent result. Rank and dim must match before any restore. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

The node-level dim guard was inert: inner_product_wrapper reports the projector's dim, so substitute(sym(A_dim3), A, E_dim2) passed the guard, restored the stale Symmetric tag onto a shape-broken node, and evaluation ran the projector at dim 3 over a dim-2 buffer (ASan-confirmed heap-buffer-overflow read). The guard now compares the actual contraction arguments' rank and dim. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…nd shape Round 3 proved the round-2 commit closed only the tag restoration: the heap overflow it cited lived in eval_projector_unary, which drives the unary wrapper with the PROJECTOR's dim and never compares the operand's - substitute(sym(A_dim3), A, E_dim2) still over-read the dim-2 buffer at evaluation (UBSan vptr abort). The short-circuit now throws evaluation_error on operand dim/rank mismatch, and the regression test evaluates the reproducer instead of only checking the tag. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

constant_mul::dispatch(tensor_to_scalar_mul) handled a numeric LHS by merging it into the coefficient, but a non-numeric scalar_wrapper LHS fell through the same path: the factor was never inserted and the coefficient was reset to the wrapped default, so wrapper(x) * (trace(A)*det(A)) evaluated as if x were 1. The promoted route (plain x * (f*g)) goes through mul_base and was correct, which is why tests missed it - the bug fires whenever the wrapper is the visitor's LHS. Non-numeric wrappers are now inserted as factors via push_or_combine (merging with an existing wrapper child through the unwrap-multiply- rewrap path). Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

A single-shot find/combine threw 'duplicate child insertion' when the combined factor collided with another existing child (w(x)*w(x) -> w(x^2) meeting a stored w(x^2)) - the same pattern merge_or_insert_mul already loops on. Pre-existing hole; #354's new route made it easier to reach. Regression test builds the chained collision explicitly. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

The tensor_inner_product_to_scalar evaluator always computed the plain tmech::dcontract(l, r): dot_product(A,{1,2},B,{2,1}) (= A_ij B_ji) silently evaluated as A : B, and rank-1 full contractions threw 'requires rank 2' even though the node is a legal dot product (which also broke evaluating derivatives of dot() on rank-1 arguments). tensor_data_dcontract_wrapper now receives both sequences: matching rank-2 sequences contract plain ({2,1}/{2,1} sums the same pairs), mismatched ones contract against the transpose, and rank-1 uses tmech::dot. Rank>2 general contraction stays a clear not-implemented error (tracked by the #383 evaluation-ceilings epic) instead of a silently wrong value. The identity half of this node (hash/== ignoring the sequences) landed in #399. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

The dispatch derives Dim/Rank from the LHS; a mixed-rank node (constructible through the weak || precondition in dot_product, #360) reached a wrong-type static_cast and, with the new rank-1 branch, returned silent garbage where it previously threw. The wrapper now throws evaluation_error on operand rank/dim mismatch and on sequence sizes not covering the rank. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…ng negative powers tensor_pow was broken beyond positive-integer/rank-2 use, each layer failing differently and silently: the evaluator cast the exponent with static_cast<int> (pow(X, 0.5) truncated to the identity) and looped k < abs(n) with no inversion (pow(X, -1) evaluated to X itself); both diff visitors' product-rule loops never ran for n < 0, silently coercing the tangent to zero; and the factory accepted any rank while hard-coding a rank-2 identity for pow(C4, 0). - Factory: rank-2 gate and integer-constant exponent gate (invalid_expression_error; fractional powers belong to the isotropic function API, #227). - Evaluator: non-integer exponent value throws evaluation_error; n < 0 inverts the accumulated power via tmech::inv. - Both diff visitors: n < 0 throws not_implemented_error pointing at the inv(pow(A, n)) spelling, which differentiates correctly today. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…nstants - The rank gate was factory-only: tensor_rebuild_visitor recreates tensor_pow ungated, so substitute(pow(X,2), X, C_rank4) evaluated into heap corruption (rank-6 contraction written into a rank-4 buffer). The evaluator now throws on rank != 2. - The exponent round-check gains a range bound so the int cast stays defined for huge/inf exponent values (would abort under the fatal UBSan leg). - The factory's integer gate now also sees negation-wrapped constants (pow(X, neg(0.5)) previously slipped to evaluation). Regression test covers the substitution route and the wrapped exponent. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

… bound The factory's integer gate unwrapped only one scalar_negative layer, so pow(X, neg(neg(0.5))) constructed and failed only at evaluation; the gate now strips any negation depth before the literal check, matching try_int_constant. The evaluator's exponent bound tightens from 1e9 to 1e6: the cast was defined at 1e9 but admitted a ~4-minute single-expression evaluation. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

PEGTL parses by C++ recursion with no depth control: ~20k nested parens, 20k nested calls, or a 20k unary-minus chain overflowed the stack (SIGSEGV) instead of raising parse_error - including on the error path (20k unclosed parens crashed while trying to report the missing parens). The parser is exactly the component fed untrusted text and every other malformed input produces a catchable error. parse() now pre-scans the input and rejects bracket nesting or unary-minus runs deeper than 512 with syntax_error (position at the offending character). Whitespace does not reset a minus run ('- - -x' recurses per minus). The wrappers (parse_scalar/tensor/t2s) route through parse() and inherit the guard. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

…uard The lock-in test was appended after the NUMSIM_CAS_PARSER_ENABLED block, breaking every build with the parser disabled. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

Two probe-confirmed bypasses of the new depth guard: the ^ chain is right-recursive in the grammar (50k carets still overflowed the stack), and the minus-run reset treated \v/\f as run breakers while PEGTL's space rule accepts them ('-\v' x 50000 crashed). Every ^ now counts toward the cap regardless of position, and the run persists across the full PEGTL space set. Lock-ins cover both bypasses plus a 100-caret happy path. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

The total-caret cap rejected 600 independent shallow powers (x1^2*x2^2*...) even though each ^ opens a fresh one-level chain - probe-confirmed false positive. The run now resets on any chain-breaking operator/bracket, keeping the 20k-chained-carets rejection while accepting arbitrarily many shallow powers. Lock-in covers both. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

Two CI jobs were structurally unable to fail on the defect class they exist to catch: GCC UBSan recovers by default (prints and exits 0, so the sanitizer leg only enforced the ASan half), and clang-tidy exits 0 on warnings with WarningsAsErrors unset. - Sanitizer blocks (library + parser) add -fno-sanitize-recover= undefined. Verified locally: the full suite passes under fatal ASan+UBSan (the real UB this would have caught was fixed in #349 and #361 first). - The clang-tidy workflow adds --warnings-as-errors='*', and the 15-warning baseline is fixed in the same change (no-op std::move on const-ref args and a trivially-copyable variant, missing override on two rebuild-visitor dtors, std::move on a forwarding reference, a cloned constexpr branch merged, a value param made const-ref, and a NOLINT that sat on the wrong line), so the job starts green. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

Two probe-confirmed bypasses survived the per-feature caps: bracketed exponents continue the ^-chain (1^(1)^(1)... - the chain-reset fix re-opened this shape, SIGSEGV from ~13k), and compound payloads multiply independent counters along the nesting path (300 minuses per level x 200 levels, every counter under 512, SIGSEGV). The guard now keeps one cumulative budget along the parse path: brackets push the current runs (they stay on the stack while inside), ^ and unary-minus runs add at the current level, a close restores the ^-chain (x^(y)^z is one chain) and ends the minus chain, and binary operators reset both. Shallow-caret products, bracketed-exponent chains under the cap, and 600-term minus chains all stay legal (locked in); both payload shapes now throw parse_error. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

Fix #356: CI can now fail on UBSan findings and clang-tidy warnings

petlenz mentioned this pull request Jul 25, 2026

Fix #356: CI can now fail on UBSan findings and clang-tidy warnings #410

Merged

petlenz force-pushed the fix-350-tensor-pow-contract branch from d0f0dd4 to c7756aa Compare July 25, 2026 12:38

petlenz force-pushed the fix-355-parser-depth-guard branch from 09ea20e to 2f73cea Compare July 25, 2026 12:41

petlenz force-pushed the fix-350-tensor-pow-contract branch from c7756aa to 74296dc Compare July 25, 2026 13:43

petlenz force-pushed the fix-355-parser-depth-guard branch from f16d5b1 to 82cfae0 Compare July 25, 2026 13:46

petlenz force-pushed the fix-350-tensor-pow-contract branch from 74296dc to 7558c86 Compare July 25, 2026 14:16

petlenz force-pushed the fix-355-parser-depth-guard branch from 82cfae0 to 82f5e3c Compare July 25, 2026 14:16

petlenz force-pushed the fix-350-tensor-pow-contract branch from 7558c86 to fcb5cac Compare July 25, 2026 22:33

petlenz force-pushed the fix-355-parser-depth-guard branch 2 times, most recently from 4b502a0 to 731764b Compare July 26, 2026 12:14

petlenz force-pushed the fix-350-tensor-pow-contract branch from fcb5cac to b0e62eb Compare July 26, 2026 12:14

petlenz force-pushed the fix-355-parser-depth-guard branch 3 times, most recently from d416613 to 5f27d2d Compare July 26, 2026 17:42

petlenz force-pushed the fix-350-tensor-pow-contract branch 2 times, most recently from 1d32d14 to 7a2fa03 Compare July 26, 2026 18:16

petlenz force-pushed the fix-355-parser-depth-guard branch 2 times, most recently from 77d47ae to c6c06f6 Compare July 26, 2026 18:40

petlenz force-pushed the fix-350-tensor-pow-contract branch from 7a2fa03 to 679576e Compare July 26, 2026 18:40

petlenz added 11 commits July 26, 2026 20:58

petlenz added 13 commits July 26, 2026 20:58

Fixup #355: ParserDepthGuard test belongs inside the parser-enabled g…

78e1b29

…uard The lock-in test was appended after the NUMSIM_CAS_PARSER_ENABLED block, breaking every build with the parser disabled. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>

petlenz force-pushed the fix-350-tensor-pow-contract branch from 679576e to 9fa3067 Compare July 26, 2026 19:03

petlenz force-pushed the fix-355-parser-depth-guard branch from c6c06f6 to a4f840a Compare July 26, 2026 19:03

Merge pull request #410 from NumSim-Stack/fix-356-ci-teeth

29514c3

Fix #356: CI can now fail on UBSan findings and clang-tidy warnings

petlenz changed the base branch from fix-350-tensor-pow-contract to main September 15, 2026 20:05

petlenz merged commit a5f5ce8 into main Sep 15, 2026
26 checks passed

petlenz deleted the fix-355-parser-depth-guard branch September 15, 2026 20:20

Navigation Menu

Fix #355: bound parser recursion depth — nested input raises parse_error - #409

Fix #355: bound parser recursion depth — nested input raises parse_error#409
petlenz merged 25 commits into
mainfrom
fix-355-parser-depth-guard

petlenz commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

petlenz commented Jul 24, 2026

Summary

Design

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant