libexpr: Fix use-after-free of StaticEnv::up

[xokdvium]

Motivation

#12527
#11286

Context

I'm not very familiar with the ownership model here, so there might be some footguns with this solution. Please take a look at my reasoning in the commit message.

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[kjeremy]

This may fix the crash part of #12527 but does it fix the performance?

[xokdvium]

I was focused on fixing the correctness and getting rid of memory crimes.

Performance will definitely not improve with this one patch. At the very least it should not regress because refcount increments are way cheaper than allocating a new shared_ptr.

[edolstra]

Prior to 21071bf StaticEnvs were allocated on the stack and they only had to outlive the recursive bindVars() calls, so there was no risk of use-after-free. I'm not sure why the debugger needs them to be dynamically allocated.

[xokdvium]

so there was no risk of use-after-free.

But wouldn't that have lead to an access to a still invalid StaticEnv object that used to live in a stack-frame? Use-after-free in this case is just a symptom of a dangling pointer.

I stepped through the debugger trying to pinpoint why the up StaticEnv gets destroyed before it stops being referred to, but I only got a vague understanding.

ASAN log here #12527 (comment) shows where the lifetime started and ended, but that doesn't really help understand why up is dangling.

[Ericson2314]

I'm not sure why the debugger needs them to be dynamically allocated.

@xokdvium pointed me to the PR description of #5416

Normally StaticEnvs are thrown away after bindVars, so if there's an exception, there's no way to know what symbols are in scope.

[...]

This PR stores each StaticEnv in a shared_ptr instead of as a stack variable. In debug mode, the shared_ptrs are in turn stored in the Exprs. Then when an exception occurs in an Expr, it can refer to its StaticEnv to see what symbols are in scope.

[xokdvium]

From #5416:

During nix script execution, Values are stored in Env structs, and nix Exprs know the offsets needed to find these values. Those offsets originate in StaticEnvs during the bindVars phase, before execution happens. StaticEnvs contain symbols and their offsets.

This PR stores each StaticEnv in a shared_ptr instead of as a stack variable. In debug mode, the shared_ptrs are in turn stored in the Exprs. Then when an exception occurs in an Expr, it can refer to its StaticEnv to see what symbols are in scope.

@edolstra My best guess for why the StaticEnv dies prematurely is that no mapping in exprEnvs exists that shared ownership of the StaticEnv, so at the end of the bindVars newEnv dies, even though some other StaticEnv refers to it by a raw pointer. Hence why this can be reproduced with an empty let.

Sharing ownership of the parent StaticEnv with to its child ensures that it does not die prematurely.