Skip to content

morestack on x86 32-bit does not save edx nor floating point stack #7158

New issue

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

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

Already on GitHub? # to your account

Closed
nikomatsakis opened this issue Jun 15, 2013 · 7 comments
Closed
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@nikomatsakis
Copy link
Contributor

Right now, we return 64-bit values and doubles using the native convention, which is to use EDX:EAX and the floating stack respectively. morestack only saves/restores EAX, however. I've observed failures as a result, in particular the src/test/run-pass/auto-encode.rs test when executed with the default options (the failure goes away with RUST_MIN_STACK set to something appropriately high). Due to the unpredictable nature of stack switching, though, the failures tend to come and go.

(cc @brson)

@graydon
Copy link
Contributor

graydon commented Jun 15, 2013

Yeah, my best guess at the end of yesterday's adventure with auto-encode debugging was someone clobbering the register holding the high half of the v variable in a logging / io function. Calling any io function after loading v caused the failure. I'd believe stack switching / morestack is the culprit.

@nikomatsakis
Copy link
Contributor Author

This is currently manifesting on my system as extra::time::run_tests failing an assert_eq even though left and right are equal. I have a patch for EDX, not sure about the matter of double returns.

@nikomatsakis
Copy link
Contributor Author

Ah, I see that wikipedia states: "The x87 floating point registers ST0 to ST7 must be empty (popped or freed) when calling a new function, and ST1 to ST7 must be empty on exiting a function." We are definitely doing this wrong, but I'm not 100% sure of the correct X86 assembly to save the stack iff it is non-empty.

@nikomatsakis
Copy link
Contributor Author

I believe the correct thing to do is to use fstp, which will set a flag if st(0) had a value. We then must push the flags register too, I guess, and then conditionally do a fldp. Alternatively, maybe we can just use outpointers to return doubles :)

@nikomatsakis
Copy link
Contributor Author

I should add: the code for doubles subtle enough that I would like a test that reproduces it. I have not been able to produce one, although I do have a test that uses the #[fixed_stack_segment] annotation to ensure a stack switch when returning a double. This leads me to think that while we are technically violating the cdecl ABI, it may in fact be harmless for doubles (though I thought I saw errors that seems to be specifically caused by the floating point stack being corrupted, so maybe I just haven't found the right code path to trigger an error).

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jun 16, 2013
@emberian
Copy link
Member

emberian commented Aug 5, 2013

I think this is still relevant, @nikomatsakis?

@alexcrichton
Copy link
Member

In today's meeting we have decided to jettison segmented stacks. Due to this no longer being used for switching stacks, I believe that this issue is moot.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

4 participants