Commit 7dda1203 authored by Matthew Fernandez's avatar Matthew Fernandez
Browse files

fix: push setjmp calls into rules/guards/etc

The motivation for this change is to fix an error in our usage of sigsetjmp().
When using jmp_bufs (JMP_BUF_NEEDED), we use sigsetjmp() and siglongjmp() to
give us an exception-handling-like mechanism to jump back to the exploration
loop after signalling an error. This pattern is fine except that these calls are
documented to leave all non-volatile locals in an indeterminate state. We had
several of these that were important (e.g. the pointer to the state that we go
on to free).

My initial planned solution to this was to simply mark the relevant variables
volatile. However, this comes with some drawbacks. Unconditionally marking these
volatile impedes the compiler's optimiser in the case when we're not using
jmp_bufs, while conditionally marking them volatile overcomplicates the code
generation logic. To further complicate this, some of the relevant variables are
generated (ruleset iterators). We would have to cast away these variables'
volatility when passing them to rules which would introduce even further
complications.

Instead, we duplicate the sigsetjmp() calls and move them inwards. E.g. for
guards, we call sigsetjmp() as the first step in the guard itself and then use
the return value of the guard to indicate to the exploration loop whether
siglongjmp() was called. The advantage of this is that the only work done in the
siglongjmp() path is now returning from the containing function; there are no
longer any relevant non-volatile locals.

This had a couple of unanticipated side effects:

  1. The error call in case of a deadlock had to be moved into its own function
     to also avoid having any non-volatile locals. This is not a problem, just
     unexpected.
  2. Return statements now awkwardly return a boolean when they have no
     associated expression. Relatedly void-returning functions (procedures) now
     have a boolean return type. This is because a return statement can be used
     in a rule (which now returns a boolean). We could have done something more
     elaborate like have an empty return statement jump to the end of the
     rule/function, but it seemed this would be more likely to confuse the
     compiler.

Github: closes #127 "--max-errors > 1 produces unsafe code"
parent 43e8d651
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment