Thread-safety analysis: differentiate between two forms of analysis; a precise
analysis that may give false positives because it is confused by aliasing, and
a less precise analysis that has fewer false positives, but may have false
negatives. The more precise warnings are enabled by -Wthread-safety-precise.
An additional note clarify the warnings in the precise case.
Wrong crtbegin/crtend pair used for PIE on Android.
Android uses the same flavour of crt*.o for PIE and non-PIE executables, and a
different one for DSOs. GNU/Linux, on the other hand, uses one set of crt*.o
for non-PIE executables, and another for both PIE executables and DSOs.
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.
clang/test/Sema/format-strings-scanf.c: Relax a couple of expressions with expected-warning-re to let matched for Win32 targets.
- format specifies type 'wchar_t **' (aka 'int **') but the argument has type 'float *'
- format specifies type 'wchar_t **' (aka 'unsigned short **') but the argument has type 'float *'
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.
Richard Smith [Sat, 8 Sep 2012 07:16:20 +0000 (07:16 +0000)]
When a bad UTF-8 encoding or bogus escape sequence is encountered in a
string literal, produce a diagnostic pointing at the erroneous character
range, not at the start of the literal.
Attempt (again) to stabilize the order of the emission of diagnostics
of the analyzer by using the FullProfile() of a PathDiagnostic
for ordering them.
Richard Smith [Sat, 8 Sep 2012 02:08:36 +0000 (02:08 +0000)]
-fcatch-undefined-behavior: Factor emission of the creation of, and branch to,
the trap BB out of the individual checks and into a common function, to prepare
for making this code call into a runtime library. Rename the existing EmitCheck
to EmitTypeCheck to clarify it and to move it out of the way of the new
EmitCheck.
Jordan Rose [Sat, 8 Sep 2012 01:47:28 +0000 (01:47 +0000)]
[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.
Jordan Rose [Sat, 8 Sep 2012 01:24:53 +0000 (01:24 +0000)]
[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.
Jordan Rose [Sat, 8 Sep 2012 01:24:38 +0000 (01:24 +0000)]
[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.
objective-C: introduce __attribute((objc_requires_super)) on method
in classes. Use it to flag those method implementations which don't
contain call to 'super' if they have 'super' class and it has the method
with this attribute set. This is wip. // rdar://6386358
John McCall [Fri, 7 Sep 2012 23:30:50 +0000 (23:30 +0000)]
In ARC, if we're emitting assembly markers for calls to
objc_retainAutoreleasedReturnValue, we need to also be killing
them during return peepholing. Make sure we recognize an
intervening bitcast, but more importantly, assert if we can't
find the asm marker at all. rdar://problem/12133032
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.
Jordan Rose [Fri, 7 Sep 2012 19:48:09 +0000 (19:48 +0000)]
[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.
Manuel Klimek [Fri, 7 Sep 2012 13:10:32 +0000 (13:10 +0000)]
Introduces anchors into LibASTMatchersReference.html.
This allows linking to LibASTMatchersRefernce.html#<matcher><N>Anchor to
link to the N'the declaration of a matcher and automatically expand
its documentation.
Daniel Jasper [Fri, 7 Sep 2012 12:48:17 +0000 (12:48 +0000)]
Change the behavior of the isDerivedFrom-matcher to not match on the
class itself. This caused some confusion (intuitively, a class is not
derived from itself) and makes it hard to write certain matchers, e.g.
"match and bind any pair of base and subclass".
The original behavior can be achieved with a new isA-matcher. Similar
to all other matchers, this matcher has the same behavior and name as
the corresponding AST-entity - in this case the isa<>() function.
Manuel Klimek [Fri, 7 Sep 2012 09:26:10 +0000 (09:26 +0000)]
Implements hasAncestor.
Implements the hasAncestor matcher. This builds
on the previous patch that introduced DynTypedNode to build up
a parent map for an additional degree of freedom in the AST traversal.
The map is only built once we hit an hasAncestor matcher, in order
to not slow down matching for cases where this is not needed.
We could implement some speed-ups for special cases, like building up
the parent map as we go and only building up the full map if we break
out of the already visited part of the tree, but that is probably
not going to be worth it, and would make the code significantly more
complex.
Major TODOs are:
- implement hasParent
- implement type traversal
- implement memoization in hasAncestor
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).
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.
Rework the retain-release.m test to use FileCheck and the "text" output
of the analyzer, as the RetainReleaseChecker has many fine-grain
path diagnostic events that were not being checked. This uncovered
an inconsistency between the path diagnostics between Objective-C
and Objective-C++ code in ConditionBRVisitor that was fixed in a recent
patch.
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.
Richard Smith [Fri, 7 Sep 2012 02:06:42 +0000 (02:06 +0000)]
PR9023: A template template parameter whose template parameter list contains an
unexpanded parameter pack is a pack expansion. Thus, as with a non-type template
parameter which is a pack expansion, it needs to be expanded early into a fixed
list of template parameters.
Since the expanded list of template parameters is not itself a parameter pack,
it is permitted to appear before the end of the template parameter list, so also
remove that restriction (for both template template parameter pack expansions and
non-type template parameter pack expansions).
Jordan Rose [Fri, 7 Sep 2012 01:19:42 +0000 (01:19 +0000)]
[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.
Jordan Rose [Thu, 6 Sep 2012 23:44:36 +0000 (23:44 +0000)]
[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.
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.
Jordan Rose [Thu, 6 Sep 2012 20:37:08 +0000 (20:37 +0000)]
[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.
Chad Rosier [Thu, 6 Sep 2012 19:56:25 +0000 (19:56 +0000)]
[ms-inline asm] Output empty asm statements for the directives we don't
handle. Otherwise, the AsmParser will explode if we try to generate an
object files.
Chad Rosier [Thu, 6 Sep 2012 19:35:00 +0000 (19:35 +0000)]
[ms-inline asm] The IR representation of inline assembly enumerates the input
and output expressions much like that in GNU-style inline assembly. Output
expressions are first. Do this for MS-style inline asms.
Use custom ABIInfo for le32/PNaCl argument codegen
This patch uses a new ABIInfo implementation specific to the le32
target, rather than falling back to DefaultABIInfo. Its behavior is
basically the same, but it also allows the regparm argument attribute.
It also includes basic tests for argument codegen and attributes.
Enabling the UTF conversion routines that were previously conditionally ifdef'ed out. This change is required to support enhancements in the LLDB data formatters
projects / clang / log