Skip to content

Conversation

kkjeer
Copy link
Contributor

@kkjeer kkjeer commented Jul 6, 2021

See issue #1107

This PR modifies BaseRange to be able to compare constant-sized ranges whose base expressions have the same incomplete type.

Consider source bounds bounds(p, p + a) and target bounds bounds(q, q + b), where a and b are constants. If p and q have types T1 and T2, where T1 and T2 are incomplete types, then we cannot normalize a and b by multiplying them by the size of the referent types of T1 and T2. However, we can prove that bounds(p, p + a) imply bounds(q, q + b) if T1 == T2 and a >= b.

@@ -1046,24 +1048,29 @@ namespace {
llvm::APSInt UpperOffsetConstant;
Expr *LowerOffsetVariable;
Expr *UpperOffsetVariable;
bool LowerConstantIncomplete;
bool UpperConstantIncomplete;
Copy link
Contributor

Choose a reason for hiding this comment

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

It may suffice to have just one boolean field IsConstantIncomplete.

In CreateBaseRange, we get LowerKind (returned by SplitIntoBaseAndOffset on Lower) and UpperKind (returned by SplitIntoBaseAndOffset on Upper) Then, IsConstantIncomplete is set to true if either LowerKind or UpperKind is IncompleteConstantSized.

If we do this, the function CheckIncompleteConstantOffsets will not be required (this is causing the function EqualTypes to be called twice unnecessarily). Then we can directly call CheckIncompleteOffset.

int f34(_Array_ptr<struct S> p : bounds(p, p), _Array_ptr<struct S> q : count(0)) { // expected-note {{(expanded) declared bounds are 'bounds(p, p)'}}
// The upper bound q + 0 is an incomplete constant sized upper offset,
// but the upper bound p is not an incomplete constant sized upper offset.
p = q; // expected-warning {{cannot prove declared bounds for 'p' are valid after assignment}} \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the expected warning on line 310 in this test case is not really correct. The lower/upper bounds of p are not marked as Lower/UpperConstantIncomplete because of the way these fields are set. Currently, these fields are set only when the call to GetReferentSizeInChars fails. In the case of the bounds expression (p, p), I guess the control is going to the exit label in SplitIntoBaseAndOffset where the lower and upper bounds are split as p + 0 without calling GetReferentSizeInChars, and therefore the incompleteness of the type of p is not recorded.

I suggest the following:
Try checking if the type of Base is incomplete in the code under the exit label in SplitIntoBaseAndOffset. If the type is incomplete, return Kind::IncompleteConstantSized. If this gets further complicated or causes more issues/failures, file an issue for this test case and adding a TODO here with a reference to the filed issue.

Ideally, a BaseRange object must be marked as having an incomplete type if the type of the base (in this case p) is incomplete.

Copy link
Contributor

@sulekhark sulekhark left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link

@mgrang mgrang left a comment

Choose a reason for hiding this comment

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

LGTM.

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