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

ADT_C sorry-free #685

Merged
merged 7 commits into from
Oct 23, 2023
Merged

ADT_C sorry-free #685

merged 7 commits into from
Oct 23, 2023

Conversation

michaelmcinerney
Copy link
Contributor

@michaelmcinerney michaelmcinerney commented Oct 19, 2023

This removes all the sorries in the file ADT_C, thereby significantly increasing our confidence that the state relation is correct.

This also removes the budget field of scheduling contexts in the executable specification (and Haskell kernel). This brings the executable spec closer to the C code and shifts the condition that the budget is always the sum of all refills into the abstract refinement proofs where the corresponding invariants are more easily available.

This modifies active_sc_valid_refills to additionally state
that an inactive scheduling context has zero budget

Signed-off-by: Michael McInerney <[email protected]>
@michaelmcinerney michaelmcinerney added the MCS related to `rt` branch and mixed-criticality systems label Oct 19, 2023
@michaelmcinerney michaelmcinerney self-assigned this Oct 19, 2023
@Xaphiosis
Copy link
Member

Xaphiosis commented Oct 20, 2023

I think it looks mostly fine, I'm confused by "rt crefine: sorries related to cpspace_refill_relation_unique" ... that commit proves some things, changes a def or two, but doesn't introduce any new sorries... what am I missing?

Won't manage any deeper than that this week, but can look at the contents more next week.

@michaelmcinerney
Copy link
Contributor Author

I think it looks mostly fine, I'm confused by "rt crefine: sorries related to cpspace_refill_relation_unique" ... that commit proves some things, changes a def or two, but doesn't introduce any new sorries... what am I missing?

That commit clears sorries related to cpspace_refill_relation_unique. The sorrying run finished in the previous PR. Should I change the commit to say clear sorries... instead?

Copy link
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

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

Excellent, this is a big chunk of additional work and brings a lot more confidence to the state relation and proof setup. Also aligns C and spec more closely, which is always good.

Only some minor style nitpicks in the comments that would be good to fix up before merging. For the commit message Raf mentioned, clear sorries .. is a good idea and would work.

@Xaphiosis
Copy link
Member

I think it looks mostly fine, I'm confused by "rt crefine: sorries related to cpspace_refill_relation_unique" ... that commit proves some things, changes a def or two, but doesn't introduce any new sorries... what am I missing?

That commit clears sorries related to cpspace_refill_relation_unique. The sorrying run finished in the previous PR. Should I change the commit to say clear sorries... instead?

Let me look at the history for a sec so I don't give bad advice... OK, Gerwin does say things like aarch64 ainvs: asid_map sorries in ArchFinalise but when he does so, he does not leave it as just one line, following it up with Close some of the more hairy asid_map-related sorries in ArchFinalise_AI. There's no standard for what we do with sorries, I've seen "prove" (I like that), "clear" (when there are no more left?), "reduce" (when generally reducing sorries in a file).

TLDR: Yes please, something like rt crefine: clear sorries related to cpspace_refill_relation_unique (as you suggested) if there are no more of these left, and rt crefine: prove sorries related to cpspace_refill_relation_unique if there might be leftovers.

proof/invariant-abstract/DetSchedInvs_AI.thy Show resolved Hide resolved
Comment on lines +668 to +672
"\<And>P. \<lbrace>\<lambda>s. (\<forall>p. pred_map (\<lambda>cfg. \<not> active_scrc cfg) (sc_refill_cfgs_of s) p
\<longrightarrow> pred_map (\<lambda>cfg. scrc_budget cfg = 0) (sc_refill_cfgs_of s) p)
\<and> I s\<rbrace>
f \<lbrace>\<lambda>_ s. \<forall>p. pred_map (\<lambda>cfg. \<not> active_scrc cfg) (sc_refill_cfgs_of s) p
\<longrightarrow> pred_map (\<lambda>cfg. scrc_budget cfg = 0) (sc_refill_cfgs_of s) p\<rbrace>"
Copy link
Member

Choose a reason for hiding this comment

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

P isn't used here.

Copy link
Member

Choose a reason for hiding this comment

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

Well spotted, we're deferring fixing that one up to a later PR.

proof/crefine/RISCV64/Refine_C.thy Show resolved Hide resolved
@michaelmcinerney michaelmcinerney merged commit a6cd4cb into rt Oct 23, 2023
11 checks passed
@michaelmcinerney michaelmcinerney deleted the michaelm-adt_c_sorry_free branch October 23, 2023 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MCS related to `rt` branch and mixed-criticality systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants