Fill in some missing cases for bounds-safe interface assignments#813
Conversation
…interface type of LValueToRValue casts and pointer arithmetic
… type of pointer dereferences and array subscripts
| /// deference or an array subscript. For rvalue expressions appearing as | ||
| /// part of the left-hand side of an assignment, only lvalue-to-rvalue casts | ||
| /// and pointer arithmetic have bounds-safe interfaces. | ||
| QualType Sema::GetCheckedCRValueInteropType(ExprResult RHS) { |
There was a problem hiding this comment.
In addition to returning the bounds-safe interface type for *e and p +/- i, should this method also return the bounds-safe interface type for &e? If e has bounds-safe interface type T, should &e have bounds-safe interface type ptr<T>?
As an example, making this change would allow the following to compile:
void address_of(int *interop_ptr : itype(ptr<int>), ptr<int> checked_ptr) {
*(&interop_ptr) = checked_ptr;
}
Currently, this example does not compile since &interop_ptr has an unchecked pointer type int **. If GetCheckedCRValueInteropType were to return the bounds-safe interface type for address-of expressions, then &interop_ptr would have a bounds-safe interface type of ptr<ptr<int>> and so *(&interop_ptr) would have a bounds-safe interface type of ptr<int>.
…into issue-810-bounds-safe-interfaces
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
Overall, looks good. I have a few suggestions. I also suggest that the issue you raised by treated separately from this initial change. I have some feedback on the issue that you raised.
| // If the LHS is a checked nt array type and the RHS is an unchecked | ||
| // nt array type with a bounds-safe interface, use the bounds-safe | ||
| // interface of the RHS to check the types. | ||
| if ((LHSType->isNtCheckedArrayType() || |
There was a problem hiding this comment.
Is the test isNtCheckedArrayType needed? I don't think C allows you to make assignments to values with array type.
| IsParam = isa<ParmVarDecl>(Var); | ||
| } | ||
| } | ||
| // If `e` has bounds-safe interface type with referent type T, then |
There was a problem hiding this comment.
I think the wording on this comment is somewhat unclear. It's subtle to say that e has a referent type. How about e has a bounds-safe interface type that is a pointer to T?
| // If `e1` has bounds-safe interface type with referent type T and `e2` | ||
| // is an integer, then `e1[e2]` and `e2[e1`] have bounds-safe interface | ||
| // type T. | ||
| else if (ArraySubscriptExpr *Array = dyn_cast<ArraySubscriptExpr>(LHSExpr)) { |
There was a problem hiding this comment.
I think the same change applies here.
| @@ -10565,6 +10565,10 @@ class Sema { | |||
| /// Returns a null QualType if there isn't one. | |||
| QualType GetCheckedCInteropType(ExprResult LHS); | |||
There was a problem hiding this comment.
Does this need a clarifying comment? Or to be renamed?
…into issue-810-bounds-safe-interfaces
2 participants
This PR addresses issue #810 by filling in two missing cases for bounds-safe interface assignment compatibility:
Testing: