Skip to content

[pointer] Document validity safety invariant #2403

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

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Feb 28, 2025

Document the semantics of Ptr validity invariants on the Validity
trait.

Makes progress on #2226, #1866


This PR is on branch ptr-validity.

@joshlf
Copy link
Member Author

joshlf commented Feb 28, 2025

This draft lays the groundwork for implementing the semantics described in the commit message of #2397. As of this writing, all I've added is a safety comment on the Validity trait. This is clearly insufficient on its own - in order to be completely sound, there must be places where this invariant is consumed. (It needs to be proven to be upheld as well, but that's more obvious.) My hunch is that this will need to be at places where we actually do something with the raw pointer inside a Ptr (read from it, convert it to a vanilla reference, etc).

Update: On further reflection, I think we will need the following:

  1. We document that validity provides a guarantee regarding the set of values that may appear in a Ptr's referent (already done)
  2. We consume this guarantee wherever a Ptr's raw pointer is dereferenced or converted into a vanilla reference
  3. We document that unsafe code is responsible for upholding this guarantee on any Ptr, including Ptrs that it produces (already done)
  4. We document specific necessary conditions that must be satisfied in order for this guarantee to be upheld (specifically, the conditions under which the validity set may not be expanded or shrunk). This technically does not add any new information (it's implicit in the original guarantee from (1)), but it's useful to write it down so we don't forget it. As a result of being implicit, it's not explicitly consumed or discharged anywhere - but again, I think it's important for future readers since it's they are subtle requirements which are easy to miss.
  5. We discharge the general form of the safety obligation (from (1)) wherever unsafe code creates Ptrs or performs operations which affect what values can be written to the referents of existing Ptrs. This discharge will likely entail repeating the conditions mentioned in (4).

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.33%. Comparing base (5aa0bca) to head (9a5a4dd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2403      +/-   ##
==========================================
+ Coverage   87.21%   87.33%   +0.12%     
==========================================
  Files          17       17              
  Lines        6390     6451      +61     
==========================================
+ Hits         5573     5634      +61     
  Misses        817      817              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch 2 times, most recently from 2bf955a to 9a83b82 Compare February 28, 2025 08:20
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from 9a83b82 to 144daf1 Compare February 28, 2025 18:46
@joshlf joshlf changed the base branch from main to Id0bf66cfb1c6359fe0dffe40c9a44dbaca7ebc12 February 28, 2025 18:46
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch 2 times, most recently from f949fb7 to adf0e35 Compare February 28, 2025 18:54
@joshlf joshlf force-pushed the Id0bf66cfb1c6359fe0dffe40c9a44dbaca7ebc12 branch from 2ce57a9 to 163f7b9 Compare February 28, 2025 18:54
Base automatically changed from Id0bf66cfb1c6359fe0dffe40c9a44dbaca7ebc12 to main February 28, 2025 19:30
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch 2 times, most recently from 7c48ee7 to fec7673 Compare February 28, 2025 20:42
@joshlf joshlf changed the base branch from main to I0a3639a58c6161a41f9cbc200cd322cc136d77fd February 28, 2025 20:42
Base automatically changed from I0a3639a58c6161a41f9cbc200cd322cc136d77fd to main February 28, 2025 21:21
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from fec7673 to feb5ccf Compare February 28, 2025 21:25
@joshlf joshlf changed the base branch from main to I7694dcd7b9c01e25ac8ad9ee1915de16dbc00fdd February 28, 2025 21:25
Base automatically changed from I7694dcd7b9c01e25ac8ad9ee1915de16dbc00fdd to main February 28, 2025 22:03
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch 2 times, most recently from 03547a1 to 0c59508 Compare March 2, 2025 23:21
@joshlf joshlf changed the base branch from main to Iff896bf79205dd13645f7f21b0c7c3623dade98f March 2, 2025 23:21
Base automatically changed from Iff896bf79205dd13645f7f21b0c7c3623dade98f to main March 2, 2025 23:59
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch 2 times, most recently from daf3a21 to c559aad Compare March 5, 2025 20:04
Document the semantics of `Ptr` validity invariants on the `Validity`
trait.

Makes progress on #2226, #1866

gherrit-pr-id: Ie66db9044be1dc310a6b7280a73652a357878376
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from c559aad to 9a5a4dd Compare March 6, 2025 19:57
@joshlf joshlf added this pull request to the merge queue Mar 6, 2025
Merged via the queue into main with commit 52f65db Mar 6, 2025
87 checks passed
@joshlf joshlf deleted the Ie66db9044be1dc310a6b7280a73652a357878376 branch March 6, 2025 21:25
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.

3 participants