Skip to content
This repository has been archived by the owner on Jul 16, 2020. It is now read-only.

Potential fix for the stackrestore problem #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

haraldh
Copy link
Contributor

@haraldh haraldh commented Sep 25, 2019

Potential fix for the stackrestore problem

By coincindence llvm.stackrestore works on x86. The other architectures
suffer from the compiler not knowing to use the old stackpointer to
load the parameters for %func() and even the value for %func.

The correct handling would be to load those values in registers before
calling llvm.stackrestore and use the to call %func.

This workaround uses alloca to store and load them, which on some
architectures (tested on armv7l) works.

This pull request assumes a merged PR #41

jump.ll:
* declare all @llvm.* routines with the correct attributes
* use the correct parameter types in @jump_* functions
* add `returns_twice` to setjmp/jump_save calls

lib.rs: correct handling of `MaybeUninit`
* don't immediately call `assume_init()`
* use `as_mut_ptr()` as pointer to uninitialized `MaybeUninit`
* use `read_volatile()`/`write_volatile()` to let compiler re-read after `jump_swap()`
By coincindence llvm.stackrestore works on x86. The other architectures
suffer from the compiler not knowing to use the old stackpointer to
load the parameters for %func() and even the value for %func.

The correct handling would be to load those values in registers before
calling llvm.stackrestore and use the to call %func.

This workaround uses alloca to store and load them, which on some
architectures (tested on armv7l) works.
@haraldh haraldh changed the title Stackrestore Potential fix for the stackrestore problem Sep 25, 2019
@haraldh haraldh requested a review from npmccallum September 25, 2019 11:58
@npmccallum
Copy link
Collaborator

This doesn't actually increase the number of tests that pass.

@haraldh
Copy link
Contributor Author

haraldh commented Sep 25, 2019

Although it does not fix any tests, it passes on a native armv7l machine with this patch.

Also all test cases now fail with sigsegv instead of sigill. Might need an additional fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants