Commit Graph

112 Commits

Author SHA1 Message Date
Anna Zaks 30d4668774 [analyzer] Fix missed leak from MSVC specific allocation functions
Add the wide character strdup variants (wcsdup, _wcsdup) and the MSVC
version of alloca (_alloca) and other differently named function used
by the Malloc checker.

A patch by Alexander Riccio!

Differential Revision: http://reviews.llvm.org/D17688

llvm-svn: 262894
2016-03-08 01:21:51 +00:00
Anna Zaks fe1eca5169 [analyzer] Assume escape is possible through system functions taking void*
The analyzer assumes that system functions will not free memory or modify the
arguments in other ways, so we assume that arguments do not escape when
those are called. However, this may lead to false positive leak errors. For
example, in code like this where the pointers added to the rb_tree are freed
later on:

		struct alarm_event *e = calloc(1, sizeof(*e));
<snip>

		rb_tree_insert_node(&alarm_tree, e);

Add a heuristic to assume that calls to system functions taking void*
arguments allow for pointer escape.

llvm-svn: 251449
2015-10-27 20:19:45 +00:00
Devin Coughlin 8177173757 [analyzer] Make realloc(ptr, 0) handling equivalent to malloc(0).
Currently realloc(ptr, 0) is treated as free() which seems to be not correct. C
standard (N1570) establishes equivalent behavior for malloc(0) and realloc(ptr,
0): "7.22.3 Memory management functions calloc, malloc, realloc: If the size of
the space requested is zero, the behavior is implementation-defined: either a
null pointer is  returned, or the behavior is as if the size were some nonzero
value, except that the returned pointer shall not be used to access an object."
The patch equalizes the processing of malloc(0) and realloc(ptr,0). The patch
also enables unix.Malloc checker to detect references to zero-allocated memory
returned by realloc(ptr,0) ("Use of zero-allocated memory" warning).

A patch by Антон Ярцев!

Differential Revision: http://reviews.llvm.org/D9040

llvm-svn: 248336
2015-09-22 22:47:14 +00:00
Devin Coughlin e39bd407ba [analyzer] Add generateErrorNode() APIs to CheckerContext.
The analyzer trims unnecessary nodes from the exploded graph before reporting
path diagnostics. However, in some cases it can trim all nodes (including the
error node), leading to an assertion failure (see
https://llvm.org/bugs/show_bug.cgi?id=24184).

This commit addresses the issue by adding two new APIs to CheckerContext to
explicitly create error nodes. Unless the client provides a custom tag, these
APIs tag the node with the checker's tag -- preventing it from being trimmed.
The generateErrorNode() method creates a sink error node, while
generateNonFatalErrorNode() creates an error node for a path that should
continue being explored.

The intent is that one of these two methods should be used whenever a checker
creates an error node.

This commit updates the checkers to use these APIs. These APIs
(unlike addTransition() and generateSink()) do not take an explicit Pred node.
This is because there are not any error nodes in the checkers that were created
with an explicit different than the default (the CheckerContext's Pred node).

It also changes generateSink() to require state and pred nodes (previously
these were optional) to reduce confusion.

Additionally, there were several cases where checkers did check whether a
generated node could be null; we now explicitly check for null in these places.

This commit also includes a test case written by Ying Yi as part of
http://reviews.llvm.org/D12163 (that patch originally addressed this issue but
was reverted because it introduced false positive regressions).

Differential Revision: http://reviews.llvm.org/D12780

llvm-svn: 247859
2015-09-16 22:03:05 +00:00
Devin Coughlin 00691e3169 Revert "[Static Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph""
This is making our internal build bot fail because it results in extra warnings being
emitted past what should be sink nodes. (There is actually an example of this in the
updated malloc.c test in the reverted commit.)

I'm working on a patch to fix the original issue by adding a new checker API to explicitly
create error nodes. This API will ensure that error nodes are always tagged in order to
prevent them from being reclaimed.

This reverts commit r246188.

llvm-svn: 247103
2015-09-08 23:50:22 +00:00
Gabor Horvath e9984b28ef [Static Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph"
The assertion is caused by reusing a “filler” ExplodedNode as an error node.
The “filler” nodes are only used for intermediate processing and are not
essential for analyzer history, so they can be reclaimed when the
ExplodedGraph is trimmed by the “collectNode” function. When a checker finds a
bug, they generate a new transition in the ExplodedGraph. The analyzer will
try to reuse the existing predecessor node. If it cannot, it creates a new
ExplodedNode, which always has a tag to uniquely identify the creation site.
The assertion is caused when the analyzer reuses a “filler” node.

In the test case, some “filler” nodes were reused and then reclaimed later
when the ExplodedGraph was trimmed. This caused an assertion because the node
was needed to generate the report. The “filler” nodes should not be reused as
error nodes. The patch adds a constraint to prevent this happening, which
solves the problem and makes the test cases pass.

Differential Revision: http://reviews.llvm.org/D11433

Patch by Ying Yi!

llvm-svn: 246188
2015-08-27 18:57:00 +00:00
Anton Yartsev b50f4ba461 [analyzer] This implements potential undefbehavior.ZeroAllocDereference checker.
TODO: support realloc(). Currently it is not possible due to the present realloc() handling. Currently RegionState is not being attached to realloc() in case of a zero Size argument. 
llvm-svn: 234889
2015-04-14 14:18:04 +00:00
Anton Yartsev 9907fc9053 [analyzer] Bug fix: do not report leaks for alloca()
llvm-svn: 231314
2015-03-04 23:18:21 +00:00
Jordan Rose 97d2c9cae7 [analyzer] Teach CastSizeChecker about flexible array members.
...as well as fake flexible array members: structs that end in arrays with
length 0 or 1.

Patch by Daniel Fahlgren!

llvm-svn: 201583
2014-02-18 17:06:30 +00:00
Anna Zaks f5308fac1e Fixup to r196593.
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
2013-12-06 19:28:16 +00:00
Anton Yartsev 968c60a554 [analyzer] Better modeling of memcpy by the CStringChecker (PR16731).
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
2013-11-17 09:18:48 +00:00
Jordan Rose 60619a639b [analyzer] Assume that strings are no longer than SIZE_MAX/4.
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
2013-08-19 16:27:34 +00:00
Jordan Rose 2f8b0229cb [analyzer] If realloc fails on an escaped region, that region doesn't leak.
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
2013-08-15 17:22:06 +00:00
Jordan Rose 757fbb0b14 [analyzer] Indirect invalidation counts as an escape for leak checkers.
Consider this example:

  char *p = malloc(sizeof(char));
  systemFunction(&p);
  free(p);

In this case, when we call systemFunction, we know (because it's a system
function) that it won't free 'p'. However, we /don't/ know whether or not
it will /change/ 'p', so the analyzer is forced to invalidate 'p', wiping
out any bindings it contains. But now the malloc'd region looks like a
leak, since there are no more bindings pointing to it, and we'll get a
spurious leak warning.

The fix for this is to notice when something is becoming inaccessible due
to invalidation (i.e. an imperfect model, as opposed to being explicitly
overwritten) and stop tracking it at that point. Currently, the best way
to determine this for a call is the "indirect escape" pointer-escape kind.

In practice, all the patch does is take the "system functions don't free
memory" special case and limit it to direct parameters, i.e. just the
arguments to a call and not other regions accessible to them. This is a
conservative change that should only cause us to escape regions more
eagerly, which means fewer leak warnings.

This isn't perfect for several reasons, the main one being that this
example is treated the same as the one above:

  char **p = malloc(sizeof(char *));
  systemFunction(p + 1);
  // leak

Currently, "addresses accessible by offsets of the starting region" and
"addresses accessible through bindings of the starting region" are both
considered "indirect" regions, hence this uniform treatment.

Another issue is our longstanding problem of not distinguishing const and
non-const bindings; if in the first example systemFunction's parameter were
a char * const *, we should know that the function will not overwrite 'p',
and thus we can safely report the leak.

<rdar://problem/13758386>

llvm-svn: 181607
2013-05-10 17:07:16 +00:00
Anna Zaks 93a21a8cfe [analyzer] Keep tracking the pointer after the escape to more aggressively report mismatched deallocator
Test that the path notes do not change. I don’t think we should print a note on escape.

Also, I’ve removed a check that assumed that the family stored in the RefStete could be
AF_None and added an assert in the constructor.

llvm-svn: 179075
2013-04-09 00:30:28 +00:00
Anna Zaks a1de8567fc [analyzer] Shorten the malloc checker’s leak message
As per Ted’s suggestion!

llvm-svn: 178938
2013-04-06 00:41:36 +00:00
Jordan Rose 5d22fcb257 [analyzer] Track malloc'd memory into struct fields.
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
2013-03-20 20:35:57 +00:00
Anna Zaks bda130f02a [analyzer] Use isLiveRegion to determine when SymbolRegionValue is dead.
Fixes a FIXME, improves dead symbol collection, suppresses a false positive,
which resulted from reusing the same symbol twice for simulation of 2 calls to the same function.

Fixing this lead to 2 possible false negatives in CString checker. Since the checker is still alpha and
the solution will not require revert of this commit, move the tests to a FIXME section.

llvm-svn: 177206
2013-03-15 23:34:29 +00:00
Anna Zaks c89ad07d39 [analyzer] Report bugs when freeing memory with offset pointer
The malloc checker will now catch the case when a previously malloc'ed
region is freed, but the pointer passed to free does not point to the
start of the allocated memory. For example:

int *p1 = malloc(sizeof(int));
p1++;
free(p1); // warn

From the "memory.LeakPtrValChanged enhancement to unix.Malloc" entry
in the list of potential checkers.

A patch by Branden Archer!

llvm-svn: 174678
2013-02-07 23:05:47 +00:00
Anna Zaks acdc13cb00 [analyzer] Add pointer escape type param to checkPointerEscape callback
The checkPointerEscape callback previously did not specify how a
pointer escaped. This change includes an enum which describes the
different ways a pointer may escape. This enum is passed to the
checkPointerEscape callback when a pointer escapes. If the escape
is due to a function call, the call is passed. This changes
previous behavior where the call is passed as NULL if the escape
was due to indirectly invalidating the region the pointer referenced.

A patch by Branden Archer!

llvm-svn: 174677
2013-02-07 23:05:43 +00:00
Anna Zaks 258f9357ef [analyzer]Revert part of r161511; suppresses leak false positives in C++
This is a "quick fix".

The underlining issue is that when a const pointer to a struct is passed
into a function, we do not invalidate the pointer fields. This results
in false positives that are common in C++ (since copy constructors are
prevalent). (Silences two llvm false positives.)

llvm-svn: 174468
2013-02-06 00:01:14 +00:00
Jordan Rose 3ba8c792e5 [analyzer] Fix test to work on non-LP64 systems.
Thanks for the original catch in r168303, Takumi.

llvm-svn: 168671
2012-11-27 02:37:49 +00:00
NAKAMURA Takumi d05b01a8df clang/test: Suppress two tests on LLP64 target, Windows x64.
llvm-svn: 168303
2012-11-19 10:00:59 +00:00
Jordan Rose e37ab50a6e [analyzer] Report leaks at the closing brace of a function body.
This fixes a few cases where we'd emit path notes like this:

  +---+
 1|   v
  p = malloc(len);
  ^   |2
  +---+

In general this should make path notes more consistent and more correct,
especially in cases where the leak happens on the false branch of an if
that jumps directly to the end of the function. There are a couple places
where the leak is reported farther away from the cause; these are usually
cases where there are several levels of nested braces before the end of
the function. This still matches our current behavior for when there /is/
a statement after all the braces, though.

llvm-svn: 168070
2012-11-15 19:11:43 +00:00
Jordan Rose b5b0fc196e [analyzer] Mark symbol values as dead in the environment.
This allows us to properly remove dead bindings at the end of the top-level
stack frame, using the ReturnStmt, if there is one, to keep the return value
live. This in turn removes the need for a check::EndPath callback in leak
checkers.

This does cause some changes in the path notes for leak checkers. Previously,
a leak would be reported at the location of the closing brace in a function.
Now, it gets reported at the last statement. This matches the way leaks are
currently reported for inlined functions, but is less than ideal for both.

llvm-svn: 168066
2012-11-15 19:11:27 +00:00
Anna Zaks 75cfbb60a8 [analyzer] Fix another false positive in malloc realloc logic.
llvm-svn: 163749
2012-09-12 22:57:34 +00:00
Chandler Carruth 66a34a6a45 Adjust some analyzer tests to place widely shared inputs inside of an
'Inputs' subdirectory.

The general desire has been to have essentially all of the non-test
input files live in such directories, with some exceptions for obvious
and common patterns like 'foo.c' using 'foo.h'.

This came up because our distributed test runner couldn't find some of
the headers, for example with stl.cpp.

No functionality changed, just shuffling around here.

llvm-svn: 163674
2012-09-12 01:11:10 +00:00
Ted Kremenek 722398f1d4 Fix analyzer tests.
llvm-svn: 162588
2012-08-24 20:39:55 +00:00
Anna Zaks fe6eb67b12 [analyzer] Fix realloc related bug in the malloc checker.
When reallocation of a non-allocated (not owned) symbol fails do not
expect it to be freed.

llvm-svn: 162533
2012-08-24 02:28:20 +00:00
Ted Kremenek 9bf9af92a4 Allow multiple PathDiagnosticConsumers to be used with a BugReporter at the same time.
This fixes several issues:

- removes egregious hack where PlistDiagnosticConsumer would forward to HTMLDiagnosticConsumer,
but diagnostics wouldn't be generated consistently in the same way if PlistDiagnosticConsumer
was used by itself.

- emitting diagnostics to the terminal (using clang's diagnostic machinery) is no longer a special
case, just another PathDiagnosticConsumer.  This also magically resolved some duplicate warnings,
as we now use PathDiagnosticConsumer's diagnostic pruning, which has scope for the entire translation
unit, not just the scope of a BugReporter (which is limited to a particular ExprEngine).

As an interesting side-effect, diagnostics emitted to the terminal also have their trailing "." stripped,
just like with diagnostics emitted to plists and HTML.  This required some tests to be updated, but now
the tests have higher fidelity with what users will see.

There are some inefficiencies in this patch.  We currently generate the report graph (from the ExplodedGraph)
once per PathDiagnosticConsumer, which is a bit wasteful, but that could be pulled up higher in the
logic stack.  There is some intended duplication, however, as we now generate different PathDiagnostics (for the same issue)
for different PathDiagnosticConsumers.  This is necessary to produce the diagnostics that a particular
consumer expects.

llvm-svn: 162028
2012-08-16 17:45:23 +00:00
Jordan Rose d86b3bdb7a [analyzer] Clean up the printing of FieldRegions for leaks.
Unfortunately, generalized region printing is very difficult:
- ElementRegions are used both for casting and as actual elements.
- Accessing values through a pointer means going through an intermediate
  SymbolRegionValue; symbolic regions are untyped.
- Referring to implicitly-defined variables like 'this' and 'self' could be
  very confusing if they come from another stack frame.

We fall back to simply not printing the region name if we can't be sure it
will print well. This will allow us to improve in the future.

llvm-svn: 161512
2012-08-08 18:23:36 +00:00
Jordan Rose 356279ca2d [analyzer] Track malloc'd regions stored in structs.
The main blocker on this (besides the previous commit) was that
ScanReachableSymbols was not looking through LazyCompoundVals.
Once that was fixed, it's easy enough to clear out malloc data on return,
just like we do when we bind to a global region.

<rdar://problem/10872635>

llvm-svn: 161511
2012-08-08 18:23:31 +00:00
Anna Zaks 6ce686e6a4 [analyzer] Malloc: remove assert since is not valid as of r161248
We can be in the situation where we did not track the symbol before
realloc was called on it.

llvm-svn: 161294
2012-08-04 02:04:27 +00:00
Anna Zaks 52242a6677 [analyzer] Malloc: track non-allocated but freed memory
There is no reason why we should not track the memory which was not
allocated in the current function, but was freed there. This would
allow to catch more use-after-free and double free with no/limited IPA.

Also fix a realloc issue which surfaced as the result of this patch.

llvm-svn: 161248
2012-08-03 18:30:18 +00:00
Jordan Rose 7ab0182e33 [analyzer] Move the last bits of CallOrObjCMessage over to CallEvent.
This involved refactoring some common pointer-escapes code onto CallEvent,
then having MallocChecker use those callbacks for whether or not to consider
a pointer's /ownership/ as escaping. This still needs to be pinned down, and
probably we want to make the new argumentsMayEscape() function a little more
discerning (content invalidation vs. ownership/metadata invalidation), but
this is a good improvement.

As a bonus, also remove CallOrObjCMessage from the source completely.

llvm-svn: 159557
2012-07-02 19:27:51 +00:00
Anna Zaks 886dfb8cfa [analyzer] Malloc leak false positive: Allow xpc context to escape.
llvm-svn: 158875
2012-06-20 23:35:57 +00:00
Jordan Rose de409b6d5f [analyzer] Buffers passed to CGBitmapContextCreate can escape.
Specifically, although the bitmap context does not take ownership of the
buffer (unlike CGBitmapContextCreateWithData), the data buffer can be extracted
out of the created CGContextRef. Thus the buffer is not leaked even if its
original pointer goes out of scope, as long as
- the context escapes, or
- it is retrieved via CGBitmapContextGetData and freed.

Actually implementing that logic is beyond the current scope of MallocChecker,
so for now CGBitmapContextCreate goes on our system function exception list.

llvm-svn: 158579
2012-06-16 00:09:20 +00:00
Anna Zaks 93205d0d12 [analyze] Change some of the malloc tests to use clang_analyzer_eval.
Thanks, Jordan.

llvm-svn: 158179
2012-06-08 00:04:40 +00:00
Anna Zaks a7dcc996a9 [analyzer] Fixit for r158136.
I falsely assumed that the memory spaces are equal when we reach this
point, they might not be when memory space of one or more is stack or
Unknown. We don't want a region from Heap space alias something with
another memory space.

llvm-svn: 158165
2012-06-07 20:18:08 +00:00
Anna Zaks 3563fde6a0 [analyzer] Anti-aliasing: different heap allocations do not alias
Add a concept of symbolic memory region belonging to heap memory space.
When comparing symbolic regions allocated on the heap, assume that they
do not alias. 

Use symbolic heap region to suppress a common false positive pattern in
the malloc checker, in code that relies on malloc not returning the
memory aliased to other malloc allocations, stack.

llvm-svn: 158136
2012-06-07 03:57:32 +00:00
Anna Zaks 46d01605ee [analyzer]Malloc: refactor and report use after free by memory
allocating functions.

llvm-svn: 157037
2012-05-18 01:16:10 +00:00
Anna Zaks 63509fbacc [analyzer]Fixup r156134: Handle the case when FunctionDecl isn't avail.
llvm-svn: 156183
2012-05-04 17:37:16 +00:00
Anna Zaks 6ccfcf346d [analyzer] Assume pointer escapes when a callback is passed inside
a struct.

llvm-svn: 156135
2012-05-03 23:50:33 +00:00
Anna Zaks 228f9c7b68 [analyzer] Allow pointers escape through calls containing callback args.
(Since we don't have a generic pointer escape callback, modify
ExprEngineCallAndReturn as well as the malloc checker.)

llvm-svn: 156134
2012-05-03 23:50:28 +00:00
Anna Zaks 1655aee1e3 [analyzer] Conjure a symbol to ensure we can identify pointer arithmetic
We need to identify the value of ptr as
ElementRegion (result of pointer arithmetic) in the following code.
However, before this commit '(2-x)' evaluated to Unknown value, and as
the result, 'p + (2-x)' evaluated to Unknown value as well.

int *p = malloc(sizeof(int));
ptr = p + (2-x);

llvm-svn: 156052
2012-05-03 02:13:56 +00:00
Anna Zaks 7124b4b124 [analyzer] Revert the functional part of r155944.
The change resulted in multiple issues on the buildbot, so it's not
ready for prime time. Only enable history tracking for tainted
data(which is experimental) for now.

llvm-svn: 156049
2012-05-03 02:13:46 +00:00
Anna Zaks 263b7e016e [analyzer] Fix the 'ptr = ptr' false negative in the Malloc checker.
llvm-svn: 155963
2012-05-02 00:05:20 +00:00
Ted Kremenek f56d4f2991 Teach SValBuilder to handle casts of symbolic pointer values to an integer twice. Fixes <rdar://problem/11212866>.
llvm-svn: 155950
2012-05-01 21:58:29 +00:00
Anna Zaks 84d70a9ecd [analyzer] Test case reported for a reported false positive, now fixed.
llvm-svn: 155945
2012-05-01 21:10:29 +00:00
Anna Zaks b35437a85e [analyzer] Construct a SymExpr even when the constraint solver cannot
reason about the expression.

This essentially keeps more history about how symbolic values were
constructed. As an optimization, previous to this commit, we only kept
the history if one of the symbols was tainted, but it's valuable keep
the history around for other purposes as well: it allows us to avoid
constructing conjured symbols.

Specifically, we need to identify the value of ptr as
ElementRegion (result of pointer arithmetic) in the following code.
However, before this commit '(2-x)' evaluated to Unknown value, and as
the result, 'p + (2-x)' evaluated to Unknown value as well.

int *p = malloc(sizeof(int));
ptr = p + (2-x);

This change brings 2% slowdown on sqlite. Fixes radar://11329382.

llvm-svn: 155944
2012-05-01 21:10:26 +00:00