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

[BoundsWidening] Use invertibility to support bounds widening in loops #1137

Merged
merged 3 commits into from
Jul 27, 2021

Conversation

mgrang
Copy link

@mgrang mgrang commented Jul 26, 2021

An expression that modifies an LValue is said to be invertible w.r.t. the
LValue if we can write an expression in terms of the original value of the
LValue before the modification. For example, the expression x + 1 is invertible
w.r.t x because we can write this expression in terms of the original value of
x which is (x - 1) + 1.

In this PR, we use invertibility of statements to support bounds widening in
loops. More specifically, if a statement modifies a variable that occurs in the
bounds expression of a null-terminated array then instead of killing its bounds
at that statement we use invertibility of the statement to try to write the
widened bounds in terms of the original value of the variable.

Mandeep Singh Grang added 2 commits July 26, 2021 14:18
An expression that modifies an LValue is said to be invertible w.r.t. the
LValue if we can write an expression in terms of the original value of the
LValue before the modification. For example, the expression x + 1 is invertible
w.r.t x because we can write this expression in terms of the original value of
x which is (x - 1) + 1.

In this PR, we use invertibility of statements to support bounds widening in
loops. More specifically, if a statement modifies a variable that occurs in the
bounds expression of a null-terminated array then instead of killing its bounds
at that statement we use invertibility of the statement to try to write the
widened bounds in terms of the original value of the variable.
// set InOfLastStmt to StmtOut.
// InOfCurrStmt contains the Out set of the second last statement of the
// block. This is equal to the In set for the last statement of this
// block. So we set InOfLastStmt to StmtOut.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The comment on lines 100 - 102 can now be further simplified.

// a bounds expression which is strictly wider than the declared upper
// bound.
// So we will proceed only if AdjustedRangeBounds is wider than
// StmtOut[V] which contains the delcared bounds of V at this point. For
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: delcared -> declared

clang/lib/Sema/BoundsWideningAnalysis.cpp Show resolved Hide resolved
// statement. If the statement is invertible then store the statement, the
// modified LValue, the original LValue and the set of null-terminated arrays
// whose bounds are affected by the statement. We will use this info in the
// computation of the Out sets of blocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: ...computation of the Out sets of blocks -> ...computation of the Out sets of statements.

@@ -300,16 +300,14 @@ void assign1(array_ptr<int> arr : count(1)) { // expected-note {{(expanded) decl
// Assignment to a variable used in other variables' bounds
void assign2(
array_ptr<int> a : count(len - 1), // expected-note {{(expanded) declared bounds are 'bounds(a, a + len - 1)'}}
char b nt_checked[0] : count(len), // expected-note {{(expanded) declared bounds are 'bounds(b, b + len)'}}
char b nt_checked[0] : count(len),
Copy link
Contributor

Choose a reason for hiding this comment

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

This change on line 303 and lines 311 and 312 do not seem to be caused by the implementation of invertibility. It will be great if you could give some context for this change.

Copy link
Author

Choose a reason for hiding this comment

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

These changes are a direct result of invertibility. b is a null-terminated array which uses len in its bounds and len is modified. Without invertibility we would have killed the bounds of b and issued the warning/note messages. But with invertibility we can "adjust" the bounds of b to bounds(b, b + len - 3).

Copy link
Author

Choose a reason for hiding this comment

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

Filed #1138 to track the invalid declaration of nt_checked array of size 0.

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!

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.

2 participants