Skip to content

Bound nesting depth and run the compiler on its own stack - #42

Draft
Leitwolf11 wants to merge 1 commit into
mainfrom
fix/deep-nesting-limits
Draft

Leitwolf11 wants to merge 1 commit into
mainfrom
fix/deep-nesting-limits

Conversation

@Leitwolf11

Copy link
Copy Markdown
Contributor

The problem

Deep nesting ended vxs with a stack overflow (0xC00000FD on Windows) and no diagnostic. Reproduced separately on main:

Input Result on main Where it overflowed
else if chain of 2000 links crash Core::CorePrep::Prepare copies every function; the Statement copy constructor recursed about 1550 deep
if nested 100 levels deep crash Core::Wire::Decode, about 3 frames of several kilobytes per level
sum of 100 operands crash the same reader, per expression level
100 nested parentheses, conditional chain of 100 tests crash the same

So the second case was much more common than reported: any expression about 100 operands deep crashed on Windows, where a process starts with a one-megabyte stack.

What changes

  • The compiler runs on its own stack. vxs, vxsi, the source fuzz targets and the smoke program run the pipeline on a thread with 256 MiB of reserved stack (Visual/XSharp/Support/CompilerStack.hpp). How deep a program may nest no longer depends on the stack the operating system gives the process. The stack is reserved, not committed.
  • The frontend bounds nesting and says where. A function body that nests statements more than 256 levels deep reports VXP0039; one that nests expressions more than 1024 levels deep reports VXP0040. Each is reported once per function, at the first node that is too deep. An else if chain is not nesting. The body of a match arm counts one level per arm of its match, up to seventeen, which is how deep its lowering nests.
  • The native Core wire codec bounds statement nesting at 4096 levels, as it already bounded expression depth, so a .core file is bounded too.
  • The adapter no longer copies functions. Chains of 5000 links compile.

The specification and the syntax are unchanged.

Why these limits

Measured on the compiler stack, the native stages pass 4000 levels of statements and of expressions, at about 12 and 15 KiB of stack per level. The limits are far below that because sanitizer builds use more stack per level and because a lowering can add levels around a source level. They are also far above hand-written code; a generated sum of more than 1024 operands is the one realistic input that now gets a diagnostic.

Verification

Run locally on Windows:

  • cabal test all: five suites pass. NestingLimitTests.hs nests each construct to the limit and one level beyond, checks the diagnostic position and runs the accepted programs.
  • develop test: all 22 native suites pass. NestingLimitTests.cpp pins the wire limit and walks 1500 levels and a chain of 3000 links through the native Core stages.
  • source_fuzz_smoke: passes, including 255 nested if statements and a sum of 1024 operands in both pipeline modes.
  • develop sanitize address-undefined: 21 of 22 suites pass under ASan and UBSan, including the new tests. The 22nd, source_fuzz_smoke, is stopped by its 240-second watchdog on this machine. Run directly, the sanitized smoke finishes in 260 seconds with exit code 0 and no sanitizer report. I did not measure whether main exceeds the watchdog here as well; this branch removes more smoke runs than it adds.
  • Sanitized vxs check on 255 nested if statements, a sum of 1024 operands and a chain of 2000 links: accepted, no report.
  • Permanent seeds in Compiler/Fuzzing/Corpus: deep nesting, both limits and one level beyond each, long chains.

Not run locally: Linux and macOS (the POSIX half of CompilerStack.cpp is unbuilt here), clang-tidy, and the libFuzzer campaigns.

Performance

vxs check, 60 runs each, alternating with a build of main, mean of the best third:

Program main this branch
small program 101.0 ms 108.8 ms
300 sequential if statements 373.5 ms 372.6 ms
match of 80 arms 157.1 ms 160.3 ms
else if chain of 80 links 147.5 ms 151.0 ms
60 nested if statements 139.6 ms 144.5 ms

There is a constant cost of 3 to 8 ms per run. My first version cost about 15 to 20 ms: LLVM's thread helper commits the whole stack on Windows, so the thread is now created with STACK_SIZE_PARAM_IS_A_RESERVATION.

Not fixed here

Compile time grows faster than the program in three cases, all in the Haskell frontend and all present on main: an else if chain of 5000 links takes about a minute, 2000 sequential if statements about 4 seconds, and each level of nested loops multiplies the time by about 1.4, so 50 nested loops do not finish. The nesting limits do not bound the last case. The changelog lists these as known limitations.

Provisional language choices

Documents/BRANCHING.md gains a section that lists the choices #40 made where the specification is silent, with the basis of each, and marks them as provisional. Two of them disagree with existing text: the required comma after an expression arm is stricter than the grammar, and the immutable pattern binding differs from every other binding, which may be rebound unless it is final.

Deep nesting ended the compiler with a stack overflow and no diagnostic.
On the one-megabyte stack a Windows process starts with, a sum of 100
operands, 100 nested parentheses, an `if` nested 100 levels deep and an
`else if` chain of about 1500 links each overflowed.

Two causes, found with stack traces:

- The native stages after Core recurse once per level of nesting, and the
  frames of the Core wire reader are several kilobytes: about 80 levels
  fit in one megabyte.
- The Core-to-CorePrep adapter copied every function before preparing it.
  Copying nested statements recurses once per level, and an `else if`
  chain is one level per link.

Changes:

- Add Visual::XSharp::Support::RunOnCompilerStack and run `vxs`, `vxsi`,
  the source fuzz targets and the smoke program on a thread with 256 MiB
  of reserved stack. The stack is reserved, not committed: on Windows the
  thread is created with STACK_SIZE_PARAM_IS_A_RESERVATION, because the
  default would commit the whole size on every run.
- Add Visual.XSharp.NestingLimits to the frontend. A function body that
  nests statements more than 256 levels deep reports VXP0039 and one that
  nests expressions more than 1024 levels deep reports VXP0040, each once
  per function at the first node that is too deep. An `else if` chain is
  not nesting. The body of a match arm counts one level per arm of its
  match, up to seventeen, which is how deep its lowering nests.
- Bound the nesting of statement bodies in the native Core wire reader
  and writer at 4096 levels, as expression depth already was. The links
  of a chain share a level and an empty body costs none.
- Stop copying functions in the adapter: the work queue refers to the
  functions of the module, and lifted closures live in a deque.

Measured on the compiler stack, the native stages pass 4000 levels of
statements and of expressions. The limits leave room for sanitizer
builds and for the levels a lowering adds.

Verification

- NestingLimitTests.hs nests every construct to the limit and one level
  beyond, checks the position of the diagnostic and runs the accepted
  programs on both forms of Core.
- NestingLimitTests.cpp pins the wire limit and walks 1500 levels and a
  chain of 3000 links through the native Core stages on the compiler
  stack.
- BranchingExecutionCases.cpp runs 255 nested `if` statements and a sum
  of 1024 operands through both pipeline modes. To keep the smoke program
  within its watchdog under sanitizers, the wide match and the long chain
  now run on two subjects each.
- The source corpus gains seeds for deep nesting, for both limits and one
  level beyond each, and for long chains.

Compile time still grows faster than the program for long `else if`
chains, long statement sequences and nested loops; the changelog records
the measurements as known limitations. The specification is unchanged.
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.

1 participant