Addressing uninitialized false positives via function/method initialization

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Addressing uninitialized false positives via function/method initialization

Nathan Herring
Ran into cases running the clang analyzer against some of my code
where an otherwise uninitialized local is initialized by a function
call or method call, but clang doesn't realize and reports the next
access as uninitialized or garbage. This appears to have at least one
representative bug, 9283.

I'm new to clang, having just gotten an enlistment and source indexing
yesterday, and would like to help work toward contributing a patch to
fix this (and learn the ropes along the way).

I've found the checker involved in CallAndMessageChecker.cpp, but this
appears to be downstream of the problem (not setting the value of the
SVal to something other than uninitialized). I've found
ExprEngine::VisitCall and VisitCXXMemberCallExpr, which is probably
the top of the world where I'd look at function arguments and look for
pointers to non-const Ts and non-const T references and make them
potential binds. Sound good so far? Next -- how do you know whether
the value will be set in the function call? Can we make it more
explicit by annotating the function arguments with something akin to
SAL's __out? Or does this walk down the AST into the call and sees if
the child (or its child, etc., etc.) do the right thing?

I've leafed through some of the Doxygen content for clang and the
InternalsManual page, but haven't seen exactly how this would play
out. Any pointers would be helpful!

Thx in advance,
nh
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Addressing uninitialized false positives via function/method initialization

Ted Kremenek
Hi Nathan,

This may already be fixed:


What version of the analyzer are you using?

Ted

On Apr 5, 2011, at 6:02 PM, Nathan Herring wrote:

Ran into cases running the clang analyzer against some of my code
where an otherwise uninitialized local is initialized by a function
call or method call, but clang doesn't realize and reports the next
access as uninitialized or garbage. This appears to have at least one
representative bug, 9283.

I'm new to clang, having just gotten an enlistment and source indexing
yesterday, and would like to help work toward contributing a patch to
fix this (and learn the ropes along the way).

I've found the checker involved in CallAndMessageChecker.cpp, but this
appears to be downstream of the problem (not setting the value of the
SVal to something other than uninitialized). I've found
ExprEngine::VisitCall and VisitCXXMemberCallExpr, which is probably
the top of the world where I'd look at function arguments and look for
pointers to non-const Ts and non-const T references and make them
potential binds. Sound good so far? Next -- how do you know whether
the value will be set in the function call? Can we make it more
explicit by annotating the function arguments with something akin to
SAL's __out? Or does this walk down the AST into the call and sees if
the child (or its child, etc., etc.) do the right thing?

I've leafed through some of the Doxygen content for clang and the
InternalsManual page, but haven't seen exactly how this would play
out. Any pointers would be helpful!

Thx in advance,
nh
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Addressing uninitialized false positives via function/method initialization

Nathan Herring
It does look like you got to it before I did. The build I'd been using was at least a week and change old. After a sync, rebuild, and recheck, it appears to no longer give that false positive.

Unfortunately, it still gives a false positive where the reference is captured in a ctor and then one of the member functions has a side effect which initializes it.

class SideEffect
{
public:
  SideEffect(int *pi); // caches pi in i_

  void Read(int *pi); // copies *pi into *i_

private:
  int *i_;
};

extern void UseMe(int i);

int main(const int /* argc */, const char *const /* argv */[]) {
  int i;

  SideEffect se(&i);
  int j = 1;
  se.Read(&j);

  UseMe(i);

  return 0;
}

Compiling this yields:

/home/nherring/test.cc:23:3: warning: Function call argument
      is an uninitialized value
  UseMe(i);
  ^     ~
1 warning generated.


It seems that, any pointer value could get captured as state inside an object if its ctor or member function takes that pointer argument. Is the checker now conservative, but only across one function call, rather than conservative in the sense of objects capturing state?

In the meanwhile, your commentary for the change suggests that the conservative C checker simply believes that these values (referenced via pointers) could have been set, and thus won't warn that it wasn't set, even though its entirely possible that the bound function and its children also don't set the value (perhaps only under certain code paths). Is there any future plan to eventually provide stronger safeguards, e.g., by custom attributes marking certain arguments as necessarily or conditionally (based on some return value) initialized, and/or captured by the object beyond the scope of the method call.

Thx,
nh

On Wed, Apr 6, 2011 at 1:07 PM, Ted Kremenek <[hidden email]> wrote:
Hi Nathan,

This may already be fixed:


What version of the analyzer are you using?

Ted

On Apr 5, 2011, at 6:02 PM, Nathan Herring wrote:

Ran into cases running the clang analyzer against some of my code
where an otherwise uninitialized local is initialized by a function
call or method call, but clang doesn't realize and reports the next
access as uninitialized or garbage. This appears to have at least one
representative bug, 9283.

I'm new to clang, having just gotten an enlistment and source indexing
yesterday, and would like to help work toward contributing a patch to
fix this (and learn the ropes along the way).

I've found the checker involved in CallAndMessageChecker.cpp, but this
appears to be downstream of the problem (not setting the value of the
SVal to something other than uninitialized). I've found
ExprEngine::VisitCall and VisitCXXMemberCallExpr, which is probably
the top of the world where I'd look at function arguments and look for
pointers to non-const Ts and non-const T references and make them
potential binds. Sound good so far? Next -- how do you know whether
the value will be set in the function call? Can we make it more
explicit by annotating the function arguments with something akin to
SAL's __out? Or does this walk down the AST into the call and sees if
the child (or its child, etc., etc.) do the right thing?

I've leafed through some of the Doxygen content for clang and the
InternalsManual page, but haven't seen exactly how this would play
out. Any pointers would be helpful!

Thx in advance,
nh
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Addressing uninitialized false positives via function/method initialization

Ted Kremenek
Thanks Nathan.  Can you file a bug report with a self-contained test case?

On Apr 6, 2011, at 4:16 PM, Nathan Herring wrote:

It does look like you got to it before I did. The build I'd been using was at least a week and change old. After a sync, rebuild, and recheck, it appears to no longer give that false positive.

Unfortunately, it still gives a false positive where the reference is captured in a ctor and then one of the member functions has a side effect which initializes it.

class SideEffect
{
public:
  SideEffect(int *pi); // caches pi in i_

  void Read(int *pi); // copies *pi into *i_

private:
  int *i_;
};

extern void UseMe(int i);

int main(const int /* argc */, const char *const /* argv */[]) {
  int i;

  SideEffect se(&i);
  int j = 1;
  se.Read(&j);

  UseMe(i);

  return 0;
}

Compiling this yields:

/home/nherring/test.cc:23:3: warning: Function call argument
      is an uninitialized value
  UseMe(i);
  ^     ~
1 warning generated.


It seems that, any pointer value could get captured as state inside an object if its ctor or member function takes that pointer argument. Is the checker now conservative, but only across one function call, rather than conservative in the sense of objects capturing state?

In the meanwhile, your commentary for the change suggests that the conservative C checker simply believes that these values (referenced via pointers) could have been set, and thus won't warn that it wasn't set, even though its entirely possible that the bound function and its children also don't set the value (perhaps only under certain code paths). Is there any future plan to eventually provide stronger safeguards, e.g., by custom attributes marking certain arguments as necessarily or conditionally (based on some return value) initialized, and/or captured by the object beyond the scope of the method call.

Thx,
nh

On Wed, Apr 6, 2011 at 1:07 PM, Ted Kremenek <[hidden email]> wrote:
Hi Nathan,

This may already be fixed:


What version of the analyzer are you using?

Ted

On Apr 5, 2011, at 6:02 PM, Nathan Herring wrote:

Ran into cases running the clang analyzer against some of my code
where an otherwise uninitialized local is initialized by a function
call or method call, but clang doesn't realize and reports the next
access as uninitialized or garbage. This appears to have at least one
representative bug, 9283.

I'm new to clang, having just gotten an enlistment and source indexing
yesterday, and would like to help work toward contributing a patch to
fix this (and learn the ropes along the way).

I've found the checker involved in CallAndMessageChecker.cpp, but this
appears to be downstream of the problem (not setting the value of the
SVal to something other than uninitialized). I've found
ExprEngine::VisitCall and VisitCXXMemberCallExpr, which is probably
the top of the world where I'd look at function arguments and look for
pointers to non-const Ts and non-const T references and make them
potential binds. Sound good so far? Next -- how do you know whether
the value will be set in the function call? Can we make it more
explicit by annotating the function arguments with something akin to
SAL's __out? Or does this walk down the AST into the call and sees if
the child (or its child, etc., etc.) do the right thing?

I've leafed through some of the Doxygen content for clang and the
InternalsManual page, but haven't seen exactly how this would play
out. Any pointers would be helpful!

Thx in advance,
nh
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Addressing uninitialized false positives via function/method initialization

Nathan Herring
Done.

http://llvm.org/bugs/show_bug.cgi?id=9645

On Thu, Apr 7, 2011 at 10:01 AM, Ted Kremenek <[hidden email]> wrote:
Thanks Nathan.  Can you file a bug report with a self-contained test case?

On Apr 6, 2011, at 4:16 PM, Nathan Herring wrote:

It does look like you got to it before I did. The build I'd been using was at least a week and change old. After a sync, rebuild, and recheck, it appears to no longer give that false positive.

Unfortunately, it still gives a false positive where the reference is captured in a ctor and then one of the member functions has a side effect which initializes it.

class SideEffect
{
public:
  SideEffect(int *pi); // caches pi in i_

  void Read(int *pi); // copies *pi into *i_

private:
  int *i_;
};

extern void UseMe(int i);

int main(const int /* argc */, const char *const /* argv */[]) {
  int i;

  SideEffect se(&i);
  int j = 1;
  se.Read(&j);

  UseMe(i);

  return 0;
}

Compiling this yields:

/home/nherring/test.cc:23:3: warning: Function call argument
      is an uninitialized value
  UseMe(i);
  ^     ~
1 warning generated.


It seems that, any pointer value could get captured as state inside an object if its ctor or member function takes that pointer argument. Is the checker now conservative, but only across one function call, rather than conservative in the sense of objects capturing state?

In the meanwhile, your commentary for the change suggests that the conservative C checker simply believes that these values (referenced via pointers) could have been set, and thus won't warn that it wasn't set, even though its entirely possible that the bound function and its children also don't set the value (perhaps only under certain code paths). Is there any future plan to eventually provide stronger safeguards, e.g., by custom attributes marking certain arguments as necessarily or conditionally (based on some return value) initialized, and/or captured by the object beyond the scope of the method call.

Thx,
nh

On Wed, Apr 6, 2011 at 1:07 PM, Ted Kremenek <[hidden email]> wrote:
Hi Nathan,

This may already be fixed:


What version of the analyzer are you using?

Ted

On Apr 5, 2011, at 6:02 PM, Nathan Herring wrote:

Ran into cases running the clang analyzer against some of my code
where an otherwise uninitialized local is initialized by a function
call or method call, but clang doesn't realize and reports the next
access as uninitialized or garbage. This appears to have at least one
representative bug, 9283.

I'm new to clang, having just gotten an enlistment and source indexing
yesterday, and would like to help work toward contributing a patch to
fix this (and learn the ropes along the way).

I've found the checker involved in CallAndMessageChecker.cpp, but this
appears to be downstream of the problem (not setting the value of the
SVal to something other than uninitialized). I've found
ExprEngine::VisitCall and VisitCXXMemberCallExpr, which is probably
the top of the world where I'd look at function arguments and look for
pointers to non-const Ts and non-const T references and make them
potential binds. Sound good so far? Next -- how do you know whether
the value will be set in the function call? Can we make it more
explicit by annotating the function arguments with something akin to
SAL's __out? Or does this walk down the AST into the call and sees if
the child (or its child, etc., etc.) do the right thing?

I've leafed through some of the Doxygen content for clang and the
InternalsManual page, but haven't seen exactly how this would play
out. Any pointers would be helpful!

Thx in advance,
nh
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev





_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev