ChezScheme icon indicating copy to clipboard operation
ChezScheme copied to clipboard

avoid stack overflow on fasl-read

Open mflatt opened this issue 5 years ago • 2 comments

This patch changes fasl-write to avoid generating a fasl content that can cause fasl-read to crash.

The fasl-read function can crash because it's implemented in the kernel using recursive C calls, which can exhaust the C stack. The change to fasl-write detects when nesting has gone 500 layers deep, and for each vale in the next layer, it adds a graph reference to the root of the fasl content. The graph reference has the effect of resetting nesting. (Most of the patch is propagating a depth counter through the graph-construction phase of fasl-write.)

mflatt avatar Dec 29 '18 12:12 mflatt

Have you considered modifying the fasl reader to maintain an explicit stack? It seems like any limit, including 500, could result in a stack overflow depending on the C run-time system.

dybvig avatar Jan 04 '19 05:01 dybvig

Hand-rolling a continuation in C for the fasl reader looked more painful than I was willing to try. Unlike a traversal of an existing tree, say, where the continuation can just be a stack of nodes, the stack for fasl reading would need to include elements like "install the result value into code object X based on the relocation at offset Y, then continue with the next relocation". (I've had to do that in other contexts, of course.)

The current Chez Scheme's boot files trigger fasl-read recursion at a depth of 50. Without this patch, i was running into trouble with uses at a depth around 3000 in a non-main thread. I picked 500 as a value that seemed unlikely to be reached often but still well within practical C stack sizes. Either a smaller or configurable depth would make sense, though.

mflatt avatar Jan 04 '19 13:01 mflatt

Included in v9.9.9 merge 47daa9b34016de84fd111801d9d589d15a523abe.

mflatt avatar Oct 17 '23 17:10 mflatt