Commit Graph

21 Commits

Author SHA1 Message Date
Jordan Rose 514f935411 [analyzer] Pointers escape into +[NSValue valueWithPointer:]...
...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
2014-01-07 21:39:48 +00:00
Alp Toker f6a24ce40f Fix a tranche of comment, test and doc typos
llvm-svn: 196510
2013-12-05 16:25:25 +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 4c56c22634 [analyzer] Silence warnings coming from allocators used by std::basic_string.
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
2013-11-15 02:11:19 +00:00
Anna Zaks 830d2f7701 [analyzer] Suppress warnings coming out of std::basic_string.
The analyzer cannot reason about the internal invariances of the data structure (radar://15194597).

llvm-svn: 194004
2013-11-04 19:13:03 +00:00
Jordan Rose 05b2f98d89 [analyzer] Treat std::initializer_list as opaque rather than aborting.
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
2013-07-17 17:16:33 +00:00
Anna Zaks e0ad10404d [analyzer] Fixup for r185609: actually do suppress warnings coming out of std::list.
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
2013-07-09 01:55:00 +00:00
Anna Zaks a42fb525e4 [analyzer] Suppress reports reported in std::list
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
2013-07-04 02:38:10 +00:00
Anna Zaks a4bc5e1201 [analyzer] Malloc checker should only escape the receiver when “[O init..]” is called.
Jordan has pointed out that it is valuable to warn in cases when the arguments to init escape.
For example, NSData initWithBytes id not going to free the memory.

llvm-svn: 183062
2013-05-31 23:47:32 +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
Jordan Rose 7023a90378 [analyzer] Don't inline the [cd]tors of C++ iterators.
This goes with r178516, which instructed the analyzer not to inline the
constructors and destructors of C++ container classes. This goes a step
further and does the same thing for iterators, so that the analyzer won't
falsely decide we're trying to construct an iterator pointing to a
nonexistent element.

The heuristic for determining whether something is an iterator is the
presence of an 'iterator_category' member. This is controlled under the
same -analyzer-config option as container constructor/destructor inlining:
'c++-container-inlining'.

<rdar://problem/13770187>

llvm-svn: 180890
2013-05-01 22:39:31 +00:00
Jordan Rose fa80736bca [analyzer] Re-enable using global regions as a symbolic base.
Now that we're invalidating global regions properly, we want to continue
taking advantage of a particular optimization: if all global regions are
invalidated together, we can represent the bindings of each region with
a "derived region value" symbol. Essentially, this lazily links each
global region with a single symbol created at invalidation time, rather
than binding each region with a new symbolic value.

We used to do this, but haven't been for a while; the previous commit
re-enabled this code path, and this handles the fallout.

<rdar://problem/13464044>

llvm-svn: 179554
2013-04-15 20:39:45 +00:00
Jordan Rose d02adbf03c [analyzer] Tests: move system functions into system header simulator files.
Some checkers ascribe different behavior to functions declared in system
headers, so when working with standard library functions it's probably best
to always have them in a standard location.

Test change only (no functionality change), but necessary for the next commit.

llvm-svn: 179552
2013-04-15 20:39:37 +00:00
Jordan Rose d11ef1aaf7 [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.

llvm-svn: 178513
2013-04-02 00:26:15 +00:00
Anton Yartsev 13df03624b [analyzer] Adds cplusplus.NewDelete checker that check for memory leaks, double free, and use-after-free problems of memory managed by new/delete.
llvm-svn: 177849
2013-03-25 01:35:45 +00:00
Jordan Rose 613f3c0022 [analyzer] Be more consistent about Objective-C methods that free memory.
Previously, MallocChecker's pointer escape check and its post-call state
update for Objective-C method calls had a fair amount duplicated logic
and not-entirely-consistent checks. This commit restructures all this to
be more consistent and possibly allow us to be more aggressive in warning
about double-frees.

New policy (applies to system header methods only):
(1) If this is a method we know about, model it as taking/holding ownership
    of the passed-in buffer.
  (1a) ...unless there's a "freeWhenDone:" parameter with a zero (NO) value.
(2) If there's a "freeWhenDone:" parameter (but it's not a method we know
    about), treat the buffer as escaping if the value is non-zero (YES) and
    non-escaping if it's zero (NO).
(3) If the first selector piece ends with "NoCopy" (but it's not a method we
    know about and there's no "freeWhenDone:" parameter), treat the buffer
    as escaping.

The reason that (2) and (3) don't explicitly model the ownership transfer is
because we can't be sure that they will actually free the memory using free(),
and we wouldn't want to emit a spurious "mismatched allocator" warning
(coming in Anton's upcoming patch). In the future, we may have an idea of a
"generic deallocation", i.e. we assume that the deallocator is correct but
still continue tracking the region so that we can warn about double-frees.

Patch by Anton Yartsev, with modifications from me.

llvm-svn: 176744
2013-03-09 00:59:10 +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
Jordan Rose b54cfa310a [analyzer] Explain why we have system-header-simulator*.h files.
Suggested by Csaba. Text based on an e-mail of mine on cfe-dev.

llvm-svn: 174213
2013-02-01 19:50:01 +00:00
Anna Zaks 2ed5125502 [analyzer] Add symbol escapes logic to the SimpleStreamChecker.
llvm-svn: 167439
2012-11-06 04:20:57 +00:00
NAKAMURA Takumi 77369457e0 clang/test/Analysis: Fix the declaration of strlen() for 32 bit targets.
- Inputs/system-header-simulator.h: Declare strlen() with size_t.

  - malloc-interprocedural.c: Move the definition of size_t into the header above.

Then XFAIL can be pruned.

llvm-svn: 164300
2012-09-20 11:03:56 +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