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

Atomic Types Challenge #82

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
- [4: Memory safety of BTreeMap's `btree::node` module](./challenges/0004-btree-node.md)
- [5: Verify functions iterating over inductive data type: `linked_list`](./challenges/0005-linked-list.md)
- [6: Safety of `NonNull`](./challenges/0006-nonnull.md)
- [7: Safety of Methods for Atomic Types and `ReentrantLock`](./challenges/0007-atomic-types.md)
- [8: Contracts for SmallSort](./challenges/0008-smallsort.md)
- [9: Safe abstractions for `core::time::Duration`](./challenges/0009-duration.md)
- [10: Memory safety of String](./challenges/0010-string.md)
Expand Down
83 changes: 83 additions & 0 deletions doc/src/challenges/0007-atomic-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Challenge 7: Safety of Methods for Atomic Types and `ReentrantLock`

- **Status:** Open
- **Tracking Issue:** [#83](https://github.com/model-checking/verify-rust-std/issues/83)
- **Start date:** *2024-09-09*
- **End date:** *2024-12-10*

-------------------

## Goal

Verify the safety of:
tautschnig marked this conversation as resolved.
Show resolved Hide resolved
- The methods for atomic types in `core::sync::atomic`
- The implementation of `ReentrantLock` in `std::sync::reentrant_lock`

### Success Criteria


#### Part 1: Atomic Types
carolynzech marked this conversation as resolved.
Show resolved Hide resolved

First, verify the safety of methods on atomic types.

Write safety contracts for each of the `from_ptr` methods:

- `AtomicBool::from_ptr`
- `AtomicPtr::from_ptr`
- `AtomicI8::from_ptr`
- `AtomicU8::from_ptr`
- `AtomicI16::from_ptr`
- `AtomicU16::from_ptr`
- `AtomicI32::from_ptr`
- `AtomicU32::from_ptr`
- `AtomicI64::from_ptr`
- `AtomicU64::from_ptr`
- `AtomicI128::from_ptr`
- `AtomicU128::from_ptr`

Specifically, encode the safety conditions marked `#Safety` in the comments above the methods.
carolynzech marked this conversation as resolved.
Show resolved Hide resolved

For the integer and boolean atomics, verify that these contracts hold on all bit-valid values. For `AtomicPtr`, verify that the contracts hold for pointees of size 0, 8, 16, 32, 64, 128, and at least one non-power-of-two.
tautschnig marked this conversation as resolved.
Show resolved Hide resolved

Then, write and verify safety contracts for the remaining unsafe functions:

- `atomic_store`
celinval marked this conversation as resolved.
Show resolved Hide resolved
- `atomic_load`
- `atomic_swap`
- `atomic_add`
- `atomic_sub`
- `atomic_compare_exchange`
- `atomic_compare_exchange_weak`
- `atomic_and`
- `atomic_nand`
- `atomic_or`
- `atomic_xor`
- `atomic_max`
- `atomic_umax`
- `atomic_umin`

These functions are wrappers around compiler intrinsics. Thus, your task is to ensure that we cannot cause undefined behavior by invoking the intrinsics on these inputs. For instance, if the intrinsic takes as input a raw pointer and reads the value at that pointer, your contracts should ensure that we would not cause UB by performing the read. For the purpose of this challenge, you may assume that any UB in the intrinsics would be a direct consequence of malformed inputs.
carolynzech marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this challenge should include writing contracts for those intrinsics and ensuring that the contracts are not violated by these methods.

Copy link
Author

@carolynzech carolynzech Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment. I can add intrinsics to this challenge as well, although we may want to be mindful of length. If we have contracts on the methods, intrinsics, and a meatier standard library application, this challenge will likely be the longest one we have. That's not necessarily bad, but I wonder if atomic intrinsics would be better as a separate challenge to keep the size of this one more manageable.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The methods are safe though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but we still want to verify unsafe code inside safe functions, no?


You are not required to write correctness contracts for these functions (e.g., that `atomic_sub` correctly subtracts the operands and stores the result), but it would be great to do so!

#### Part 2: Reentrant Lock

Verify that the `ReentrantLock` implementation in `std::sync::reentrant_lock` is safe. In particular, verify:

* That all uses of the methods on atomic types uphold the safety contracts you wrote in Part 1, and
* No other unsafe code in `ReentrantLock` causes UB or arithmetic overflow.
carolynzech marked this conversation as resolved.
Show resolved Hide resolved

## List of UBs

In addition to any properties called out as SAFETY comments in the source code, all proofs must automatically ensure the absence of the following [undefined behaviors](https://github.com/rust-lang/reference/blob/142b2ed77d33f37a9973772bd95e6144ed9dce43/src/behavior-considered-undefined.md):

* Data races.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that adding data races means that we cannot complete this challenge with Kani. I can take this out if we'd prefer to leave data races out of scope for this challenge. Not sure the feasibility of adding a tool later that can reason about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying CBMC supports concurrency, so perhaps Kani can do it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in principle we should keep this, since that is the main safety property of many safety guarantees offered by the type.

It's not clear to me what forms these proofs would take.

* Accessing (loading from or storing to) a place that is dangling or based on a misaligned pointer.
* Reading from uninitialized memory.
* Invoking undefined behavior via compiler intrinsics.
* Producing an invalid value.
* Breaking the aliasing or conversion rules of `UnsafeCell` (defined [here](https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html)).

Note: All solutions to verification challenges need to satisfy the criteria established in the [challenge book](../general-rules.md) in addition to the ones listed above.


carolynzech marked this conversation as resolved.
Show resolved Hide resolved
Loading