Jordan Rose [Wed, 3 Apr 2013 21:16:58 +0000 (21:16 +0000)]
[analyzer] Correctly handle destructors for lifetime-extended temporaries.
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.
Anna Zaks [Wed, 3 Apr 2013 19:28:12 +0000 (19:28 +0000)]
[analyzer] Properly handle the ternary operator in trackNullOrUndefValue
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.)
http://lab.llvm.org:8011/builders/clang-x86_64-darwin10-gdb went back green
before it processed the reverted 178663, so it could not have been the culprit.
Objective-C modern rewriter. Fixes a bug
rewriting typedef for a qualified object type
and also when two declarations happen to be on the
same line. // rdar://13562505
[preprocessor] Allow comparing two macro definitions syntactically instead of only lexically.
Syntactically means the function macro parameter names do not need to use the same
identifiers in order for the definitions to be considered identical.
Syntactic equivalence is a microsoft extension for macro redefinitions and we'll also
use this kind of comparison to check for ambiguous macros coming from modules.
Give the default CorrectionCandidateCallback::ValidateCandidate some
smarts so that it doesn't approve of keywords and/or type names when it
knows (based on its flags) that those kinds of corrections are not
wanted.
For variables and functions clang used to store two storage classes. The one
"as written" in the code and a patched one, which, for example, propagates
static to the following decls.
This apparently is from the days clang lacked linkage computation. It is now
redundant and this patch removes it.
Daniel Jasper [Wed, 3 Apr 2013 13:36:17 +0000 (13:36 +0000)]
Improve formatting of for loops and multi-variable DeclStmts.
This combines several related changes:
a) Don't break before after the variable types in for loops with a
single variable.
b) Better indent DeclStmts defining multiple variables.
Even better way to handle comments adjacent to preprocessor directives.
Summary:
It turns out that we don't need to store CommentsBeforeNextToken in the
line state, but rather flush them before we start parsing preprocessor
directives. This fixes wrong comment indentation in code blocks in macro calls
(the test is included).
Richard Trieu [Wed, 3 Apr 2013 02:22:12 +0000 (02:22 +0000)]
Fix a crasher in Template Diffing.
When support was added for declaration arguments, the case of variadic
declaration arguments was not supported. This patch fixes that problem by
not crashing when certain ValueDecl's are null.
Richard Trieu [Wed, 3 Apr 2013 02:11:36 +0000 (02:11 +0000)]
Fix a crasher in Template Diffing.
Value depenedent expressions for default arguments cannot be evaluated.
Instead, use the desugared template type to get an argument expression that
can be used. This is needed for both integer and declaration arguements.
Also, move this common code into a separate function.
From PR9121 gcc defaulted to omitting the frame pointer on linux,
however, it doesn't do that unless we're optimizing. Change
that and haul out to a helper function. Also make this a driver
test appropriate rather than an assembly test.
Jordan Rose [Wed, 3 Apr 2013 01:39:23 +0000 (01:39 +0000)]
Escape more @ signs in Doxygen comments.
Doxygen treats "@command" the same as "\command" in a doc comment, so
whenever we talk about Objective-C things like "@interface" we have to
make sure to escape them.
Jordan Rose [Wed, 3 Apr 2013 01:39:08 +0000 (01:39 +0000)]
[analyzer] Better model for copying of array fields in implicit copy ctors.
- 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.
John McCall [Wed, 3 Apr 2013 00:56:07 +0000 (00:56 +0000)]
In ObjC++ on legacy runtimes, push an EH cleanup as well as
a normal cleanup when entering a @try or @synchronized to
ensure that we clean that up if an exception is triggered.
Apparently GCC did this, so it's hard to argue that we shouldn't
do at least as much.
Objective-C arc [qui]. Don't issue the bridge cast
warning when doing a __bride cast in non-arc
mode (which has no retain count effect).
// rdar://13514210
Richard Smith [Tue, 2 Apr 2013 19:38:47 +0000 (19:38 +0000)]
If a defaulted special member is implicitly deleted, check whether it's
overriding a non-deleted virtual function. The existing check for this doesn't
catch this case, because it fires before we mark the method as deleted.
Objective-C: Provide fixit hints when warning
about 'isa' ivar being explicitely accessed
when base is a user class object reference.
// rdar://13503456
Fixed "fallthrough annotation does not directly precede switch label" warning in
case when [[clang::fallthrough]]; is used in a method of a local class.
Daniel Jasper [Tue, 2 Apr 2013 14:33:13 +0000 (14:33 +0000)]
Fix some inconsistent use of indentation.
Basically we have always special-cased the top-level statement of an
unwrapped line (the one with ParenLevel == 0) and that lead to several
inconsistencies. All added tests were formatted in a strange way, for
example:
John McCall [Tue, 2 Apr 2013 02:48:58 +0000 (02:48 +0000)]
Add -Wstatic-local-in-inline, which warns about using a static local
variable in a C99 inline (but not static-inline or extern-inline)
function definition.
The standard doesn't actually say that this doesn't apply to
"extern inline" definitions, but that seems like a useful extension,
and it at least doesn't have the obvious flaw that a static
mutable variable in an externally-available definition does.
Anna Zaks [Tue, 2 Apr 2013 01:28:24 +0000 (01:28 +0000)]
[analyzer] Teach invalidateRegions that regions within LazyCompoundVal need 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).
Adrian Prantl [Tue, 2 Apr 2013 01:00:48 +0000 (01:00 +0000)]
un-break remaining gdb buildbot testcases.
Make sure we do not generate line info for debugging-related frame setup.
Follow-up to r178361 / rdar://problem/12767564
Jordan Rose [Tue, 2 Apr 2013 00:26:35 +0000 (00:26 +0000)]
[analyzer] For now, don't inline [cd]tors of C++ containers.
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.
Jordan Rose [Tue, 2 Apr 2013 00:26:29 +0000 (00:26 +0000)]
[analyzer] Cache whether a function is generally inlineable.
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.
Jordan Rose [Tue, 2 Apr 2013 00:26:26 +0000 (00:26 +0000)]
[analyzer] Use inline storage in the FunctionSummary DenseMap.
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.
Jordan Rose [Tue, 2 Apr 2013 00:26:15 +0000 (00:26 +0000)]
[analyzer] Allow suppressing diagnostics reported within the 'std' namespace
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.
Richard Smith [Mon, 1 Apr 2013 21:43:41 +0000 (21:43 +0000)]
PR15633: Note that we are EnteringContext when parsing the nested name
specifier for an enumeration. Also fix a crash-on-invalid if a non-dependent
name specifier is used to declare an enum template.
Adrian Prantl [Mon, 1 Apr 2013 19:02:06 +0000 (19:02 +0000)]
* Attempt to un-break gdb buildbot by emitting a lexical block end only
when we actually end a lexical block.
* Added new test for line table / block cleanup.
* Follow-up to r177819 / rdar://problem/13115369
John McCall [Mon, 1 Apr 2013 18:34:28 +0000 (18:34 +0000)]
Only merge down a variable type if the previous declaration was
visible. There's a lot of potential badness in how we're modelling
these things, but getting this much correct is reasonably easy.
Thread safety analysis: Turn on checking for non-scalar types by default.
These were previously enabled as a "beta" feature, but they have now been
extensively tested.
Jordan Rose [Sat, 30 Mar 2013 01:31:42 +0000 (01:31 +0000)]
[analyzer] Handle caching out while evaluating a C++ new expression.
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.
Anna Zaks [Fri, 29 Mar 2013 22:32:38 +0000 (22:32 +0000)]
[analyzer] Address Jordan’s review of r178309 - do not register an extra 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.
Anna Zaks [Fri, 29 Mar 2013 22:32:34 +0000 (22:32 +0000)]
[analyzer] Look for a StmtPoint node instead of PostStmt in 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.
When looking for overridden ObjC methods, don't ignore 'hidden' ones.
When using modules we should not ignore overridden methods from
categories that are hidden because the module is not visible.
This will give more consistent results (when imports change) and it's more
correct since the methods are indeed overridden even if they are not "visible"
for lookup purposes.
[cmake] Add clang-headers as a dependency of libclang and if we have to copy them
for the IDE case, also create a symlink inside the libclang.dylib directory.
Benjamin Kramer [Fri, 29 Mar 2013 21:43:21 +0000 (21:43 +0000)]
Sema: Warn on sizeof on binary ops on decayed arrays.
The array will decay into a pointer, creating an unexpected result.
sizeof(array + int) is an easy to make typo for sizeof(array) + int.
This was motivated by a NetBSD security bug, used sizeof(key - r) instead of
sizeof(key) - r, reducing entropy in a random number generator.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/subr_cprng.c.diff?r1=1.14&r2=1.15&only_with_tag=MAIN&f=h
Adrian Prantl [Fri, 29 Mar 2013 19:20:35 +0000 (19:20 +0000)]
Bugfix/Followup for r177086.
* Store the .block_descriptor (instead of self) in the alloca so we
can guarantee that all captured variables are available at -O0.
* Add the missing OpDeref for the alloca.
rdar://problem/12767564
projects / clang / log