-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang][analyzer] Extend lifetime of dynamic extent information #163562
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
Changes from 1 commit
8c8277f
b7d9cc0
0318773
946fe1a
9b2d370
d384997
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -381,30 +381,12 @@ int *symbolicExtent(int arg) { | |
| return 0; | ||
| int *mem = (int*)malloc(arg); | ||
|
|
||
| // TODO: without the following reference to 'arg', the analyzer would discard | ||
| // the range information about (the symbolic value of) 'arg'. This is | ||
| // incorrect because while the variable itself is inaccessible, it becomes | ||
| // the symbolic extent of 'mem', so we still want to reason about its | ||
| // potential values. | ||
| (void)arg; | ||
|
Comment on lines
-384
to
-389
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there other similar void casts remaining int the test set that could be cleaned up?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not find more in the "ArrayBound" tests. Other tests may contain similar casts but this is more difficult to find.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This is why I was curious to see how many of the casts in other test could be dropped after this change. This is not too simple, but with search and replacing with some regex might make it doable if you apply the replacement per file and add the change to the staging area if all the clang tests still pass. (And log what files didn't work out of the box so you could check them manually)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be done of course separately from this patch.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked the Analysis tests but did not find similar casts (for purpose of keeping constraints of dynamic extent), just one place (that is changed in the last commit). |
||
|
|
||
| mem[8] = -2; | ||
| // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} | ||
| // expected-note@-2 {{Access of 'int' element in the heap area at index 8}} | ||
| return mem; | ||
| } | ||
|
|
||
| int *symbolicExtentDiscardedRangeInfo(int arg) { | ||
| // This is a copy of the case 'symbolicExtent' without the '(void)arg' hack. | ||
| // TODO: if the analyzer can detect the out-of-bounds access within this | ||
| // testcase, then remove this and the `(void)arg` hack from `symbolicExtent`. | ||
| if (arg >= 5) | ||
| return 0; | ||
| int *mem = (int*)malloc(arg); | ||
| mem[8] = -2; | ||
| return mem; | ||
| } | ||
|
|
||
| void symbolicIndex(int arg) { | ||
| // expected-note@+2 {{Assuming 'arg' is >= 12}} | ||
| // expected-note@+1 {{Taking true branch}} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps rename this to
transferLivenessToDynamicExtentor something similar now that we no longer mark all the dynamic extent values as live?