Matching tests to the new logic on handling empty/invalid ranges #393
Conversation
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
I have a few suggestions for improvements and a question about why a new flag was added to some command lines.
|
|
||
| // assignment through pointer | ||
| *t1 = 1; // expected-error {{expression has unknown bounds}} | ||
| *t2 = 2; // expected-warning {{out-of-bounds memory access}} |
There was a problem hiding this comment.
Could you file a GitHub issue for this? We should give an error when assign a non-zero value to the upper bound of an nt_array_ptr.
| // The following lines are for the clang automated test suite. | ||
| // | ||
| // RUN: %clang %s -o %t1 -DTEST_READ -Werror -Wno-unused-value -Wno-check-memory-accesses %checkedc_target_flags | ||
| // RUN: %clang %s -o %t1 -DTEST_READ -Werror -Wno-unused-value -Wno-check-memory-accesses -Wno-check-bounds-decls-unchecked-scope %checkedc_target_flags |
There was a problem hiding this comment.
I think the flag -Wno-check-memory-accesses can be deleted from all of these command lines. I am surprised you had to add -Wno-check-bounds-decls-unchecked-scope. Were there specific lines in the file that triggered this warning?
There was a problem hiding this comment.
The -Wno-check-bounds-decls-unchecked-scope was needed because in cases that we see an invalid range declaration, we give a warning and this flag ignores that warning. For example, for the declaration at line 226 of deref.c
There was a problem hiding this comment.
Yes, -Wno-check-memory-accesses was for out-of-bounds memory access warnings.
I removed them.
| // The following lines are for the clang automated test suite. | ||
| // | ||
| // RUN: %clang %S/deref.c -o %t1 -DTEST_READ -Werror -Wno-unused-value -Wno-check-memory-accesses -O3 %checkedc_target_flags | ||
| // RUN: %clang %S/deref.c -o %t1 -DTEST_READ -Werror -Wno-unused-value -Wno-check-memory-accesses -Wno-check-bounds-decls-unchecked-scope -O3 %checkedc_target_flags |
There was a problem hiding this comment.
The comments for deref.c apply to this file too.
saeednj
left a comment
saeednj
left a comment
There was a problem hiding this comment.
Thanks for the suggestions.
The added flag was for checking invalid range declarations.
This is to update the tests to match the new handling of empty and invalid ranges. The corresponding code for the new handling is in this pull-request.