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

Prove three of the four assumptions in parsePath #316

Closed
wants to merge 171 commits into from

Conversation

Dspil
Copy link
Contributor

@Dspil Dspil commented Apr 15, 2024

No description provided.

@jcp19
Copy link
Contributor

jcp19 commented May 9, 2024

To guarantee that this does not fall out of sync with the main code, we should soon merge master here and try to merge this on master soon. I will add few preliminary comments in case that helps

Comment on lines +88 to +99
// @ requires len(raw) >= HopLen
// @ preserves acc(h)
// @ preserves acc(slices.AbsSlice_Bytes(raw, 0, HopLen), R46)
// @ ensures h.ConsIngress >= 0
// @ ensures h.ConsEgress >= 0
// @ ensures let h1 := BytesToIO_HF(raw, 0, 0, HopLen) in
// @ let h2 := h.ToIO_HF() in
// @ h1.InIF2 == h2.InIF2 &&
// @ h1.EgIF2 == h2.EgIF2
// @ decreases
// @ outline(
// @ unfold acc(slices.AbsSlice_Bytes(raw, 0, HopLen), R46)
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering all the effort that went into making Gobra/silicon faster in the last few months, I think that these outline blocks are now unnecessary because

  1. this package is not problematic in the first place
  2. we can afford it now, even if this is ever so slightly slower (which probably is not the case anymore), and the annotation overhead is not worth it

requires acc(slices.AbsSlice_Bytes(raw, start, end), _)
decreases
pure func BytesToIO_HF(raw [] byte, start int, middle int, end int) (io.IO_HF) {
return unfolding acc(slices.AbsSlice_Bytes(raw, start, end), _) in BytesToIO_HF_Helper(raw, start, middle, end)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know that you still will refine the code, but as a heads-up, please try to keep lines shorter (e.g., by introducing line-breaks after in)

decreases
pure func BytesToIO_HF(raw [] byte, start int, middle int, end int) (io.IO_HF) {
pure func BytesToIO_HF_Helper(raw [] byte, start int, middle int, end int) (io.IO_HF) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These names are getting more and more unidiomatic :D I think calling these functions BytesToHfSpec and BytesToHfSpecUnfolded would be fine

Comment on lines 1974 to 1976
// TemporaryAssumeForIO(slayers.ValidPktMetaHdr(ub))
// TemporaryAssumeForIO(len(absPkt(dp, ub).CurrSeg.Future) > 0)
// TemporaryAssumeForIO(p.EqAbsHopField(absPkt(dp, ub)))
Copy link
Contributor

Choose a reason for hiding this comment

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

If you haven't started the proof for the fourth assumption, maybe it is a good idea to do that on a separate PR, so that we can keep the PRs small and easy to review

let idx := int(s.PathMeta.CurrHF) in
let hopOffset := MetaLen + s.NumINF*path.InfoLen + idx*path.HopLen in
let _ := Assuming(idx >= 0) in // Gobra can't figure this out from the type system at the moment
Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully, this will not be a problem soon because I intend to fix the overflow checking of Gobra soon and to make it generate the necessary obligations here.

At any rate, I would like to avoid introduce assumptions in pure functions due to their "global" nature. I would rather have a precondition in this method requiring s.PathMeta.CurrHF to be non-negative and introducing an assumption in the caller (if it is a non-pure function) or a precondition (if it is a pure function). I think that opening a precedent where we introduce assumptions in the bodies of pure functions is a bad idea

ensures len(absPkt(dp, ub).CurrSeg.Future) > 0
ensures p.EqAbsHopField(absPkt(dp, ub))
decreases
func (p *scionPacketProcessor) CurrentHopFieldBytesLemma(dp io.DataPlaneSpec , ub []byte, ubPath []byte)
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably don't need dp in this function anymore, and in a few other functions that you introduced in this PR (check #341)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, is this the only proof obligation that is missing?

Comment on lines +137 to +141

ghost
ensures b
decreases
pure func Assuming(b bool) bool
Copy link
Contributor

Choose a reason for hiding this comment

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

I would drop this function for the reasons stated above

ensures s.absPktFutureBytePosition(dp, raw)
decreases
func (s *Raw) absPktFutureBytePositionLemma(dp io.DataPlaneSpec, raw []byte) (res io.IO_pkt2) {
_ := reveal validPktMetaHdr(raw)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_ := reveal validPktMetaHdr(raw)
reveal validPktMetaHdr(raw)

@jcp19 jcp19 changed the title eqabshopfield lemma Prove three of the four assumptions in parsePath May 9, 2024
@jcp19
Copy link
Contributor

jcp19 commented May 20, 2024

This PR is superseded by PR #345

@jcp19 jcp19 closed this May 20, 2024
@jcp19 jcp19 deleted the dspil_eqabshopfield branch May 20, 2024 12:14
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