ares icon indicating copy to clipboard operation
ares copied to clipboard

OOM bug when building stack trace

Open ashelkovnykov opened this issue 1 year ago • 2 comments

There was a bug in the guard page implementations of #202 / #211 that would occur when if the NockStack ran out of memory during the copying procedure. @eamsden describes it fully in PR #217. That same PR patches this issue by checking whether or not the NockStack was in copy mode when an error occurred, and immediately popping a frame if so. This abandons the data on the current frame, of which only the stack trace data is valuable at that point. There is no way around this, as elaborated upon here.

However, there remains a bug in the code that can further corrupt the stack traces. The NockStack eagerly moves the alloc / stack pointers on allocation and relies on the guard page to catch OOM errors. As a result, it's possible that the pointers get moved past each other, far beyond the guard page. Therefore, the attempts of the exit() function to stitch together the stack traces into a $tang (which is also allocated on the NockStack) might clobber exiting data.

If the alloc and stack pointers were moved back to their initial positions and we were now allowed to ignore the guard page, then allocating new cells for returning a $tang of traces would be fine since we would have 16 KiB of room for allocating new cells.

ashelkovnykov avatar Mar 14 '24 17:03 ashelkovnykov

#213 also presumes that flog! is going to be called from within interpret() so that a guard page exists. It's another case where technically our usage of the NockStack is unsafe without an outside safety net.

Should we only explicitly document it, or should we bring back the now-abandoned checks for whether an allocation pushes the alloc / stack pointers past each other? Not sure what the performance cost would be to make that check on every allocation, but obviously non-zero.

ashelkovnykov avatar Mar 14 '24 17:03 ashelkovnykov

Explicitly document for now.

eamsden avatar Mar 20 '24 01:03 eamsden