Passing a pointer to an uninitialized memory buffer is normally okay,
but if the function is declared to take a pointer-to-const then it's
very unlikely it will be modifying the buffer. In this case the analyzer
should warn that there will likely be a read of uninitialized memory.
This doesn't check all elements of an array, only the first one.
It also doesn't yet check Objective-C methods, only C functions and
C++ methods.
This is controlled by a new check: alpha.core.CallAndMessageUnInitRefArg.
Patch by Per Viberg!
llvm-svn: 203822
Like the binary operator check of r201702, this actually checks the
condition of every if in a chain against every other condition, an
O(N^2) operation. In most cases N should be small enough to make this
practical, and checking all cases like this makes it much more likely
to catch a copy-paste error within the same series of branches.
Part of IdenticalExprChecker; patch by Daniel Fahlgren!
llvm-svn: 203585
null comparison when the pointer is known to be non-null.
This catches the array to pointer decay, function to pointer decay and
address of variables. This does not catch address of function since this
has been previously used to silence a warning.
Pointer to bool conversion is under -Wbool-conversion.
Pointer to null comparison is under -Wtautological-pointer-compare, a sub-group
of -Wtautological-compare.
void foo() {
int arr[5];
int x;
// warn on these conditionals
if (foo);
if (arr);
if (&x);
if (foo == null);
if (arr == null);
if (&x == null);
if (&foo); // no warning
}
llvm-svn: 202216
For now, just ignore them. Later, we could try looking through LazyCompoundVals,
but we at least shouldn't crash.
<rdar://problem/16153464>
llvm-svn: 202212
Somehow both Daniel and I missed the fact that while loops are only identical
if they have identical bodies.
Patch by Daniel Fahlgren!
llvm-svn: 201829
IdenticalExprChecker now warns if any expressions in a logical or bitwise
chain (&&, ||, &, |, or ^) are the same. Unlike the previous patch, this
actually checks all subexpressions against each other (an O(N^2) operation,
but N is likely to be small).
Patch by Daniel Fahlgren!
llvm-svn: 201702
This extends the checks for identical expressions to handle identical
statements, and compares the consequent and alternative ("then" and "else")
branches of an if-statement to see if they are identical, treating a single
statement surrounded by braces as equivalent to one without braces.
This does /not/ check subsequent branches in an if/else chain, let alone
branches that are not consecutive. This may improve in a future patch, but
it would certainly take more work.
Patch by Daniel Fahlgren!
llvm-svn: 201701
This will let us stage in the modeling of operator new. The -analyzer-config
opton 'c++-inline-allocators' is currently off by default.
Patch by Karthik Bhat!
llvm-svn: 201122
This means always walking the whole call stack for the end path node, but
we'll assume that's always fairly tractable.
<rdar://problem/15952973>
llvm-svn: 200980
redeclaration, not just when looking them up for a use -- we need the implicit
declaration to appropriately check various properties of them (notably, whether
they're deleted).
llvm-svn: 200729
Due to statement expressions supported as GCC extension, it is possible
to put 'break' or 'continue' into a loop/switch statement but outside
its body, for example:
for ( ; ({ if (first) { first = 0; continue; } 0; }); )
This code is rejected by GCC if compiled in C mode but is accepted in C++
code. GCC bug 44715 tracks this discrepancy. Clang used code generation
that differs from GCC in both modes: only statement of the third
expression of 'for' behaves as if it was inside loop body.
This change makes code generation more close to GCC, considering 'break'
or 'continue' statement in condition and increment expressions of a
loop as it was inside the loop body. It also adds error for the cases
when 'break'/'continue' appear outside loop due to this syntax. If
code generation differ from GCC, warning is issued.
Differential Revision: http://llvm-reviews.chandlerc.com/D2518
llvm-svn: 199897
If there are non-trivially-copyable types /other/ than C++ records, we
won't have a synthesized copy expression, but we can't just use a simple
load/return.
Also, add comments and shore up tests, making sure to test in both ARC
and non-ARC.
llvm-svn: 199869
Citation: C++11 [expr.shift]p1 (and the equivalent text in C11).
This fixes PR18073, but the right thing to do (as noted in the FIXME) is to
have a real checker for too-large shifts.
llvm-svn: 199405
test the CC1 layer.
This actually uncovered that the test semes to no longer be passing for
the reasons intended. =[ The name of the test would lead me to believe
that it should be testing the semantics of noreturn in the static
analyzer.... but there are in fact no -verify assertions about noreturn
that i can find. And the noreturn checker is no longer in 'alpha.core'.
It is in 'core.builtins'. The test *does* have one assertion for a null
dereference warning. This *also* isn't in 'alpha.core', but the driver
inserts a pile of other checker packages, including 'core' which has
this warning.
So I have switch the RUN line to actually do the minimal thing that this
test currently exercises, but someone who works on the static analyzer
should probably look at this and either nuke it or move it to actually
check the noreturn behavior.
llvm-svn: 199307
In preparation for making the Win32 triple imply MS ABI mode,
make all tests pass in this mode, or make them use the Itanium
mode explicitly.
Differential Revision: http://llvm-reviews.chandlerc.com/D2401
llvm-svn: 199130
In an expression like "new (a, b) Foo(x, y)", two things happen:
- Memory is allocated by calling a function named 'operator new'.
- The memory is initialized using the constructor for 'Foo'.
Currently the analyzer only models the second event, though it has special
cases for both the default and placement forms of operator new. This patch
is the first step towards properly modeling both events: it changes the CFG
so that the above expression now generates the following elements.
1. a
2. b
3. (CFGNewAllocator)
4. x
5. y
6. Foo::Foo
The analyzer currently ignores the CFGNewAllocator element, but the next
step is to treat that as a call like any other.
The CFGNewAllocator element is not added to the CFG for analysis-based
warnings, since none of them take advantage of it yet.
llvm-svn: 199123
...by synthesizing their body to be "return self->_prop;", with an extra
nudge to RetainCountChecker to still treat the value as +0 if we have no
other information.
This doesn't handle weak properties, but that's mostly correct anyway,
since they can go to nil at any time. This also doesn't apply to properties
whose implementations we can't see, since they may not be backed by an
ivar at all. And finally, this doesn't handle properties of C++ class type,
because we can't invoke the copy constructor. (Sema has actually done this
work already, but the AST it synthesizes is one the analyzer doesn't quite
handle -- it has an rvalue DeclRefExpr.)
Modeling setters is likely to be more difficult (since it requires
handling strong/copy), but not impossible.
<rdar://problem/11956898>
llvm-svn: 198953
...rather somewhere in the destructor when we try to access something and
realize the object has already been deleted. This is necessary because
the destructor is processed before the 'delete' itself.
Patch by Karthik Bhat!
llvm-svn: 198779
...even though the argument is declared "const void *", because this is
just a way to pass pointers around as objects. (Though NSData is often
a better one.)
PR18262
llvm-svn: 198710
This checker has not been updated to work with interprocedural analysis,
and actually contains both logical correctness issues but also
memory bugs. We can resuscitate it from version control once there
is focused interest in making it a real viable checker again.
llvm-svn: 198476
We have assertions for this, but a few edge cases had snuck through where
we were still unconditionally using 'int'.
<rdar://problem/15703011>
llvm-svn: 197733
Fixes <rdar://problem/15584219> and <rdar://problem/12241361>.
This change looks large, but all it does is reuse and consolidate
the delayed diagnostic logic for deprecation warnings with unavailability
warnings. By doing so, it showed various inconsistencies between the
diagnostics, which were close, but not consistent. It also revealed
some missing "note:"'s in the deprecated diagnostics that were showing
up in the unavailable diagnostics, etc.
This change also changes the wording of the core deprecation diagnostics.
Instead of saying "function has been explicitly marked deprecated"
we now saw "'X' has been been explicitly marked deprecated". It
turns out providing a bit more context is useful, and often we
got the actual term wrong or it was not very precise
(e.g., "function" instead of "destructor"). By just saying the name
of the thing that is deprecated/deleted/unavailable we define
this issue away. This diagnostic can likely be further wordsmithed
to be shorter.
llvm-svn: 197627
cstring, converted to NSString, produce the
matching AST for it. This also required some
refactoring of the previous code. // rdar://14106083
llvm-svn: 197605
This patch was submitted to the list for review and didn't receive a LGTM.
(In fact one explicit objection and one query were raised.)
This reverts commit r197295.
llvm-svn: 197299
Previously, a line like
// expected-error-re {{foo}}
treats the entirety of foo as a regex. This is inconvenient when matching type
names containing regex characters. For example, to match
"void *(class test8::A::*)(void)" inside such a regex, one would have to type
"void \*\(class test8::A::\*\)\(void\)".
This patch changes the semantics of expected-error-re to only treat the parts
of the directive wrapped in double curly braces as regexes. This avoids the
escaping problem and leads to nicer patterns for those cases; see e.g. the
change to test/Sema/format-strings-scanf.c.
(The balanced search for closing }} of a directive also makes us handle the
full directive in test\SemaCXX\constexpr-printing.cpp:41 and :53.)
Differential Revision: http://llvm-reviews.chandlerc.com/D2388
llvm-svn: 197092
Warn if both result expressions of a ternary operator (? :) are the same.
Because only one of them will be executed, this warning will fire even if
the expressions have side effects.
Patch by Anders Rönnholm and Per Viberg!
llvm-svn: 196937
This is another regression fixed by reverting r189090.
In this case, the problem is not live variables but the approach that was taken in r189090. This regression was caused by explicitly binding "true" to the condition when we take the true branch. Normally that's okay, but in this case we're planning to reuse that condition as the value of the expression.
llvm-svn: 196599
This reverts commit r189090.
The original patch introduced regressions (see the added live-variables.* tests). The patch depends on the correctness of live variable analyses, which are not computed correctly. I've opened PR18159 to track the proper resolution to this problem.
The patch was a stepping block to r189746. This is why part of the patch reverts temporary destructor tests that started crashing. The temporary destructors feature is disabled by default.
llvm-svn: 196593
New rules of invalidation/escape of the source buffer of memcpy: the source buffer contents is invalidated and escape while the source buffer region itself is neither invalidated, nor escape.
In the current modeling of memcpy the information about allocation state of regions, accessible through the source buffer, is not copied to the destination buffer and we can not track the allocation state of those regions anymore. So we invalidate/escape the source buffer indirect regions in anticipation of their being invalidated for real later. This eliminates false-positive leaks reported by the unix.Malloc and alpha.cplusplus.NewDeleteLeaks checkers for the cases like
char *f() {
void *x = malloc(47);
char *a;
memcpy(&a, &x, sizeof a);
return a;
}
llvm-svn: 194953
This is similar to r194004: because we can't reason about the data structure
invariants of std::basic_string, the analyzer decides it's possible for an
allocator to be used to deallocate the string's inline storage. Just ignore
this by walking up the stack, skipping past methods in classes with
"allocator" in the name, and seeing if we reach std::basic_string that way.
PR17866
llvm-svn: 194764
This syntactic checker looks for expressions on both sides of comparison
operators that are structurally the same. As a special case, the
floating-point idiom "x != x" for "isnan(x)" is left alone.
Currently this only checks comparison operators, but in the future we could
extend this to include logical operators or chained if-conditionals.
Checker by Per Viberg!
llvm-svn: 194236
An Objective-C for-in loop will have zero iterations if the collection is
empty. Previously, we could only detect this case if the program asked for
the collection's -count /before/ the for-in loop. Now, the analyzer
distinguishes for-in loops that had zero iterations from those with at
least one, and can use this information to constrain the result of calling
-count after the loop.
In order to make this actually useful, teach the checker that methods on
NSArray, NSDictionary, and the other immutable collection classes don't
change the count.
<rdar://problem/14992886>
llvm-svn: 194235
The path note that says "Loop body executed 0 times" has been changed to
"Loop body skipped when range is empty" for C++11 for-range loops, and to
"Loop body skipped when collection is empty" for Objective-C for-in loops.
Part of <rdar://problem/14992886>
llvm-svn: 194234
We could certainly be more precise in many of our diagnostics, but before we
were printing "Assuming x is && y", which is just ridiculous.
<rdar://problem/15167979>
llvm-svn: 193455
This ensures that variables accessible through a union are invalidated when
the union value is passed to a function. We still don't fully handle union
values, but this should at least quiet some false positives.
PR16596
llvm-svn: 193265
This patch wasn't reviewed, and isn't correctly preserving the behaviors
relied upon by QT. I don't have a direct example of fallout, but it
should go through the standard code review process. For example, it
should never have removed the QT test case that was added when fixing
those users.
llvm-svn: 193174
Due to statement expressions supported as GCC extension, it is possible
to put 'break' or 'continue' into a loop/switch statement but outside its
body, for example:
for ( ; ({ if (first) { first = 0; continue; } 0; }); )
Such usage must be diagnosed as an error, GCC rejects it. To recognize
this and similar patterns the flags BreakScope and ContinueScope are
temporarily turned off while parsing condition expression.
Differential Revision: http://llvm-reviews.chandlerc.com/D1762
llvm-svn: 193073
Since these aren't lexically in the constructor, drawing arrows would
be a horrible jump across the body of the class. We could still do
better here by skipping over unimportant initializers, but this at least
keeps everything within the body of the constructor.
<rdar://problem/14960554>
llvm-svn: 192818
This will emit a warning if a call to clang_analyzer_warnIfReached is
executed, printing REACHABLE. This is a more explicit way to declare
expected reachability than using clang_analyzer_eval or triggering
a bug (divide-by-zero or null dereference), and unlike the former will
work the same in inlined functions and top-level functions. Like the
other debug helpers, it is part of the debug.ExprInspection checker.
Patch by Jared Grubb!
llvm-svn: 191909
Also add some tests that there is actually a message and that the bug is
actually a hard error. This actually behaved correctly before, because:
- addTransition() doesn't actually add a transition if the new state is null;
it assumes you want to propagate the predecessor forward and does nothing.
- generateSink() is called in order to emit a bug report.
- If at least one new node has been generated, the predecessor node is /not/
propagated forward.
But now it's spelled out explicitly.
Found by Richard Mazorodze, who's working on a patch that may require this.
llvm-svn: 191805
...rather than trying to figure it out from the call site, and having
people complain that we guessed wrong and that a prototype-less call is
the same as a variadic call on their system. More importantly, fix a
crash when there's no decl at the call site (though we could have just
returned a default value).
<rdar://problem/15037033>
llvm-svn: 191599
Now that the CFG includes nodes for the destructors in a delete-expression,
process them in the analyzer using the same common destructor interface
currently used for local, member, and base destructors. Also, check for when
the value is known to be null, in which case no destructor is actually run.
This does not yet handle destructors for deleted /arrays/, which may need
more CFG work. It also causes a slight regression in the location of
double delete warnings; the double delete is detected at the destructor
call, which is implicit, and so is reported on the first access within the
destructor instead of at the 'delete' statement. This will be fixed soon.
Patch by Karthik Bhat!
llvm-svn: 191381
We now have symbols with floating-point type to make sure that
(double)x == (double)x comes out true, but we still can't do much with
these. For now, don't even bother trying to create a floating-point zero
value; just give up on conversion to bool.
PR14634, C++ edition.
llvm-svn: 190953
"+method_name: cannot take ownership of memory allocated by 'new'."
instead of the old
"Memory allocated by 'new' should be deallocated by 'delete', not +method_name"
llvm-svn: 190800
variable uninitialized every time we reach its (reachable) declaration, or
every time we call the surrounding function, promote the warning from
-Wmaybe-uninitialized to -Wsometimes-uninitialized.
This is still slightly weaker than desired: we should, in general, warn
if a use is uninitialized the first time it is evaluated.
llvm-svn: 190623
RegionStore tries to protect against accidentally initializing the same
region twice, but it doesn't take subregions into account very well. If
the outer region being initialized is a struct with an empty base class,
the offset of the first field in the struct will be 0. When we initialize
the base class, we may invalidate the contents of the struct by providing
a default value of Unknown (or some new symbol). We then go to initialize
the member with a zeroing constructor, only to find that the region at
that offset in the struct already has a value. The best we can do here is
to invalidate that value and continue; neither the old default value nor
the new 0 is correct for the entire struct after the member constructor call.
The correct solution for this is to track region extents in the store.
<rdar://problem/14914316>
llvm-svn: 190530
Summary:
If a noreturn destructor is executed while returning a value from a function,
the resulting CFG has had two edges to the exit block. This crashed the analyzer,
because it expects that blocks with no terminators have only one outgoing edge.
I added code to avoid creating the second edge in this case.
PS: The crashes did not manifest themselves always, as usually the
NoReturnFunctionChecker would stop program evaluation before the analyzer hit
the assertion, but in the case of lifetime extended temporaries, the checker
failed to do that (which is a separate bug in itself).
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1513
llvm-svn: 190125
Summary:
I've had a test failure here while experimenting and I've found that it's
impossible to find what is wrong with the previous structure of the file. So I
have grouped the expected output with the function that produces it, to make
searching for discrepancies more obvious.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1595
llvm-svn: 190037
This paves the way for adding support for modeling the destructor of a
region before it is deleted. The statement "delete <expr>" now generates
this series of CFG elements:
1. <expr>
2. [B1.1]->~Foo() (Implicit destructor)
3. delete [B1.1]
Patch by Karthik Bhat!
llvm-svn: 189828
This is an improved version of r186498. It enables ExprEngine to reason about
temporary object destructors. However, these destructor calls are never
inlined, since this feature is still broken. Still, this is sufficient to
properly handle noreturn temporary destructors.
Now, the analyzer correctly handles expressions like "a || A()", and executes the
destructor of "A" only on the paths where "a" evaluted to false.
Temporary destructor processing is still off by default and one has to
explicitly request it by setting cfg-temporary-dtors=true.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1259
llvm-svn: 189746
This will never happen in the analyzed code code, but can happen for checkers
that over-eagerly dereference pointers without checking that it's safe.
UnknownVal is a harmless enough value to get back.
Fixes an issue added in r189590, caught by our internal buildbot.
llvm-svn: 189688
Summary:
Previously, Sema was reusing parts of the AST when synthesizing an assignment
operator, turning it into a AS-dag. This caused problems for the static
analyzer, which assumed an expression appears in the tree only once.
Here I make sure to always create a fresh Expr, when inserting something into
the AST, fixing PR16745 in the process.
Reviewers: doug.gregor
CC: cfe-commits, jordan_rose
Differential Revision: http://llvm-reviews.chandlerc.com/D1425
llvm-svn: 189659
Summary:
RegionStoreManager had an optimization which replaces references to empty
structs with UnknownVal. Unfortunately, this check didn't take into account
possible field members in base classes.
To address this, I changed this test to "is empty and has no base classes". I
don't consider it worth the trouble to go through base classes and check if all
of them are empty.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1547
llvm-svn: 189590
When casting the address of a FunctionTextRegion to bool, or when adding
constraints to such an address, use a stand-in symbol to represent the
presence or absence of the function if the function is weakly linked.
This is groundwork for possible simple availability testing checks, and
can already catch mistakes involving inverted null checks for
weakly-linked functions.
Currently, the implementation reuses the "extent" symbols, originally created
for tracking the size of a malloc region. Since FunctionTextRegions cannot
be dereferenced, the extent symbol will never be used for anything else.
Still, this probably deserves a refactoring in the future.
This patch does not attempt to support testing the presence of weak
/variables/ (global variables), which would likely require much more of
a change and a generalization of "region structure metadata", like the
current "extents", vs. "region contents metadata", like CStringChecker's
"string length".
Patch by Richard <tarka.t.otter@googlemail.com>!
llvm-svn: 189492
Summary:
-fno-exceptions does not implicitly attach a nothrow specifier to every operator
new. Even in this mode, non-nothrow new must not return a null pointer. Failure
to allocate memory can be signalled by other means, or just by killing the
program. This behaviour is consistent with the compiler - even with
-fno-exceptions, the generated code never tests for null (and would segfault if
the opeator actually happened to return null).
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1528
llvm-svn: 189452
Summary:
Instead of digging through the ExplodedGraph, to figure out which edge brought
us here, I compute the value of conditional expression by looking at the
sub-expression values.
To do this, I needed to change the liveness algorithm a bit -- now, the full
conditional expression also depends on all atomic sub-expressions, not only the
outermost ones.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1340
llvm-svn: 189090
This is still an alpha checker, but we use it in certain tests to make sure
something is not being executed.
This should fix the buildbots.
llvm-svn: 188682
This keeps the analyzer from making silly assumptions, like thinking
strlen(foo)+1 could wrap around to 0. This fixes PR16558.
Patch by Karthik Bhat!
llvm-svn: 188680
This builtin does not actually evaluate its arguments for side effects,
so we shouldn't include them in the CFG. In the analyzer, rely on the
constant expression evaluator to get the proper semantics, at least for
now. (In the future, we could get ambitious and try to provide path-
sensitive size values.)
In theory, this does pose a problem for liveness analysis: a variable can
be used within the __builtin_object_size argument expression but not show
up as live. However, it is very unlikely that such a value would be used
to compute the object size and not used to access the object in some way.
<rdar://problem/14760817>
llvm-svn: 188679
This once again restores notes to following their associated warnings
in -analyzer-output=text mode. (This is still only intended for use as a
debugging aid.)
One twist is that the warning locations in "regular" analysis output modes
(plist, multi-file-plist, html, and plist-html) are reported at a different
location on the command line than in the output file, since the command
line has no path context. This commit makes -analyzer-output=text behave
like a normal output format, which means that the *command line output
will be different* in -analyzer-text mode. Again, since -analyzer-text is
a debugging aid and lo-fi stand-in for a regular output mode, this change
makes sense.
Along the way, remove a few pieces of stale code related to the path
diagnostic consumers.
llvm-svn: 188514
When a region is realloc()ed, MallocChecker records whether it was known
to be allocated or not. If it is, and the reallocation fails, the original
region has to be freed. Previously, when an allocated region escaped,
MallocChecker completely stopped tracking it, so a failed reallocation
still (correctly) wouldn't require freeing the original region. Recently,
however, MallocChecker started tracking escaped symbols, so that if it were
freed we could check that the deallocator matched the allocator. This
broke the reallocation model for whether or not a symbol was allocated.
Now, MallocChecker will actually check if a symbol is owned, and only
require freeing after a failed reallocation if it was owned before.
PR16730
llvm-svn: 188468
Various tests had sprung up over the years which had --check-prefix=ABC on the
RUN line, but "CHECK-ABC:" later on. This happened to work before, but was
strictly incorrect. FileCheck is getting stricter soon though.
Patch by Ron Ofir.
llvm-svn: 188174
Summary:
ExprEngine had code which specificaly disabled using CXXTempObjectRegions in
InitListExprs. This was a hack put in r168757 to silence a false positive.
The underlying problem seems to have been fixed in the mean time, as removing
this code doesn't seem to break anything. Therefore I propose to remove it and
solve PR16629 in the process.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1325
llvm-svn: 188059
We process autorelease counts when we exit functions, but if there's an
issue in a synthesized body the report will get dropped. Just skip the
processing for now and let it get handled when the caller gets around to
processing autoreleases.
(This is still suboptimal: objects autoreleased in the caller context
should never be warned about when exiting a callee context, synthesized
or not.)
Second half of <rdar://problem/14611722>
llvm-svn: 187625
Much of our diagnostic machinery is set up to assume that the report
end path location is valid. Moreover, the user may be quite confused
when something goes wrong in our BodyFarm-synthesized function bodies,
which may be simplified or modified from the real implementations.
Rather than try to make this all work somehow, just drop the report so
that we don't try to go on with an invalid source location.
Note that we still handle reports whose /paths/ go through invalid
locations, just not those that are reported in one.
We do have to be careful not to lose warnings because of this.
The impetus for this change was an autorelease being processed within
the synthesized body, and there may be other possible issues that are
worth reporting in some way. We'll take these as they come, however.
<rdar://problem/14611722>
llvm-svn: 187624
Summary:
When binding a temporary object to a static local variable, the analyzer would
complain about a dangling reference even though the temporary's lifetime should
be extended past the end of the function. This commit tries to detect these
cases and construct them in a global memory region instead of a local one.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1133
llvm-svn: 187196
These are cases where a scalar type is "destructed", usually due to
template instantiation (e.g. "obj.~T()", where 'T' is 'int'). This has
no actual effect and the analyzer should just skip over it.
llvm-svn: 186927
The analyzer doesn't currently expect CFG blocks with terminators to be
empty, but this can happen when generating conditional destructors for
a complex logical expression, such as (a && (b || Temp{})). Moreover,
the branch conditions for these expressions are not persisted in the
state. Even for handling noreturn destructors this needs more work.
This reverts r186498.
llvm-svn: 186925
Previously, we would simply abort the path when we saw a default member
initialization; now, we actually attempt to evaluate it. Like default
arguments, the contents of these expressions are not actually part of the
current function, so we fall back to constant evaluation.
llvm-svn: 186521
Previously, SValBuilder knew how to evaluate StringLiterals, but couldn't
handle an array-to-pointer decay for constant values. Additionally,
RegionStore was being too strict about loading from an array, refusing to
return a 'char' value from a 'const char' array. Both of these have been
fixed.
llvm-svn: 186520
Previously, the use of a std::initializer_list (actually, a
CXXStdInitializerListExpr) would cause the analyzer to give up on the rest
of the path. Now, it just uses an opaque symbolic value for the
initializer_list and continues on.
At some point in the future we can add proper support for initializer_list,
with access to the elements in the InitListExpr.
<rdar://problem/14340207>
llvm-svn: 186519
Summary:
This patch enables ExprEndgine to reason about temporary object destructors.
However, these destructor calls are never inlined, since this feature is still
broken. Still, this is sufficient to properly handle noreturn temporary
destructors and close bug #15599. I have also enabled the cfg-temporary-dtors
analyzer option by default.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1131
llvm-svn: 186498
Previously, we asserted that whenever 'new' did not include a constructor
call, the type must be a non-record type. In C++11, however, uniform
initialization syntax (braces) allow 'new' to construct records with
list-initialization: "new Point{1, 2}".
Removing this assertion should be perfectly safe; the code here matches
what VisitDeclStmt does for regions allocated on the stack.
<rdar://problem/14403437>
llvm-svn: 186028
list is the name of a class, not a namespace. Change the test as well - the previous
version did not test properly.
Fixes radar://14317928.
llvm-svn: 185898
The motivation is to suppresses false use-after-free reports that occur when calling
std::list::pop_front() or std::list::pop_back() twice. The analyzer does not
reason about the internal invariants of the list implementation, so just do not report
any of warnings in std::list.
Fixes radar://14317928.
llvm-svn: 185609
Summary:
The analyzer incorrectly handled noreturn destructors which were hidden inside
function calls. This happened because NoReturnFunctionChecker only listened for
PostStmt events, which are not executed for destructor calls. I've changed it to
listen to PostCall events, which should catch both cases.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1056
llvm-svn: 185522
While we don't model pointers-to-members besides "null" and "non-null",
we were using Loc symbols for valid pointers and NonLoc integers for the
null case. This hit the assert committed in r185401.
Fixed by using a true (Loc) null for null member pointers.
llvm-svn: 185444
Summary:
Static analyzer used to abort when encountering AttributedStmts, because it
asserted that the statements should not appear in the CFG. This is however not
the case, since at least the clang::fallthrough annotation makes it through.
This commit simply makes the analyzer ignore the statement attributes.
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1030
llvm-svn: 185417
Re-apply r184511, reverted in r184561, with the trivial default constructor
fast path removed -- it turned out not to be necessary here.
Certain expressions can cause a constructor invocation to zero-initialize
its object even if the constructor itself does no initialization. The
analyzer now handles that before evaluating the call to the constructor,
using the same "default binding" mechanism that calloc() uses, rather
than simply ignoring the zero-initialization flag.
<rdar://problem/14212563>
llvm-svn: 184815
In order to make sure virtual base classes are always initialized once,
the AST contains initializers for the base class in /all/ of its
descendents, not just the immediate descendents. However, at runtime,
the most-derived object is responsible for initializing all the virtual
base classes; all the other initializers will be ignored.
The analyzer now checks to see if it's being called from another base
constructor, and if so does not perform virtual base initialization.
<rdar://problem/14236851>
llvm-svn: 184814
This fixes false positives by allowing us to know that a loop is always entered if
the collection count method returns a positive value and vice versa.
Addresses radar://14169391.
llvm-svn: 184618
Per review from Anna, this really should have been two commits, and besides
it's causing problems on our internal buildbot. Reverting until these have
been worked out.
This reverts r184511 / 98123284826bb4ce422775563ff1a01580ec5766.
llvm-svn: 184561
Certain expressions can cause a constructor invocation to zero-initialize
its object even if the constructor itself does no initialization. The
analyzer now handles that before evaluating the call to the constructor,
using the same "default binding" mechanism that calloc() uses, rather
than simply ignoring the zero-initialization flag.
As a bonus, trivial default constructors are now no longer inlined; they
are instead processed explicitly by ExprEngine. This has a (positive)
effect on the generated path edges: they no longer stop at a default
constructor call unless there's a user-provided implementation.
<rdar://problem/14212563>
llvm-svn: 184511
Summary:
When doing a reinterpret+dynamic cast from an incomplete type, the analyzer
would crash (bug #16308). This fix makes the dynamic cast evaluator ignore
incomplete types, as they can never be used in a dynamic_cast. Also adding a
regression test.
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1006
llvm-svn: 184403
Summary:
When processing a call to a function, which got passed less arguments than it
expects, the analyzer would crash.
I've also added a test for that and a analyzer warning which detects these
cases.
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D994
llvm-svn: 184288
This silences warnings that could occur when one is swapping partially initialized structs. We suppress
not only the assignments of uninitialized members, but any values inside swap because swap could
potentially be used as a subroutine to swap class members.
This silences a warning from std::try::function::swap() on partially initialized objects.
llvm-svn: 184256
We drew the diagnostic edges to wrong statements in cases the note was on a macro.
The fix is simple, but seems to work just fine for a whole bunch of test cases (plist-macros.cpp).
Also, removes an unnecessary edge in edges-new.mm, when function signature starts with a macro.
llvm-svn: 183599
The function in which we were doing it used to be conditionalized. Add a new unconditional
cleanup step.
This fixes PR16227 (radar://14073870) - a crash when generating html output for one of the test files.
llvm-svn: 183451
Previously our edges were completely broken here; now, the final result
is a very simple set of edges in most cases: one up to the "for" keyword
for context, and one into the body of the loop. This matches the behavior
for ObjC for-in loops.
In the AST, however, CXXForRangeStmts are handled very differently from
ObjCForCollectionStmts. Since they are specified in terms of equivalent
statements in the C++ standard, we actually have implicit AST nodes for
all of the semantic statements. This makes evaluation very easy, but
diagnostic locations a bit trickier. Fortunately, the problem can be
generally defined away by marking all of the implicit statements as
part of the top-level for-range statement.
One of the implicit statements in a for-range statement is the declaration
of implicit iterators __begin and __end. The CFG synthesizes two
separate DeclStmts to match each of these decls, but until now these
synthetic DeclStmts weren't in the function's ParentMap. Now, the CFG
keeps track of its synthetic statements, and the AnalysisDeclContext will
make sure to add them to the ParentMap.
<rdar://problem/14038483>
llvm-svn: 183449
We based decisions during analysis and during path generation on whether
or not an expression is consumed, so if a top-level expression has
cleanups it's important for us to look through that.
<rdar://problem/14076125>
llvm-svn: 183368
We previously asserted that there was a top-level function entry edge, but
if the function decl's location is invalid (or within a macro) this edge
might not exist. Change the assertion to an actual check, and don't drop
the first path piece if it doesn't match.
<rdar://problem/14070304>
llvm-svn: 183358
Jordan Rose