Fix passing negative value as count to function returning counted_by - #13892
Fix passing negative value as count to function returning counted_by#13892pandaaured wants to merge 1 commit into
Conversation
Added error to case where a function returning `counted_by` is passed a negative value as a count to the function. Changed tests to correspond with correct program behavior and removed rdar://80808704 and rdar://103368466 comments from the tests which errored.
| Expr *CountE = ReturnCount.get(); | ||
| if (auto *OVE = dyn_cast<OpaqueValueExpr>(CountE)) | ||
| CountE = OVE->getSourceExpr(); | ||
| CountE = CountE->IgnoreParenCasts(); |
There was a problem hiding this comment.
Do paren casts prevent const eval? If so, this should probably be IgnoreParenImpCasts. Otherwise we should be able to skip it
There was a problem hiding this comment.
Would be worth trying to come up with a test that would show the behavioral change
| @@ -71,6 +71,6 @@ void call_param_with_shared_size(void) { | |||
| void *__sized_by(count * size) return_with_count_size(int count, int size); | |||
|
|
|||
| void call_return_with_count_size(void) { | |||
| // FIXME: rdar://103368466 | |||
| // expected-error@+1{{negative size value of -1 for 'void *__single __sized_by(count * size)' (aka 'void *__single')}} | |||
There was a problem hiding this comment.
I suggest we add another option to the select in err_bounds_safety_dynamic_count_negative so that we can emit for return value of type ...
ojhunt
left a comment
ojhunt
left a comment
There was a problem hiding this comment.
I don't have code change feedback, I'm assuming the new error messages in the Sema tests correspond to the required codegen changes, but want to be sure that that is the case.
| @@ -71,7 +71,8 @@ void TestUCountNegFail(void) { | |||
| // CHECK-NEXT: unreachable, {{!annotation ![0-9]+}} | |||
| // | |||
| void TestCountNegRetFail(void) { | |||
| int *local_p = my_alloc_int(-1); //rdar://80808704 | |||
| int x = -1; | |||
| int *local_p = my_alloc_int(x); | |||
There was a problem hiding this comment.
This removed the 80808704 rdar reference, but I can't see a similar reference in the Sema tests so I'm not sure which of the tests this matches.
Part of me actually just wants the original test case copied verbatim into the Sema tests to make it unambiguous that it fails in Sema. (this is not a strongly held opinion)
@hnrklssn thoughts?
3 participants
Added check for value of return type's count expression such that no negative values are passed as arguments to
functions returning
counted_by.Changed tests to correspond with correct program behavior and removed rdar://80808704 and rdar://103368466 comments from the tests which errored.
Fixes: #13550