summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer/Core
Commit message (Collapse)AuthorAgeFilesLines
...
* [analyzer] Fix null tracking for the given test case, by using the proper ↵Anna Zaks2013-04-051-18/+3
| | | | | | state and removing redundant code. llvm-svn: 178933
* [analyzer] Show path diagnostic for C++ initializersAnna Zaks2013-04-053-2/+17
| | | | | | Also had to modify the PostInitializer ProgramLocation to contain the field region. llvm-svn: 178826
* [analyzer] Enable destructor inlining by default (c++-inlining=destructors).Jordan Rose2013-04-041-1/+1
| | | | | | | | | | | | | | | | | | | | | This turns on not only destructor inlining, but inlining of constructors for types with non-trivial destructors. Per r178516, we will still not inline the constructor or destructor of anything that looks like a container unless the analyzer-config option 'c++-container-inlining' is set to 'true'. In addition to the more precise path-sensitive model, this allows us to catch simple smart pointer issues: #include <memory> void test() { std::auto_ptr<int> releaser(new int[4]); } // memory allocated with 'new[]' should not be deleted with 'delete' <rdar://problem/12295363> llvm-svn: 178805
* [analyzer] Allow tracknullOrUndef look through the ternary operator even ↵Anna Zaks2013-04-031-13/+17
| | | | | | | | | | | | | when condition is unknown Improvement of r178684 and r178685. Jordan has pointed out that I should not rely on the value of the condition to know which expression branch has been taken. It will not work in cases the branch condition is an unknown value (ex: we do not track the constraints for floats). The better way of doing this would be to find out if the current node is the right or left successor of the node that has the ternary operator as a terminator (which is how this is done in other places, like ConditionBRVisitor). llvm-svn: 178701
* [analyzer] Correctly handle destructors for lifetime-extended temporaries.Jordan Rose2013-04-031-14/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | The lifetime of a temporary can be extended when it is immediately bound to a local reference: const Value &MyVal = Value("temporary"); In this case, the temporary object's lifetime is extended for the entire scope of the reference; at the end of the scope it is destroyed. The analyzer was modeling this improperly in two ways: - Since we don't model temporary constructors just yet, we create a fake temporary region when it comes time to "materialize" a temporary into a real object (lvalue). This wasn't taking base casts into account when the bindings being materialized was Unknown; now it always respects base casts except when the temporary region is itself a pointer. - When actually destroying the region, the analyzer did not actually load from the reference variable -- it was basically destroying the reference instead of its referent. Now it does do the load. This will be more useful whenever we finally start modeling temporaries, or at least those that get bound to local reference variables. <rdar://problem/13552274> llvm-svn: 178697
* [analyzer] make peelOffOuterExpr in BugReporterVisitors recursively peel off ↵Anna Zaks2013-04-031-30/+31
| | | | | | select Exprs llvm-svn: 178685
* [analyzer] Properly handle the ternary operator in trackNullOrUndefValueAnna Zaks2013-04-032-11/+22
| | | | | | | | | | | | 1) Look for the node where the condition expression is live when checking if it is constrained to true or false. 2) Fix a bug in ProgramState::isNull, which was masking the problem. When the expression is not a symbol (,which is the case when it is Unknown) return unconstrained value, instead of value constrained to “false”! (Thankfully other callers of isNull have not been effected by the bug.) llvm-svn: 178684
* [analyzer] Fix typo.Anna Zaks2013-04-031-1/+1
| | | | | | Thanks Jordan! llvm-svn: 178683
* [analyzer] Better model for copying of array fields in implicit copy ctors.Jordan Rose2013-04-033-26/+73
| | | | | | | | | | | | - Find the correct region to represent the first array element when constructing a CXXConstructorCall. - If the array is trivial, model the copy with a primitive load/store. - Don't warn about the "uninitialized" subscript in the AST -- we don't use the helper variable that Sema provides. <rdar://problem/13091608> llvm-svn: 178602
* Silencing warnings in MSVC due to duplicate identifiers.Aaron Ballman2013-04-021-2/+2
| | | | llvm-svn: 178591
* [analyzer] Teach invalidateRegions that regions within LazyCompoundVal need ↵Anna Zaks2013-04-023-54/+127
| | | | | | | | | | | | to be invalidated Refactor invalidateRegions to take SVals instead of Regions as input and teach RegionStore about processing LazyCompoundVal as a top-level “escaping” value. This addresses several false positives that get triggered by the NewDelete checker, but the underlying issue is reproducible with other checkers as well (for example, MallocChecker). llvm-svn: 178518
* [analyzer] For now, don't inline [cd]tors of C++ containers.Jordan Rose2013-04-022-4/+63
| | | | | | | | | | | | | | | | | | | | | This is a heuristic to make up for the fact that the analyzer doesn't model C++ containers very well. One example is modeling that 'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be nice to model this explicitly, but for now it just results in a lot of false positives. The actual heuristic checks if the base type has a member named 'begin' or 'iterator'. If so, we treat the constructors and destructors of that type as opaque, rather than inlining them. This is intended to drastically reduce the number of false positives reported with experimental destructor support turned on. We can tweak the heuristic in the future, but we'd rather err on the side of false negatives for now. <rdar://problem/13497258> llvm-svn: 178516
* [analyzer] Cache whether a function is generally inlineable.Jordan Rose2013-04-021-52/+96
| | | | | | | | | | | | | | | Certain properties of a function can determine ahead of time whether or not the function is inlineable, such as its kind, its signature, or its location. We can cache this value in the FunctionSummaries map to avoid rechecking these static properties for every call. Note that the analyzer may still decide not to inline a specific call to a function because of the particular dynamic properties of the call along the current path. No intended functionality change. llvm-svn: 178515
* [analyzer] Use inline storage in the FunctionSummary DenseMap.Jordan Rose2013-04-021-10/+4
| | | | | | | | | | | | | The summaries lasted for the lifetime of the map anyway; no reason to include an extra allocation. Also, use SmallBitVector instead of BitVector to track the visited basic blocks -- most functions will have less than 64 basic blocks -- and use bitfields for the other fields to reduce the size of the structure. No functionality change. llvm-svn: 178514
* [analyzer] Allow suppressing diagnostics reported within the 'std' namespaceJordan Rose2013-04-022-7/+38
| | | | | | | | | | | | This is controlled by the 'suppress-c++-stdlib' analyzer-config flag. It is currently off by default. This is more suppression than we'd like to do, since obviously there can be user-caused issues within 'std', but it gives us the option to wield a large hammer to suppress false positives the user likely can't work around. llvm-svn: 178513
* [analyzer] Restructure ExprEngine::VisitCXXNewExpr to do a bit less work.Jordan Rose2013-03-301-10/+11
| | | | | | No functionality change. llvm-svn: 178402
* [analyzer] Handle caching out while evaluating a C++ new expression.Jordan Rose2013-03-301-3/+4
| | | | | | | | | | | | | Evaluating a C++ new expression now includes generating an intermediate ExplodedNode, and this node could very well represent a previously- reachable state in the ExplodedGraph. If so, we can short-circuit the rest of the evaluation. Caught by the assertion a few lines later. <rdar://problem/13510065> llvm-svn: 178401
* [analyzer] Address Jordan’s review of r178309 - do not register an extra ↵Anna Zaks2013-03-291-20/+19
| | | | | | | | | | | | visitor for nil receiver We can check if the receiver is nil in the node that corresponds to the StmtPoint of the message send. At that point, the receiver is guaranteed to be live. We will find at least one unreclaimed node due to my previous commit (look for StmtPoint instead of PostStmt) and the fact that the nil receiver nodes are tagged. + a couple of extra tests. llvm-svn: 178381
* [analyzer] Look for a StmtPoint node instead of PostStmt in ↵Anna Zaks2013-03-291-1/+1
| | | | | | | | | | trackNullOrUndefValue. trackNullOrUndefValue tries to find the first node that matches the statement it is tracking. Since we collect PostStmt nodes (in node reclamation), none of those might be on the current path, so relax the search to look for any StmtPoint. llvm-svn: 178380
* Add static analyzer support for conditionally executing static initializers.Ted Kremenek2013-03-294-20/+47
| | | | llvm-svn: 178318
* Add configuration plumbing to enable static initializer branching in the CFG ↵Ted Kremenek2013-03-292-1/+7
| | | | | | | | | for the analyzer. This setting still isn't enabled yet in the analyzer. This is just prep work. llvm-svn: 178317
* [analyzer] Add support for escape of const pointers and use it to allow ↵Anna Zaks2013-03-284-25/+63
| | | | | | | | | | | | | “newed” pointers to escape Add a new callback that notifies checkers when a const pointer escapes. Currently, this only works for const pointers passed as a top level parameter into a function. We need to differentiate the const pointers escape from regular escape since the content pointed by const pointer will not change; if it’s a file handle, a file cannot be closed; but delete is allowed on const pointers. This should suppress several false positives reported by the NewDelete checker on llvm codebase. llvm-svn: 178310
* [analyzer] Apply the suppression rules to the nil receiver only if the value ↵Anna Zaks2013-03-281-37/+57
| | | | | | | | | | | | | | participates in the computation of the nil we warn about. We should only suppress a bug report if the IDCed or null returned nil value is directly related to the value we are warning about. This was not the case for nil receivers - we would suppress a bug report that had an IDCed nil receiver on the path regardless of how it’s related to the warning. 1) Thread EnableNullFPSuppression parameter through the visitors to differentiate between tracking the value which is directly responsible for the bug and other values that visitors are tracking (ex: general tracking of nil receivers). 2) in trackNullOrUndef specifically address the case when a value of the message send is nil due to the receiver being nil. llvm-svn: 178309
* [analyzer] For now assume all standard global 'operator new' functions ↵Anton Yartsev2013-03-281-5/+25
| | | | | | | | allocate memory in heap. + Improved test coverage for cplusplus.NewDelete checker. llvm-svn: 178244
* [analyzer] Use evalBind for C++ new of scalar types.Jordan Rose2013-03-271-7/+10
| | | | | | | | | | These types will not have a CXXConstructExpr to do the initialization for them. Previously we just used a simple call to ProgramState::bindLoc, but that doesn't trigger proper checker callbacks (like pointer escape). Found by Anton Yartsev. llvm-svn: 178160
* [analyzer] Cleanup: only get the PostStmt when we need the underlying Stmt + ↵Anna Zaks2013-03-271-4/+4
| | | | | | comment llvm-svn: 178153
* [analyzer] Ensure that the node NilReceiverBRVisitor is looking for is not ↵Anna Zaks2013-03-271-1/+1
| | | | | | | | | reclaimed The visitor should look for the PreStmt node as the receiver is nil in the PreStmt and this is the node. Also, tag the nil receiver nodes with a special tag for consistency. llvm-svn: 178152
* [analyzer] Make sure IDC works for ‘NSContainer value/key is nil’ checks.Anna Zaks2013-03-261-14/+22
| | | | | | | | | Register the nil tracking visitors with the region and refactor trackNullOrUndefValue a bit. Also adds the cast and paren stripping before checking if the value is an OpaqueValueExpr or ExprWithCleanups. llvm-svn: 178093
* [analyzer] Change inlining policy to inline small functions when reanalyzing ↵Anna Zaks2013-03-261-1/+7
| | | | | | | | ObjC methods as top level. This allows us to better reason about(inline) small wrapper functions. llvm-svn: 178063
* [analyzer] micro optimization as per Jordan’s feedback on r177905.Anna Zaks2013-03-261-1/+1
| | | | llvm-svn: 178062
* [analyzer] Set concrete offset bindings to UnknownVal when processing ↵Anna Zaks2013-03-251-2/+10
| | | | | | | | | | symbolic offset binding, even if no bindings are present. This addresses an undefined value false positive from concreteOffsetBindingIsInvalidatedBySymbolicOffsetAssignment. Fixes PR14877; radar://12991168. llvm-svn: 177905
* [analyzer] Teach ConstraintManager to ignore NonLoc <> NonLoc comparisons.Jordan Rose2013-03-241-2/+7
| | | | | | | | | These aren't generated by default, but they are needed when either side of the comparison is tainted. Should fix our internal buildbot. llvm-svn: 177846
* [analyzer] Teach constraint managers about unsigned comparisons.Jordan Rose2013-03-233-20/+35
| | | | | | | | | | | In C, comparisons between signed and unsigned numbers are always done in unsigned-space. Thus, we should know that "i >= 0U" is always true, even if 'i' is signed. Similarly, "u >= 0" is also always true, even though '0' is signed. Part of <rdar://problem/13239003> (false positives related to std::vector) llvm-svn: 177806
* [analyzer] Loc-Loc operations (subtraction or comparison) produce a NonLoc.Jordan Rose2013-03-232-8/+8
| | | | | | | | | | For two concrete locations, we were producing another concrete location and then casting it to an integer. We should just create a nonloc::ConcreteInt to begin with. No functionality change. llvm-svn: 177805
* [analyzer] Also transform "a < b" to "(b - a) > 0" in the constraint manager.Jordan Rose2013-03-231-15/+17
| | | | | | | | | | | | We can support the full range of comparison operations between two locations by canonicalizing them as subtraction, as in the previous commit. This won't work (well) if either location includes an offset, or (again) if the comparisons are not consistent about which region comes first. <rdar://problem/13239003> llvm-svn: 177803
* Add reverseComparisonOp and negateComparisonOp to BinaryOperator.Jordan Rose2013-03-232-46/+5
| | | | | | ...and adopt them in the analyzer. llvm-svn: 177802
* [analyzer] Translate "a != b" to "(b - a) != 0" in the constraint manager.Jordan Rose2013-03-233-23/+44
| | | | | | | | | | | | | | | | | | | | | | | Canonicalizing these two forms allows us to better model containers like std::vector, which use "m_start != m_finish" to implement empty() but "m_finish - m_start" to implement size(). The analyzer should have a consistent interpretation of these two symbolic expressions, even though it's not properly reasoning about either one yet. The other unfortunate thing is that while the size() expression will only ever be written "m_finish - m_start", the comparison may be written "m_finish == m_start" or "m_start == m_finish". Right now the analyzer does not attempt to canonicalize those two expressions, since it doesn't know which length expression to pick. Doing this correctly will probably require implementing unary minus as a new SymExpr kind (<rdar://problem/12351075>). For now, the analyzer inverts the order of arguments in the comparison to build the subtraction, on the assumption that "begin() != end()" is written more often than "end() != begin()". This is purely speculation. <rdar://problem/13239003> llvm-svn: 177801
* [analyzer] Use SymExprs to represent '<loc> - <loc>' and '<loc> == <loc>'.Jordan Rose2013-03-231-17/+22
| | | | | | | | | | We just treat this as opaque symbols, but even that allows us to handle simple cases where the same condition is tested twice. This is very common in the STL, which means that any project using the STL gets spurious errors. Part of <rdar://problem/13239003>. llvm-svn: 177800
* [analyzer] Correct the stale comment.Anna Zaks2013-03-231-3/+3
| | | | llvm-svn: 177788
* Revert "[analyzer] Break cycles (optionally) when trimming an ExplodedGraph."Jordan Rose2013-03-222-19/+3
| | | | | | | | | | | | | | The algorithm used here was ridiculously slow when a potential back-edge pointed to a node that already had a lot of successors. The previous commit makes this feature unnecessary anyway. This reverts r177468 / f4cf6b10f863b9bc716a09b2b2a8c497dcc6aa9b. Conflicts: lib/StaticAnalyzer/Core/BugReporter.cpp llvm-svn: 177765
* [analyzer] Use a forward BFS instead of a reverse BFS to find shortest paths.Jordan Rose2013-03-221-150/+93
| | | | | | | | | | | | | | | | | | | | | | | | | For a given bug equivalence class, we'd like to emit the report with the shortest path. So far to do this we've been trimming the ExplodedGraph to only contain relevant nodes, then doing a reverse BFS (starting at all the error nodes) to find the shortest paths from the root. However, this is fairly expensive when we are suppressing many bug reports in the same equivalence class. r177468-9 tried to solve this problem by breaking cycles during graph trimming, then updating the BFS priorities after each suppressed report instead of recomputing the whole thing. However, breaking cycles is not a cheap operation because an analysis graph minus cycles is still a DAG, not a tree. This fix changes the algorithm to do a single forward BFS (starting from the root) and to use that to choose the report with the shortest path by looking at the error nodes with the lowest BFS priorities. This was Anna's idea, and has the added advantage of requiring no update step: we can just pick the error node with the next lowest priority to produce the next bug report. <rdar://problem/13474689> llvm-svn: 177764
* [analyzer] Fix ExprEngine::ViewGraph to handle C++ initializers.Jordan Rose2013-03-221-40/+55
| | | | | | Debugging aid only, no functionality change. llvm-svn: 177762
* [analyzer] Appease buildbots: include template arguments in base class ref.Jordan Rose2013-03-201-1/+1
| | | | llvm-svn: 177583
* [analyzer] Don't invalidate globals when there's no call involved.Jordan Rose2013-03-201-11/+9
| | | | | | | | | | | | | | This fixes some mistaken condition logic in RegionStore that caused global variables to be invalidated when /any/ region was invalidated, rather than only as part of opaque function calls. This was only being used by CStringChecker, and so users will now see that strcpy() and friends do not invalidate global variables. Also, add a test case we don't handle properly: explicitly-assigned global variables aren't being invalidated by opaque calls. This is being tracked by <rdar://problem/13464044>. llvm-svn: 177572
* [analyzer] Track malloc'd memory into struct fields.Jordan Rose2013-03-201-6/+0
| | | | | | | | | | | | | | | Due to improper modelling of copy constructors (specifically, their const reference arguments), we were producing spurious leak warnings for allocated memory stored in structs. In order to silence this, we decided to consider storing into a struct to be the same as escaping. However, the previous commit has fixed this issue and we can now properly distinguish leaked memory that happens to be in a struct from a buffer that escapes within a struct wrapper. Originally applied in r161511, reverted in r174468. <rdar://problem/12945937> llvm-svn: 177571
* [analyzer] Invalidate regions indirectly accessible through const pointers.Jordan Rose2013-03-203-66/+79
| | | | | | | | | | | | | | | | | | | | | | | | | | In this case, the value of 'x' may be changed after the call to indirectAccess: struct Wrapper { int *ptr; }; void indirectAccess(const Wrapper &w); void test() { int x = 42; Wrapper w = { x }; clang_analyzer_eval(x == 42); // TRUE indirectAccess(w); clang_analyzer_eval(x == 42); // UNKNOWN } This is important for modelling return-by-value objects in C++, to show that the contents of the struct are escaping in the return copy-constructor. <rdar://problem/13239826> llvm-svn: 177570
* [analyzer] Remove strip of ElementRegion in CallEvent::invalidateRegions.Jordan Rose2013-03-201-26/+0
| | | | | | | | | | | | | | This is a bit of old code trying to deal with the fact that functions that take pointers often use them to access an entire array via pointer arithmetic. However, RegionStore already conservatively assumes you can use pointer arithmetic to access any part of a region. Some day we may want to go back to handling this specifically for calls, but we can do that in the future. No functionality change. llvm-svn: 177569
* [analyzer] Re-apply "Do part of the work to find shortest bug paths up front".Jordan Rose2013-03-201-43/+126
| | | | | | | | | | With the assurance that the trimmed graph does not contain cycles, this patch is safe (with a few tweaks), and provides the performance boost it was intended to. Part of performance work for <rdar://problem/13433687>. llvm-svn: 177469
* [analyzer] Break cycles (optionally) when trimming an ExplodedGraph.Jordan Rose2013-03-202-3/+19
| | | | | | | | | Having a trimmed graph with no cycles (a DAG) is much more convenient for trying to find shortest paths, which is exactly what BugReporter needs to do. Part of the performance work for <rdar://problem/13433687>. llvm-svn: 177468
* [analyzer] Do not believe lazy binding when symbolic region types do not matchAnna Zaks2013-03-191-5/+16
| | | | | | | | | | | This fixes a crash when analyzing LLVM that was exposed by r177220 (modeling of trivial copy/move assignment operators). When we look up a lazy binding for “Builder”, we see the direct binding of Loc at offset 0. Previously, we believed the binding, which led to a crash. Now, we do not believe it as the types do not match. llvm-svn: 177453
OpenPOWER on IntegriCloud