Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: enable random stack relocation #44949

Open
JAicewizard opened this issue Mar 11, 2021 · 6 comments
Open

runtime: enable random stack relocation #44949

JAicewizard opened this issue Mar 11, 2021 · 6 comments

Comments

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Mar 11, 2021

It is not allowed to have C pointers into go memory, from my understanding, mostly because the go stack is movable.
The only reliable way to find bugs in this is currently just using manual review.

I propose that there should be a way to force the runtime to move the stack at random intervals.

This could be done entirely in runtime with a ENV variable, or a compile option due to runtime overhead.

context:
I am currently working on improving performance of a c->go transpiler (ccgo by @cznic, https://gitlab.com/cznic/ccgo) in this case by allowing some pointers to be *T instead of uintptr. It is a fine line between the 2, but in no circumstance should they ever be mixed because of the movable stack (no C pointer to go stack). ccgo runs all GCC tests to verify it translates properly. Getting all the tests to work isnt too complicated. However there might be cases where a uintptr points to go allocated object, and it just works because the stack wasn't moved. It is impossible to verify this doesn't happen, I would have to manually check 1300 GCC test, and preferably also some randomly generated code.
This is of course a unique situation, however I think this could also help less aware people find these problems.

This would of course not be able to verify the code, however, just like fuzzing, it means we can be much more confident in the code.

CC @josharian @ianlancetaylor @prattmic @aclements on recommendations of josh

@randall77
Copy link
Contributor

@randall77 randall77 commented Mar 11, 2021

How are you creating pointers into the stack that you're passing to C?
Escape analysis should move those allocations to the heap. Are you defeating escape analysis somehow?

@josharian
Copy link
Contributor

@josharian josharian commented Mar 11, 2021

I think the goal here is to help ensure that you aren't accidentally defeating escape analysis. Running in this mode helps generate crashes soon that would have happened anyway (similar to the race detector).

@randall77
Copy link
Contributor

@randall77 randall77 commented Mar 11, 2021

Defeating escape analysis how? Is it by casting to uintptr then passing to C?

That would also defeat GC, because it loses a reference to an object that might be the only one keeping it alive. We have a GODEBUG=clobberfree=1 mode to help catch those. Maybe we can expand clobberfree to also clobbber unreachable stack objects? That might be just as good for this case as copying whole stacks.

@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Mar 11, 2021

Yes that is how I do it, I dont pass it to C, the pointer stays doesnt leave go but you could call it "C land". The GCC tests are mostly very small, so they would probably not live long enough for GC to really have en effect. Same with simply globber-ing old stack, I never actually checked (and dont know how to) but I would suspect that the programs dont live long enough to have their stack moved.

Having clobberfree also globber moved stack objects would definitely aid in finding these bugs, if the stack actually moved.

@randall77
Copy link
Contributor

@randall77 randall77 commented Mar 11, 2021

Then I think there's a few things we can do here:

  1. Have clobberfree mode clobber the old stack whenever we copy.
  2. Have clobberfree mode clobber dead stack objects at every GC. This is a bit tricky, because stack objects can be live either by pointer (what we're talking about here) or by having direct uses (as offsets from SP). An object has to be dead in both senses before we can clobber it. That's computable given the liveness map for a frame, but might be somewhat difficult to plumb up in the runtime.
  3. Copy stacks more often / gratuitously when clobberfree is set (together with some other env variable?), which should enhance 1.

Number 1 should be easy. Number 2 is harder, but has the benefit of equalizing the behavior of heap objects and stack objects with respect to clobberfree.
For number 3, how much more often? At every GC is a convenient timer. If we do 2, then 3 doesn't help unless the frequency is more often than every GC.

@toothrot toothrot added this to the Backlog milestone Mar 11, 2021
@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Mar 11, 2021

I think a combination of 1 and 3 would be ideal. 2 would (from my outside view) be difficult, with minimal gain.

How often should option 3 run? As often as possible of course! :) I just tested a couple of tests in the ccgo test-suite, and they dont seem to generate enough garbage to trigger a GC. A couple ware triggered with GOGC=1 but this is still not a lot.

It would be 100x better already, but to be on the safe side I am going to say; preferably more often than once per GC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants