Unnecessary "Control stack overflow"

[guest]

I am not terribly familiar with the Hugs source code, but I believe I have found a bug relating to the interaction between the global variable evalDepth in machine.c and the implementation of exception handling.

It seems that evalDepth is used to keep track of the recursion depth of the eval function; the first thing eval does is increment evalDepth and the last thing it does is decrement it. If evalDepth ever gets too large, Hugs generates a "Control stack overflow".

Now consider the function evalWithNoError (defined in machine.c), which is called by the primitive primCatchException to evaluate an expression which might cause an exception.

Cell evalWithNoError(e)                /* Evaluate expression, returning */
Cell e; {                              /* NIL if successful, */
    Cell caughtEx;                     /* Exception value if not... */
    jmp_buf *oldCatch = evalError;

    jmp_buf catcherr[1];
    evalError = catcherr;
    if (setjmp(catcherr[0])==0) {
      eval(e);
      caughtEx = NIL;
    }
    else {
      caughtEx = exception;
    }

    evalError = oldCatch;
    return caughtEx;
}

Notice that setjmp is called to save the state; if eval happens to invoke the primitive throwException (which calls longjmp) then the second branch of the of the conditional is taken, and an exception is returned. The problem is that in such a situation, by calling longjmp, the eval function exits WITHOUT decrementing evalDepth. As far as I can tell, setjmp/longjmp does not save/restore evalDepth. The result is that Hugs often generates "Control stack overflow" when evaluating expressions that throw and catch lots of exceptions. I've attached an example of such a program -- this program causes no problems at all when run using GHC.

By manually saving the evalDepth before calling setjmp and restoring it when longjmp is called, the problem can be fixed. Here is the new definition of evalWithNoError.

Cell evalWithNoError(e)                /* Evaluate expression, returning */
Cell e; {                              /* NIL if successful, */
    Cell caughtEx;                     /* Exception value if not... */
    jmp_buf *oldCatch = evalError;
    int oldEvalDepth;                  /* ADDED BY ME */
    oldEvalDepth = evalDepth;          /* ADDED BY ME */

    jmp_buf catcherr[1];
    evalError = catcherr;
    if (setjmp(catcherr[0])==0) {
      eval(e);
      caughtEx = NIL;
    }
    else {
      evalDepth = oldEvalDepth;        /* ADDED BY ME */
      caughtEx = exception;
    }

    evalError = oldCatch;
    return caughtEx;
}

Of course, I do not know if this is a genuine fix or a broken hack. In particular, I worry about other side-effects of eval that are not undone when an exception is caught. It is not obvious to me why setjmp and longjmp are required at all --- why not model an exception as a normal-form and simply inspect it after an eval?

Is there a regression test-suite that I can try --- to gain confidence that the proposed fix does not break anything?

Matthew Naylor (mfn@cs.york.ac.uk).

[ross]

Patch applied, thanks.

If exceptions were treated as special normal forms, we'd have to test for them all over the evaluation logic.