Skip to content

Prove admit() in ostd/src with spec modification#392

Open
Marsman1996 wants to merge 7 commits intoasterinas:mainfrom
Marsman1996:prove-admit-new
Open

Prove admit() in ostd/src with spec modification#392
Marsman1996 wants to merge 7 commits intoasterinas:mainfrom
Marsman1996:prove-admit-new

Conversation

@Marsman1996
Copy link
Copy Markdown
Collaborator

This time I add a spec modfication evaluation module and the agent drops the specs whose rating is lower than 7

@Marsman1996
Copy link
Copy Markdown
Collaborator Author

The slot.write_meta(metadata); is commented, is this right?

@Marsman1996 Marsman1996 marked this pull request as draft April 1, 2026 09:01
@Marsman1996
Copy link
Copy Markdown
Collaborator Author

Marsman1996 commented Apr 1, 2026

KVerus cannot prove the rest 9 admit() with high score spec modification.

@Marsman1996

This comment was marked as off-topic.

@Marsman1996 Marsman1996 marked this pull request as ready for review April 1, 2026 11:13
@Marsman1996 Marsman1996 requested a review from Copilot April 1, 2026 11:21

This comment was marked as outdated.

@rikosellic
Copy link
Copy Markdown
Collaborator

Reviewing this requires a deep understanding of the current MetaSlot model, which, unfortunately, I do not have yet. Need some time.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +474 to +480
let cur_idx = frame_to_index_spec(cur_mapped_pa);
let tracked slot_perm = EntryOwner::<KernelPtConfig>::placeholder_slot_perm(
cur_mapped_pa,
&*regions,
);
regions.slots.tracked_insert(cur_idx, slot_perm);
regions_before_map.frame_slot_perm_insert_preserves_inv(cur_idx, slot_perm, *regions);
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

This proof mints a slot_perm via EntryOwner::placeholder_slot_perm(...) and inserts it into regions.slots to satisfy item_slot_in_regions. placeholder_slot_perm is an axiomatically-created permission intended as a temporary placeholder, so using it here effectively bypasses the linear “slot permission” discipline and can make the verification unsound. Instead, thread the real slot permission for cur_mapped_pa into this function (e.g., via preconditions/arguments that provide the existing regions.slots[idx] permission) or restructure so the CursorMut::map precondition is met without fabricating permissions.

Copilot uses AI. Check for mistakes.
@SNoAnd
Copy link
Copy Markdown
Collaborator

SNoAnd commented Apr 2, 2026

I need to dig into this a little more, but my current thoughts are that the spec modifications look reasonable but some of the proofs are questionable. In particular the ones that use placeholder_slot_perm. placeholder_slot_perm was added to bridge a temporary gap in the tracking of the metadata slot permissions. I don't remember exactly why, but it should be deprecated.

write_metadata should not be commented, I'll look into that.

@SNoAnd
Copy link
Copy Markdown
Collaborator

SNoAnd commented Apr 8, 2026

I looked into write_meta, the issue was that it was causing a crash in verus so I'd temporarily commented out. I have a fix, which I'll merge and push tomorrow.

@Marsman1996
Copy link
Copy Markdown
Collaborator Author

I looked into write_meta, the issue was that it was causing a crash in verus so I'd temporarily commented out. I have a fix, which I'll merge and push tomorrow.

You can directly modify this PR if you want.

@Marsman1996 Marsman1996 added the AI-assist AI-aided proof or generation label Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-assist AI-aided proof or generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants