This is an archive of the discontinued LLVM Phabricator instance.

[analyzer][UninitializedObjectChecker] Support for nonloc::LocAsInteger
ClosedPublic

Authored by Szelethus on Jul 17 2018, 10:41 AM.

Diff Detail

Event Timeline

Szelethus created this revision.Jul 17 2018, 10:41 AM
george.karpenkov requested changes to this revision.Jul 17 2018, 11:06 AM

I think a checker for uninitialized values left after a constructor call is very valuable.

It is uncertain whether it is valuable to check that all pointers point to initialized objects for a number of reasons:

  • As we have seen, it increases the number of false positives
  • It significantly increases the complexity of the checker
  • Projects would be more reluctant to try the checker due to the first point

Could we actually go in an opposite direction and try to separate the pointer-chasing into perhaps a separate checker?

This revision now requires changes to proceed.Jul 17 2018, 11:06 AM

I just added a new patch as you wrote that comment (D49438)! Separating this functionality to a separate checker sounds great. I didn't actually check it, but I bet that around ~30-40% is at least indirectly in relation with pointer/reference handling, and it is getting a little out of hand.

I'll definitely explore this option too. Thanks for the idea! :)

george.karpenkov accepted this revision.Aug 2 2018, 2:30 PM

I'm fine with this if pointer-chasing is not on by default.

This revision is now accepted and ready to land.Aug 2 2018, 2:30 PM
NoQ requested changes to this revision.Aug 7 2018, 1:43 PM
NoQ added inline comments.
test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
26

This one also suffers from D49228#1176136

This revision now requires changes to proceed.Aug 7 2018, 1:43 PM
Szelethus updated this revision to Diff 159929.Aug 9 2018, 8:24 AM

Re-implemented with the refactored checker.

Szelethus marked an inline comment as done.Aug 9 2018, 10:20 AM
NoQ accepted this revision.Aug 28 2018, 12:58 PM

Let's see what it finds, i guess.

This revision is now accepted and ready to land.Aug 28 2018, 12:58 PM
This revision was automatically updated to reflect the committed changes.