Commit Graph

1336 Commits

Author SHA1 Message Date
Jordan Rose 12f669e3cd [analyzer] Member function calls that use qualified names are non-virtual.
C++11 [expr.call]p1: ...If the selected function is non-virtual, or if the
  id-expression in the class member access expression is a qualified-id,
  that function is called. Otherwise, its final overrider in the dynamic type
  of the object expression is called.

<rdar://problem/12255556>

llvm-svn: 163577
2012-09-11 00:31:02 +00:00
Anna Zaks 1ded453e36 [analyzer] Turn stl inlining back on.
The one reported bug, which was exposed by stl inlining, is addressed in
r163558.

llvm-svn: 163574
2012-09-10 23:59:02 +00:00
Anna Zaks 4f9c460874 [analyzer] Do not count calls to small functions when computing stack
depth.

We only want to count how many substantial functions we inlined. This
is an improvement to r163558.

llvm-svn: 163571
2012-09-10 23:35:11 +00:00
Anna Zaks 5446f4dfb1 [analyzer] Add an option to enable/disable objc inlining.
llvm-svn: 163562
2012-09-10 22:56:41 +00:00
Anna Zaks 14ce52492f [analyzer] Add ipa-always-inline-size option (with 3 as the default).
The option allows to always inline very small functions, whose size (in
number of basic blocks) is set using -analyzer-config
ipa-always-inline-size option.

llvm-svn: 163558
2012-09-10 22:37:19 +00:00
Jordan Rose c6fcbf06a6 [analyzer] Make the defaults explicit for each of the new config options.
Also, document both new inlining options in IPA.txt.

llvm-svn: 163551
2012-09-10 21:54:24 +00:00
Jordan Rose 1e0e4001c8 [analyzer] For now, don't inline C++ standard library functions.
This is a (heavy-handed) solution to PR13724 -- until we know we can do
a good job inlining the STL, it's best to be consistent and not generate
more false positives than we did before. We can selectively whitelist
certain parts of the 'std' namespace that are known to be safe.

This is controlled by analyzer config option 'c++-stdlib-inlining', which
can be set to "true" or "false".

This commit also adds control for whether or not to inline any templated
functions (member or non-member), under the config option
'c++-template-inlining'. This option is currently on by default.

llvm-svn: 163548
2012-09-10 21:27:35 +00:00
Dmitri Gribenko 76bb5cabfa Remove redundant semicolons which are null statements.
llvm-svn: 163546
2012-09-10 21:20:09 +00:00
Ted Kremenek a0fa5d6564 Fix another case where we should be using isBeforeInTranslationUnit().
llvm-svn: 163533
2012-09-10 19:07:56 +00:00
Ted Kremenek 54fd079265 Add a few more cases where we should be using isBeforeInTranslationUnit().
llvm-svn: 163531
2012-09-10 19:02:33 +00:00
Ted Kremenek 6c7a5eae6d Revert "Revert Ted's r163489 and r163490, due to breakage."
I need to see how this breaks on other platforms when I fix the issue
that Benjamin Kramer pointed out.

This includes r163489 and r163490, plus a two line change.

llvm-svn: 163512
2012-09-10 14:50:55 +00:00
Benjamin Kramer 199f8da3ae Make helper functions static.
llvm-svn: 163505
2012-09-10 11:57:16 +00:00
NAKAMURA Takumi 6eb1399088 Revert Ted's r163489 and r163490, due to breakage.
r163489, "Take another crack at stabilizing the emission order of analyzer"
r163490, "Use isBeforeInTranslationUnitThan() instead of operator<."

llvm-svn: 163497
2012-09-10 09:17:27 +00:00
Ted Kremenek f1fc8ce65d Use isBeforeInTranslationUnitThan() instead of operator<.
llvm-svn: 163490
2012-09-10 06:56:07 +00:00
Ted Kremenek 3d92699d3c Take another crack at stabilizing the emission order of analyzer
diagnostics without using FoldingSetNodeIDs.  This is done
by doing a complete recursive comparison of the PathDiagnostics.

Note that the previous method of comparing FoldingSetNodeIDs did
not end up relying on unstable things such as pointer addresses, so
I suspect this may still have some issues on various buildbots because
I'm not sure if the true source of non-determinism has been eliminated.
The tests pass for me, so the only way to know is to commit this change
and see what happens.

llvm-svn: 163489
2012-09-10 06:20:06 +00:00
Ted Kremenek 9b9ee2a616 Indent the "message" key in analyzer plist output.
llvm-svn: 163487
2012-09-10 06:19:43 +00:00
Ted Kremenek e9764d8f91 Remove dead method ProgramState::MarshalState().
llvm-svn: 163479
2012-09-09 14:55:59 +00:00
Ted Kremenek e7ec4ef48d Fix bug in BugReporter::RemoveUneededCalls() where "prunable"
PathDiagnosticEventPieces were *always* pruned.  Instead, they
are suppose to only be pruned if the entire call gets pruned.

llvm-svn: 163460
2012-09-08 07:18:18 +00:00
Ted Kremenek b0d1c70258 Attempt (again) to stabilize the order of the emission of diagnostics
of the analyzer by using the FullProfile() of a PathDiagnostic
for ordering them.

llvm-svn: 163455
2012-09-08 04:26:37 +00:00
Jordan Rose 5481cfefa6 [analyzer] ObjCSelfInitChecker should always clean up in postCall checks.
ObjCSelfInitChecker stashes information in the GDM to persist it across
function calls; it is stored in pre-call checks and retrieved post-call.
The post-call check is supposed to clear out the stored state, but was
failing to do so in cases where the call did not have a symbolic return
value.

This was actually causing the inappropriate cache-out from r163361.
Per discussion with Anna, we should never actually cache out when
assuming the receiver of an Objective-C message is non-nil, because
we guarded that node generation by checking that the state has changed.
Therefore, the only states that could reach this exact ExplodedNode are
ones that should have merged /before/ making this assumption.

r163361 has been reverted and the test case removed, since it won't
actually test anything interesting now.

llvm-svn: 163449
2012-09-08 01:47:28 +00:00
Jordan Rose bd94e5d815 [analyzer] Add debug output for ObjCSelfInitChecker's state.
No functionality change.

llvm-svn: 163448
2012-09-08 01:47:11 +00:00
Ted Kremenek 1fdcfcdf13 Revert "Attempt to make the PathDiagnostic emission order more deterministic by"
llvm-svn: 163446
2012-09-08 01:25:00 +00:00
Ted Kremenek af4cc7eab1 Revert "Further tweaks to hopefully make the PathDiagnostic emission more deterministic."
llvm-svn: 163445
2012-09-08 01:24:53 +00:00
Jordan Rose 5860e329a4 [analyzer] Remove constraints on dead symbols as part of removeDeadBindings.
Previously, we'd just keep constraints around forever, which means we'd
never be able to merge paths that differed only in constraints on dead
symbols.

Because we now allow constraints on symbolic expressions, not just single
symbols, this requires changing SymExpr::symbol_iterator to include
intermediate symbol nodes in its traversal, not just the SymbolData leaf
nodes.

llvm-svn: 163444
2012-09-08 01:24:53 +00:00
Jordan Rose dd5e8c4975 [analyzer] Symbolic regions are live if any subregions are live.
RegionStoreManager was only treating a SymbolicRegion's symbel as live
if there was a binding referring to the region itself.

No test case because constraints are currently not being cleaned out
of the constraint manager at all (even if the symbol is legitimately dead).

llvm-svn: 163443
2012-09-08 01:24:49 +00:00
Jordan Rose aaf8318480 [analyzer] Cast the result of a placement new-expression to the correct type.
This is necessary because further analysis will assume that the SVal's
type matches the AST type. This caused a crash when trying to perform
a derived-to-base cast on a C++ object that had been new'd to be another
object type.

Yet another crash in PR13763.

llvm-svn: 163442
2012-09-08 01:24:38 +00:00
Anna Zaks 044f3e2694 [analyzer] Address John's code review for r163407.
Teach malloc sizeof checker to find type inconsistencies in multi-
dimensional arrays.

llvm-svn: 163438
2012-09-08 00:09:02 +00:00
Ted Kremenek a11a741e2f Further tweaks to hopefully make the PathDiagnostic emission more deterministic.
llvm-svn: 163430
2012-09-07 23:13:11 +00:00
Ted Kremenek 244e1d7d0f Remove ProgramState::getSymVal(). It was being misused by Checkers,
with at least one subtle bug in MacOSXKeyChainAPIChecker where the
calling the method was a substitute for assuming a symbolic value
was null (which is not the case).

We still keep ConstraintManager::getSymVal(), but we use that as
an optimization in SValBuilder and ProgramState::getSVal() to
constant-fold SVals.  This is only if the ConstraintManager can
provide us with that information, which is no longer a requirement.
As part of this, introduce a default implementation of
ConstraintManager::getSymVal() which returns null.

For Checkers, introduce ConstraintManager::isNull(), which queries
the state to see if the symbolic value is constrained to be a null
value.  It does this without assuming it has been implicitly constant
folded.

llvm-svn: 163428
2012-09-07 22:31:01 +00:00
Ted Kremenek 334ad6ac13 Attempt to make the PathDiagnostic emission order more deterministic by
looking at PathPieces.

llvm-svn: 163427
2012-09-07 22:24:24 +00:00
Ted Kremenek 58ec11612c Remove ConstraintManager:isEqual(). It is no longer used.
llvm-svn: 163425
2012-09-07 22:24:18 +00:00
Jordan Rose 8dc77398a1 [analyzer] Use cast<> instead of getAs<> for a CFGElement known to be a CFGStmt.
When adding the next statement to the CoreEngine's work list, we take care
of all the special cases first. We certainly shouldn't be building
PostStmts with null statements (the diagnostics machinery assumes such
StmtPoints do not exist), and we should find out sooner if we're missing
a special case.

A refinement of r163402 that should help prevent further issues like PR13760.

llvm-svn: 163409
2012-09-07 19:48:09 +00:00
Anna Zaks 694be01519 [analyzer] Fix a false positive in sizeof malloc checker.
Don't warn when the sizeof argument is an array with the same element
type as the pointee of the return type.

llvm-svn: 163407
2012-09-07 19:20:13 +00:00
Jordan Rose 3c2713accf [analyzer] Don't use the address of a temporary CFGElement.
GCC destroys temporary objects more aggressively than clang, so this
results in incorrect behavior when compiling GCC Release builds.

We could avoid this issue under C++11 by preventing getAs from being
called when 'this' is an rvalue:

  template<class ElemTy> const ElemTy *getAs() const & { ... }
  template<class ElemTy> const ElemTy *getAs() const && = delete;

Unfortunately, we do not have compatibility macros for this behavior yet.

This will hopefully fix PR13760 and PR13762.

llvm-svn: 163402
2012-09-07 18:36:17 +00:00
Anna Zaks 67e0062b7c [analyzer] Explain why we need condition 8.
llvm-svn: 163394
2012-09-07 16:22:09 +00:00
Ted Kremenek 3f58774c8d Fix off-by-one bug in diagnostic prose of ObjCContainersASTChecker.
While the check itself should count 0-based for the parameter index,
the diagnostic should be 1-based (first, second, third, not start at 0).

Fixes <rdar://problem/12249569>.

llvm-svn: 163375
2012-09-07 07:13:08 +00:00
Ted Kremenek 891bcdb644 ExplodedGraph::shouldCollectNode() should not collect nodes for non-Expr Stmts
(as this previously was the case before this was refactored).  We also shouldn't
need to specially handle BinaryOperators since the eagerly-assume heuristic tags
such nodes.

llvm-svn: 163374
2012-09-07 06:56:18 +00:00
Ted Kremenek 7c15040e98 Fix bug in ConditionBRVisitor where for C++ (and not C) we were not ignoring
implicit pointer-to-boolean conversions in condition expressions.  This would
result in inconsistent diagnostic emission between C and C++.

A consequence of this is now ConditionBRVisitor and TrackConstraintBRVisitor may
emit redundant diagnostics, for example:

  "Assuming pointer value is null" (TrackConstraintBRVisitor)
  "Assuming 'p' is null" (ConditionBRVisitor)

We need to reconcile the two, and perhaps prefer one over the other in some
cases.

llvm-svn: 163372
2012-09-07 06:51:37 +00:00
Jordan Rose 81456d9f6d [analyzer] Fail gracefully when the dynamic type is outside the hierarchy.
With some particularly evil casts, we can get an object whose dynamic type
is not actually a subclass of its static type. In this case, we won't even
find the statically-resolved method as a devirtualization candidate.

Rather than assert that this situation cannot occur, we now simply check
that the dynamic type is not an ancestor or descendent of the static type,
and leave it at that.

This error actually occurred analyzing LLVM: CallEventManager uses a
BumpPtrAllocator to allocate a concrete subclass of CallEvent
(FunctionCall), but then casts it to the actual subclass requested
(such as ObjCMethodCall) to perform the constructor.

Yet another crash in PR13763.

