Skip to content

Fix nesting too deep - #401

Open
stringhandler wants to merge 1 commit into
BlockstreamResearch:masterfrom
stringhandler:st-nesting-too-deep
Open

Fix nesting too deep#401
stringhandler wants to merge 1 commit into
BlockstreamResearch:masterfrom
stringhandler:st-nesting-too-deep

Conversation

@stringhandler

Copy link
Copy Markdown
Contributor

Fixes #399.

Deeply nested input overflowed the stack and aborted the process instead of producing an error, making any service that compiles .simf files trivially killable.

@apoelstra

Copy link
Copy Markdown
Contributor

lmao debug builds are using 80kb of stack per frame? I think we need to fix that.

A recursion depth limit of 16 is far too low. I'd prefer no limit at all. Is the recursion inherent to chumsky or can we fix it within our own code.

@stringhandler

Copy link
Copy Markdown
Contributor Author

Yeah, this is a concerning bug though, as wallets will potentially be compiling code. I agree that 16 is too low. Let me dig in and see if I can find out what is taking up so much space.

@apoelstra

Copy link
Copy Markdown
Contributor

The compiler shouldn't be recursing here. In Expression::parser we take pains to pass the expr variable into the sub-parsers and use that rather than calling Expression::parser again.

We should investigate where the actual stack overflow comes from.

apoelstra added a commit that referenced this pull request Sep 4, 2026
e2530af parse: parse block bodies in linear time (stringhandler)
73751e4 tests: measure how parse time scales with nesting depth (stringhandler)

Pull request description:

  In an attempt to increase the nesting limit in #401, an LLM found this incredibly subtle bug in statements parsing.
  
  It's difficult to explain why this works, basically it would match a block body, then fail and parse each statement again. I've left the extraneous descriptions in the commits, but happy to remove them.
  
  There are two commits. 
  Commit 1 adds an ignored test that shows the problem. Run it with `cargo test --test parser_scaling -- --ignored --nocapture`
  Commit 2 adds the fix that converts it from exponential to linear. Run the test again afterwards to see the improvement.
  
  Here are some figures from my computer:
  
  ```
  running 1 test
  
    nesting construct, time per depth, and growth across two levels
  
    blocks
      depth   8     4.85ms       -
      depth  10    16.86ms   3.48x
      depth  12    62.02ms   3.68x
      depth  14   243.23ms   3.92x
      depth  16   989.35ms   4.07x
  
  ....
  ...
  
    depth 16 against depth 8 (linear is about 2x, doubling per level is about 256x)
      blocks            204.1x  TOO STEEP
      parentheses         1.3x  ok
      option types        1.6x  ok
      tuple types         1.4x  ok
      array types         1.4x  ok
   
  ```
  
  After
  ```
  running 1 test
  
    nesting construct, time per depth, and growth across two levels
  
    blocks
      depth   8     1.24ms       -
      depth  10     1.33ms   1.07x
      depth  12     1.26ms   0.95x
      depth  14     1.32ms   1.05x
      depth  16     1.30ms   0.98x
  ```


ACKs for top commit:
  apoelstra:
    ACK e2530af; successfully ran local tests


Tree-SHA512: 7b37aace675dcb5d2cbb2430c92f65041d43222b56c58eb4bb6e2662e1e421714ec149330d9a183fdca6abfd6d43d32f49f396f6710d7ef4c979d515fb5acb2f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recursive Parser has no limit, leading to stack overflow on deeply nested inputs

2 participants