Handle thread escaping via global and after thread creation

[sim642]

Closes #246.

Lots of tests fail and the new ones don't even pass. Coincidentally, #687 fixes many of the failures: from 41 down to 9. And avoiding escapes in singlethreaded mode gets from 9 down to just the new 2 tests. All passing now!

Changes

1. Handle assigns to globals/other escaped variables also as escapes.
2. Collect assign escapes even in singlethreaded mode, but actually publish them when entering multithreaded mode. This avoids unnecessarily imprecise handling of those locals as globals while still single threaded.
3. Use per-escapee global unknowns to collect escapes after thread creation. The thread entry point depends on these, so the escape information can "flow backwards".

TODO

• Remove debug prints.
• Need transitive lookups of global unknowns? Added TODO.

[michael-schwarz]

Nice, looks good!

What I started wondering about is whether the collecting of things potentially escaped into globals already needs to be done during single-threaded mode as we currently do it here?

Intuitively, it would suffice to treat those things as escaped, that are still reachable from globals when we go multi-threaded, right?

It has the effect of making &y a global in the following example without the being really necessary.

#include <pthread.h>
#include <assert.h>

int* gptr;

void *foo(void* p){
    *gptr = 17;
    return NULL;
}

int main(){
    int x = 0;
    int y = 0;
    gptr = &y;
    gptr = &x;
    assert(x==0);
    pthread_t thread;
    pthread_create(&thread, NULL, foo, NULL);
    sleep(3);
    assert(x == 0); // UNKNOWN!
    assert(y == 0);
    y = 5;
    assert(y == 5);
    pthread_join(thread, NULL);
    return 0;
}

But on the other hand, I'm not sure if this does not just make it more complicated than it needs to be without much of a benefit, as we would still need all the things you have in place now to account for things escaping via globals after we went multi-threaded.

[sim642]

Intuitively, it would suffice to treat those things as escaped, that are still reachable from globals when we go multi-threaded, right?

That sounds right, but it's not so easy for the escape analysis to know, what exactly all those things are. It's the base privatizations that know and publish those in enter_multithreaded. Also it would be additionally sophisticated in combination with earlyglobs because they wouldn't even be iterable in the local state.

I agree, it'd be possible to be a bit more precise, but I suspect then essentially all the escape logic would have to be baked into base analysis itself. Unless there's particular need for that precision, we shouldn't unnecessarily overthink it for now.

[sim642]

This seems to have caused non-termination/extreme slowdown on zstd. I'll see tomorrow if I can find out why. Otherwise we'll have to have an option to disable these indirect escapes for zstd benchmarking, unless zstd actually does these weird things.

[sim642]

Turns out without this our zstd analysis is also unsound exactly due to this pattern here: https://github.com/facebook/zstd/blob/7543085013db1a20a848d166e5931edc49e3cc2f/lib/dictBuilder/cover.c#L1207.
The local struct best is written to a freshly allocated struct data, which escapes into a thread below via POOL_add.

I found this because pool.c isn't the only file in zstd containing mutexes, but even after removing locking from cover.c we don't get race warnings even though we should. And that's because the best struct doesn't even escape, so it's considered local in threads and no accesses are recorded.

[michael-schwarz]

Is this also unsound without your malloc freshness analysis? Maybe we can turn that off to salvage our benchmarks?

[sim642]

I have a fix for the extreme slowdown of threadEscape: it should only consider local variables for possible escaping! The analysis, even before this PR, didn't have that filtering.
But that doesn't fix the unsoundness, we still lose some known pointers somewhere, so the intended accesses don't happen.

Yes, it's also unsound without symb_locks, var_eq and mallocFresh, so it's been unsound all along. And it's suspicious indeed that the basic analysis before all my precision attempts had a few thousand lines of dead code.

[michael-schwarz]

So the problem clearly is that this best struct does not escape?
Or is is that we lose the pointers that are used to access it?

These seem to be two different issues at least to me?

[sim642]

Without this PR, best doesn't even escape. With just this PR, the analysis is too slow that I haven't even waited for termination. With an additional fix that filters escapes to locals, it does terminate reasonably and best escapes.

But even then we're unsound and it seems to be because the function pointer gets lost somewhere. So indeed that's the new issue I'm looking into, but that only showed after fixing the termination of this PR.

[michael-schwarz]

With an additional fix that filters escapes to locals

Fixed by @sim642 in 7e03cec on master