llvm-svn: 163367
2012-09-07 01:19:42 +00:00
Ted Kremenek c008db99d7 Teach RetainCountChecker that CFPlugInInstanceCreate does not
return a CF object at all.

Fixes <rdar://problem/9566345>

llvm-svn: 163362
2012-09-06 23:47:02 +00:00
Jordan Rose 7e97996f4e [analyzer] Don't crash if we cache out while evaluating an ObjC message.
A bizarre series of coincidences led us to generate a previously-seen
node in the middle of processing an Objective-C message, where we assume
the receiver is non-nil. We were assuming that such an assumption would
never "cache out" like this, and blithely went on using a null ExplodedNode
as the predecessor for the next step in evaluation.

Although the test case committed here is complicated, this could in theory
happen in other ways as well, so the correct fix is just to test if the
non-nil assumption results in an ExplodedNode we've seen before.

<rdar://problem/12243648>

llvm-svn: 163361
2012-09-06 23:44:36 +00:00
Ted Kremenek 2a786959b1 Refine diagnostics for leaks reported when returning an object
via function/method with [CF,NS]_RETURNS_NOT_RETAINED.

Fixes <rdar://problem/11379000>.

llvm-svn: 163355
2012-09-06 23:03:07 +00:00
Ted Kremenek cadd9f186a Tweak DeadStoresChecker to not warn about dead stores to variables that
are used in EH code.  Right now the CFG doesn't support exceptions well,
so we need this hack to avoid bogus dead store warnings.

Fixes <rdar://problem/12147586>

llvm-svn: 163353
2012-09-06 22:32:48 +00:00
Jordan Rose 2bc9674b0a [analyzer] Don't attempt to devirtualize calls to base class destructors.
CXXDestructorCall now has a flag for when it is a base destructor call.
Other kinds of destructor calls (locals, fields, temporaries, and 'delete')
all behave as "whole-object" destructors and do not behave differently
from one another (specifically, in these cases we /should/ try to
devirtualize a call to a virtual destructor).

This was causing crashes in both our internal buildbot, the crash still
being tracked in PR13765, and some of the crashes being tracked in PR13763,
due to a assertion failure. (The behavior under -Asserts happened to be
correct anyway.)

Adding this knowledge also allows our DynamicTypePropagation checker to do
a bit less work; the special rules about virtual method calls during a
destructor only require extra handling during base destructors.

llvm-svn: 163348
2012-09-06 20:37:08 +00:00
Roman Divacky e637711ae0 Dont cast away const needlessly. Found by gcc48 -Wcast-qual.
llvm-svn: 163325
2012-09-06 15:59:27 +00:00
Anna Zaks 3245e584db [analyzer] Enhance the member expr tracking to account for references.
As per Jordan's suggestion. (Came out of code review for r163261.)

llvm-svn: 163269
2012-09-05 23:41:54 +00:00
Jordan Rose 6d671cc34a [analyzer] Always include destructors in the analysis CFG.
While destructors will continue to not be inlined (unless the analyzer
config option 'c++-inlining' is set to 'destructors'), leaving them out
of the CFG is an incomplete model of the behavior of an object, and
can cause false positive warnings (like PR13751, now working).

Destructors for temporaries are still not on by default, since
(a) we haven't actually checked this code to be sure it's fully correct
    (in particular, we probably need to be very careful with regard to
    lifetime-extension when a temporary is bound to a reference,
    C++11 [class.temporary]p5), and
(b) ExprEngine doesn't actually do anything when it sees a temporary
    destructor in the CFG -- not even invalidate the object region.

To enable temporary destructors, set the 'cfg-temporary-dtors' analyzer
config option to '1'. The old -cfg-add-implicit-dtors cc1 option, which
controlled all implicit destructors, has been removed.

llvm-svn: 163264
2012-09-05 22:55:23 +00:00
Anna Zaks e5cb4981d0 [analyzer] Fix a crash PR13762.
llvm-svn: 163262
2012-09-05 22:31:58 +00:00
Anna Zaks b4b2b57ee0 [analyzer] NullOrUndef diagnostics: track symbols binded to regions.
If a region is binded to a symbolic value, we should track the symbol.

(The code I changed was not previously exercised by the regression
tests.)

llvm-svn: 163261
2012-09-05 22:31:55 +00:00
Anna Zaks 1e2a0dc405 [analyzer] Remove unneeded code.
This region is set as interesting as part of trackNullOrUndefValue call,
no need to mark it as interesting twice.

llvm-svn: 163260
2012-09-05 22:31:49 +00:00