Environ shim

[pvdrz]

Remake of #1147. There are three main problems with this:

1. For some reason update_environ is not updating environ when setenv or unsetenv are called. Even then it works during initialization.
2. I am not deallocating the old array with the variables in update_environ.
3. I had to store the environ place into MemoryExtra as a field to update it. I was thinking about changing extern_statics to store places instead of AllocIDs to avoid this.

@RalfJung

Fixes #756

[RalfJung]

For some reason update_environ is not updating environ when setenv or unsetenv are called. Even then it works during initialization.

That is curious. Could you please add a testcase demonstrating the problem?

[RalfJung]

I had to store the environ place into MemoryExtra as a field to update it. I was thinking about changing extern_statics to store places instead of AllocIDs to avoid this.

No I think what you did makes sense. There's just multiple parties that care about this particular allocation.
It is crucial though that the field in MemoryExtra never gets changed after initialization; maybe add a comment along those lines.

[pvdrz]

That is curious. Could you please add a testcase demonstrating the problem?

sure, check 18f0339

[RalfJung]

What remains is to deallocate the old memory, and to add a test that make sure it got deallocated.

[bors]

☔ The latest upstream changes (presumably #1209) made this pull request unmergeable. Please resolve the merge conflicts.

[pvdrz]

add a test that make sure it got deallocated.

How can I do that?

[RalfJung]

Yeah it's a bit tricky... it has to be a compile-fail test I think.
I suggest you declare environ as an extern static in that file, then get the ptr once early and deref it to test if that works. Then change the env, and try re-using the old ptr, now Miri should error.

[pvdrz]

Yeah it's a bit tricky... it has to be a compile-fail test I think.
I suggest you declare environ as an extern static in that file, then get the ptr once early and deref it to test if that works. Then change the env, and try re-using the old ptr, now Miri should error.

That sounds really nasty... I like it

[RalfJung]

Looking great, thanks. :)

@bors r+

[bors]

📌 Commit 8392a0c has been approved by RalfJung

[bors]

⌛ Testing commit 8392a0c with merge 574d81c...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing 574d81c to master...

[RalfJung]

I just realized we already have the EnvVars struct for extra state for environment variables... @christianpoveda would you mind submitting a follow-up PR to move environ over to that struct?

[pvdrz]

sure