r184256 - [analyzer] Do not report uninitialized value warnings inside swap functions.

[Jordan Rose]

On Jun 18, 2013, at 16:16 , Anna Zaks <ganna at apple.com> wrote:

> Author: zaks
> Date: Tue Jun 18 18:16:15 2013
> New Revision: 184256
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=184256&view=rev
> Log:
> [analyzer] Do not report uninitialized value warnings inside swap functions.
> 
> This silences warnings that could occur when one is swapping partially initialized structs. We suppress
> not only the assignments of uninitialized members, but any values inside swap because swap could
> potentially be used as a subroutine to swap class members.
> 
> This silences a warning from std::try::function::swap() on partially initialized objects.
> 
> Modified:
>    cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
>    cfe/trunk/test/Analysis/uninit-vals-ps-region.m
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp?rev=184256&r1=184255&r2=184256&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp Tue Jun 18 18:16:15 2013
> @@ -40,6 +40,15 @@ void UndefResultChecker::checkPostStmt(c
>   ProgramStateRef state = C.getState();
>   const LocationContext *LCtx = C.getLocationContext();
>   if (state->getSVal(B, LCtx).isUndef()) {
> +
> +    // Do not report assignments of uninitialized values inside swap functions.
> +    // This should allow to swap partially uninitialized structs
> +    // (radar://14129997)

We generally don't put Radar numbers in the source proper, although they do show up in test files. Also, is there a reason this is in UndefResultChecker and not just UndefinedAssignmentChecker?


> +    if (const FunctionDecl *EnclosingFunctionDecl =
> +        dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl()))
> +      if (C.getCalleeName(EnclosingFunctionDecl) == "swap")
> +        return;
> +
>     // Generate an error node.
>     ExplodedNode *N = C.generateSink();
>     if (!N)
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp?rev=184256&r1=184255&r2=184256&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp Tue Jun 18 18:16:15 2013
> @@ -38,6 +38,14 @@ void UndefinedAssignmentChecker::checkBi
>   if (!val.isUndef())
>     return;
> 
> +  // Do not report assignments of uninitialized values inside swap functions.
> +  // This should allow to swap partially uninitialized structs
> +  // (radar://14129997)

Ditto about Radar numbers.

> +  if (const FunctionDecl *EnclosingFunctionDecl =
> +      dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl()))
> +    if (C.getCalleeName(EnclosingFunctionDecl) == "swap")
> +      return;
> +
>   ExplodedNode *N = C.generateSink();
> 
>   if (!N)
> 
> Modified: cfe/trunk/test/Analysis/uninit-vals-ps-region.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals-ps-region.m?rev=184256&r1=184255&r2=184256&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/uninit-vals-ps-region.m (original)
> +++ cfe/trunk/test/Analysis/uninit-vals-ps-region.m Tue Jun 18 18:16:15 2013
> @@ -76,3 +76,18 @@ void PR10163 (void) {
>   test_PR10163(x[1]); // expected-warning{{uninitialized value}}
> }
> 
> +struct MyStr {
> +  int x;
> +  int y;
> +};
> +void swap(struct MyStr *To, struct MyStr *From) {
> +  // This is not really a swap but close enough for our test.
> +  To->x = From->x;
> +  To->y = From->y; // no warning
> +}
> +int test_undefined_member_assignment_in_swap(struct MyStr *s2) {
> +  struct MyStr s1;
> +  s1.x = 5;
> +  swap(s2, &s1);
> +  return s2->y; // expected-warning{{Undefined or garbage value returned to caller}}
> +}

Can we have a path-notes test to prove that the tracing goes all the way back to the initial lack of initialization, and that we get an "assigned here" note inside swap()?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130618/5a7db4aa/attachment.html>