experiment: remove volatile from read.c and stream.c

[fingolfin]

In PR #490 we were wondering why these volatile keywords are there.
Let's find out by removing them, and testing the result...

[ChrisJefferson]

I think I found where these might have come from. Quoting the C standard:

All accessible objects have values, and all other components of the abstract machine
have state, as of the time the longjmp function was called, except that the values of
objects of automatic storage duration that are local to the function containing the
invocation of the corresponding setjmp macro that do not have volatile-qualified type
and have been changed between the setjmp invocation and longjmp call are
indeterminate.

Therefore, technically any function which calls READ_ERROR (which is a macro which calls SySetjmp, which is a macro defined to be setjmp), have to have volatile on local variables.

[ChrisJefferson]

Turns out, this is a real problem too. Just when I think I've learnt all of C! Simplified from http://blog.sam.liddicott.com/2013/09/why-are-setjmp-volatile-hacks-still.html

#include <stdio.h>
#include <setjmp.h>

int main(void) {
  static jmp_buf j;
  volatile int x = 3;
  int y = 3;
  if (setjmp(j) == 1) {
    printf("%d %d\n", x, y);
    return 0;
  }
  y++; x++;
  longjmp(j, 1);
}

This prints 4,4 with no optimisation, but 4,3 with optimisation, so the non-volatile variable's value changes.

[fingolfin]

Urgh.

Some more setjmp related weirdness: We have a macro sySetjmp which on my system expands to sigsetjmp; but I noticed that the DEBUG_READ_ERROR version of READ_ERROR calls plain setjmp (the only place which is not using sySetjmp). Intentional?

I also see that FuncCALL_WITH_TIMEOUT and FuncCALL_WITH_CATCH use setjmp but don't declare any locals as volatile, yet we access several of them after setjmp returns with a non-zero value.

And in addition, apparently the standard says that using the result of setjmp in an arithmetic expression is unsage (see http://en.cppreference.com/w/c/program/setjmp). But in READ_ERROR we do TLS(NrError)+=sySetjmp(TLS(ReadJmpError)). I guess the correct way to do it would be this (using the fact that we only ever longjmp with a value of 1):

if (sySetjmp(TLS(ReadJmpError)) {
  TLS(NrError)++;
}

[ChrisJefferson]

I believe the setjmp is just an accident, can't imagine it being on purpose.

We should add more volatiles.

The switching to an 'if' looks good too.

Not sure if we are ever going to get bitten by this, looking at some functions, we are often safe and also after a longjmp we are usually in error mode anyway, so just want to get out. But it still feels best to follow the rules.