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

Jump to bottom

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
Labels
Debugging NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@JAicewizard
Copy link

JAicewizard commented Mar 11, 2021 •
edited by josharian

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

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

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

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

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

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 11, 2021
@toothrot toothrot added this to the Backlog milestone Mar 11, 2021
@JAicewizard
Copy link
Author

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.

@randall77 randall77 added NeedsFix The path to resolution is known, but the work has not been done. Debugging and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Debugging NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@toothrot @josharian @randall77 @JAicewizard