Repository navigation
verify: Bound pointers into a record by the array, field, or record they point into - #53
Merged
Merged
Conversation
…unds of that subobject.
… offset, and keep bound companions out of origins.
…records of a pointee with their bounds.
…nter inside its bounds.
…ure an origin in its own elements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #50, the fields and fixed-array elements of a record reached through a pointer can be read and written (
r->buf[i]). A pointer into such a record was still an error, so common forms could not be verified:mainfirst(r->buf, 8)a pointer to the array member 'buf' is unsupportedfor (const int *q = r->buf; q != r->buf + 8; q = q + 1)&r->buf[i],&r->head,&r->innerraw address-of is unsupported until lexical lifetime and escape effects are modeled&a[2]The second message also gave the wrong reason: the pointee is the caller's storage, so lexical lifetime has nothing to do with it.
All of these are supported now:
Design
A pointer into a record may address only the subobject it points into. That is the array it decays from or whose element it addresses, or else the field or record. A check against the object alone would accept
q[8]above, because it isr->head, inside*r.How the references handle this:
index_boundfors->a[i](plugins/rte/src/rte.ml:71-83). But onceq = s->a, it checks only\valid(q + i)against the whole object, soq[8]passes there.This PR combines them: the bounds are narrowed where the pointer is formed, as in CHERI, carried with the pointer, as in a slice, and proved instead of checked at run time.
Forming the pointer.
ASTConverter::subobjectPointerlowers the pointer.Inside a statement (a declaration, an assignment, a call argument, a return), it first asserts what C++ requires for the address to exist:
&r->buf[i]needsi < 8, whiler->buf + 8is fine;It then binds the address to a fresh pointer,
__subobject_N, with two ghost companions,.__loand.__hi. They hold the first byte and the end of the subobject.In a condition or a contract, the pointer is only a value to compare, so it is the address alone.
Spreading the bounds.
propagateSubobjectBounds(a new pass,Transform/SubobjectBounds.cpp) runs on the converted body:c ? r->buf : r->otherselects the bounds like the value);q == 0 || q.__lo == 0 || q % S == q.__lo % S. Like the origin analysis's own invariants, it is proved, not assumed. It keeps a stepped pointer on element boundaries, which the subobject's bounds need: the origin analysis's alignment invariant applies only when the pointer has its origin's type, andint *into aRingdoes not.Returning a bounded pointer, or storing it in memory, would lose the bounds, so it is a conversion error.
Checking the bounds:
lo <= addr < hiat each access, andlo <= p <= hiat each step.&r->head - r->bufis an error, whileq - r->bufwithqinbufis fine.emitCallStmt): the bytes a callee's extent or single object takes from a bounded argument must lie in the bounds. Sofirst(r->buf, 9)fails, as doesget(r->buf + 8).Bugs this uncovered, fixed here because the new pointers expose them
r + 32is a sum of two pointers. The origin analysis and the step rule did not treat it as a step fromr.it[1].v) lost its bound check and verified.steppedPointer(Origins.cpp) reads the constant in a sum of two pointers as the offset, keeping a global's literal address a pointer. It is shared by the origin analysis,pointerOrigins,pointerOriginTerm, and the bounds pass.int *, rather than the origin object's, 40 bytes forRing. That held only while every pointer with originrhadr's type.q - r->buffailed, a wrong failure.PassivizerImpl::OriginObjectsgivesoriginPositionSafetyeach parameter's own object.Tests
suite/subobject_pointers.cpphas three runs.set, a nested record passed to a function over it;q == r->buf + i, and aforloop comparing with the end and measured byq - r->buf;modifies(r->buf[0 : 8]), and acppverify::checkthrough the pointer.buf, and 4 frombuf + 5;q[8], which isr->head;q[6]from&buf[2];q[1]from&r->head, which isr->inner.x;q[4]of a row, which is the next row;it[1]pastitems;q[-1]beforeother, which is the last item;otherside of a choice; a walk one element too far;get(r->buf + 8), which reads the end;&r->buf[i]withi == 8; a null root;otheroutside the frame.r->bufis formed, and their copy intoq.&r->buf[2], with locations.pointer_subobjects.cpp(#50) no longer expects the decay error for a member through a pointer; its global cases stay.Docs
q[8]), the C++ rules for forming, steps, differences and comparisons, and the escape error as a rejected example.Checked
On
mainat 573c171 (with #52 and #43):Verify lit suite: 431 passed and 1 unsupported, on both the release build and an assertions build. This includes the docs tests.
VerifyTests: 60 passed, 1 skipped.
On
main: all three RUN lines ofsubobject_pointers.cppfail;mainrejects the file's subobject pointers with the decay and address-of errors.Verdict differential against
main: 439 test sources. Onlysubobject_pointers.cppdiffers, and no file's time changed (3510 s → 3466 s in total). The changes to origins and pointer-difference positions alter no existing verdict.Mutation: 21 mutants; 20 are killed by the tests:
The
before_memberandpass_endcases were added for mutants that first survived. The 21st mutant lets the bound companions have origins. It is equivalent, since no verdict can depend on it; the exclusion keeps the companions' "any byte" sentinel, 2^64, from being read by the origin analysis as a global's address.Docs:
check-doc-examples.pypasses on every page this changes (62 examples, 13 against the verdicts their pages show).Merges: this merges cleanly with each open PR, verify: Make chooses with one condition one choice, kept across writes elsewhere #44, verify: Send crash reports to cpp-verify's issue tracker #46, verify: Bound what a run builds and what its solvers allocate #48, and docs: Publish the documentation of each release, and of main at /dev/ #49.