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

issue #42 moving join and yield into stdlib #185

Closed
wants to merge 2 commits into from
Closed

issue #42 moving join and yield into stdlib #185

wants to merge 2 commits into from

Conversation

gmfawcett
Copy link
Contributor

Hi graydon,

Let me know what you think.

Best,
Graham

@graydon
Copy link
Contributor

graydon commented Dec 12, 2010

This is great. Sorry for the slow uptake; got side tracked the past week and this fell through the cracks. Can you file a contributor form with the foundation? It's just a blanket declaration of IP originator-ship on anything you contribute. Only has to be done once.

@gmfawcett
Copy link
Contributor Author

I'm glad you're happy with the patch!

I've just submitted a CA to Mozilla.

@graydon
Copy link
Contributor

graydon commented Jan 5, 2011

Hi,

We've received your committer agreement, thanks. A few points:

First, it's best to make a separate branch and rebase that branch forwards while it's waiting for me to integrate. As presented, this change requires me to cherry-pick just the commit in question, which is not ideal for history-keeping, and gets cumbersome with multiple commits on a branch.

Second, the change doesn't, unfortunately, work. It's close but there are a couple bugs. Aside from a new test being added since your patch (which needs updating to handle it), there's a bigger issue that the yield and join upcalls have ... quite intimate knowledge of the call stack that's calling them. If you try running 'make check' on linux with valgrind, you'll see the tests generate memory faults.

What's happening is that the upcalls eventually call task->yield(), which is very specific about having to know how many args are in its caller's frame, and assumes that its caller is a cross-stack call (rust-to-C). Since you're violating both of those assumptions by indirecting through builtin_yield and builtin_join, it doesn't work.

I've tried moving the code in question out of rust_upcall and into rust_builtin ("upcall" as a term we try to reserve for calls-the-compiler-synthesizes) and it .. works slightly better. But I still get valgrind errors. Not sure what else is going wrong.

I'll be around over the next few days if you want to diagnose this further, figure out what's up. Sorry, yielding (safely descheduling a task mid-call) is the most delicate code-path in the runtime. It's devilishly hard to get right.

@gmfawcett
Copy link
Contributor Author

Hey graydon,

Sorry for the very slow response, it's been a hectic season. Given these issues, please let's reject my pull request. If I can revisit this, and fix the bugs, I'll submit a new request in the spring. [Marking this request closed.]

oli-obk added a commit to oli-obk/rust that referenced this pull request Jul 19, 2017
dlrobertson pushed a commit to dlrobertson/rust that referenced this pull request Nov 29, 2018
Fix broken example in previous PR
ZuseZ4 referenced this pull request in EnzymeAD/rust Mar 7, 2023
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2023
carolynzech pushed a commit to carolynzech/rust that referenced this pull request Feb 19, 2025
This is a draft pull request towards solving #19. 

## Changes

- Added wrappers for `transmute_unchecked()` 
- Annotated these wrappers with contracts
- Wrote harnesses that verify these wrappers

Note: the reason we write wrappers for `transmute_unchecked()` and we
annotate those wrappers is that function contracts do not appear to be
currently supported for compiler intrinsics (as discussed in
[rust-lang#3345](model-checking/kani#3345)). Also,
rather than using a single wrapper for `transmute_unchecked()`, we write
several with different constraints on the input (since leaving the
function parameters completely generic severely restricts what we can do
in the contracts, e.g., testing for equality).

This is not intended to be a complete solution for verifying
`transmute_unchecked()`, but instead a proof of concept to see how
aligned this is with the expected solution. Any feedback would be
greatly appreciated -- thank you!

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.

---------

Co-authored-by: AlexLB99 <a6leblan@uwaterloo.ca>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants