Commit Graph

37 Commits

Author SHA1 Message Date
Artem Dergachev bbc6d68297 [analyzer] Fix the "Zombie Symbols" bug.
It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:

   FILE *fp = fopen("myfile.txt", "w");
   fp = 0; // leak of file descriptor

In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.

This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.

What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.

No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.

Differential Revision: https://reviews.llvm.org/D18860

llvm-svn: 347953
2018-11-30 03:27:50 +00:00
Dominic Chen 184c6242fa Reland 4: [analyzer] NFC: Update test infrastructure to support multiple constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.

Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin

Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits

Differential Revision: https://reviews.llvm.org/D30373

llvm-svn: 296895
2017-03-03 18:02:02 +00:00
Dominic Chen 09d66f7528 Revert "Reland 3: [analyzer] NFC: Update test infrastructure to support multiple constraint managers"
This reverts commit ea36f1406e1f36bf456c3f3929839b024128e468.

llvm-svn: 296841
2017-03-02 23:30:53 +00:00
Dominic Chen feaf9ff5ee Reland 3: [analyzer] NFC: Update test infrastructure to support multiple constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.

Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin

Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits

Differential Revision: https://reviews.llvm.org/D30373

llvm-svn: 296837
2017-03-02 23:05:45 +00:00
Dominic Chen 4a90bf8c3f Revert "Reland 2: [analyzer] NFC: Update test infrastructure to support multiple constraint managers"
This reverts commit f93343c099fff646a2314cc7f4925833708298b1.

llvm-svn: 296836
2017-03-02 22:58:06 +00:00
Dominic Chen 1cb0256a3c Reland 2: [analyzer] NFC: Update test infrastructure to support multiple constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.

Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin

Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits

Differential Revision: https://reviews.llvm.org/D30373

llvm-svn: 296835
2017-03-02 22:45:24 +00:00
Dominic Chen 00355a51d0 Revert "Reland: [analyzer] NFC: Update test infrastructure to support multiple constraint managers"
This reverts commit 1b28d0b10e1c8feccb971abb6ef7a18bee589830.

llvm-svn: 296422
2017-02-28 01:50:23 +00:00
Dominic Chen 59cd893320 Reland: [analyzer] NFC: Update test infrastructure to support multiple constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.

Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin

Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits

Differential Revision: https://reviews.llvm.org/D30373

llvm-svn: 296414
2017-02-28 00:02:36 +00:00
Dominic Chen 8589e10c30 Revert "[analyzer] NFC: Update test infrastructure to support multiple constraint managers"
This reverts commit 8e7780b9e59ddaad1800baf533058d2c064d4787.

llvm-svn: 296317
2017-02-27 03:29:25 +00:00
Dominic Chen 02064a3076 [analyzer] NFC: Update test infrastructure to support multiple constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.

Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin

Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits

Differential Revision: https://reviews.llvm.org/D30373

llvm-svn: 296312
2017-02-27 02:36:15 +00:00
Anna Zaks 14b1af5dcd [analyzer] Fix false positives in Keychain API checker
The checker has several false positives that this patch addresses:
- Do not check if the return status has been compared to error (or no error) at the time when leaks are reported since the status symbol might no longer be alive. Instead, pattern match on the assume and stop tracking allocated symbols on error paths.
- The checker used to report error when an unknown symbol was freed. This could lead to false positives, let's not repot those. This leads to loss of coverage in double frees.
- Do not enforce that we should only call free if we are sure that error was not returned and the pointer is not null. That warning is too noisy and we received several false positive reports about it. (I removed: "Only call free if a valid (non-NULL) buffer was returned")
- Use !isDead instead of isLive in leak reporting. Otherwise, we report leaks for objects we loose track of. This change triggered change #1.

This also adds checker specific dump to the state.

Differential Revision: https://reviews.llvm.org/D28330

llvm-svn: 291866
2017-01-13 00:50:41 +00:00
Anna Zaks 030e65d1b2 [analyzer] Fix a false positive in Secure Keychain API checker.
Better handle the blacklisting of known bad deallocators when symbol
escapes through a call to CFStringCreateWithBytesNoCopy.

Addresses radar://12702952.

llvm-svn: 171770
2013-01-07 19:13:00 +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 4d9fbd7ec4 [analyzer] -analyzer-ipa=inlining is now the default. Remove it from tests.
The actual change here is a little more complicated than the summary above.
What we want to do is have our generic inlining tests run under whatever
mode is the default. However, there are some tests that depend on the
presence of C++ inlining, which still has some rough edges. These tests have
been explicitly marked as -analyzer-ipa=inlining in preparation for a new
mode that limits inlining to C functions and blocks. This will be the
default until the false positives for C++ have been brought down to
manageable levels.

llvm-svn: 162317
2012-08-21 21:44:07 +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
Anna Zaks 0af3e06ff6 [analyzer] Rework inlining related command line options.
- Remove -analyzer-inline-call.
 - Add -analyzer-ipa=[none|inlining]
 - Add -analyzer-inlining-mode to allow experimentation for
different performance tuning methods.

llvm-svn: 152351
2012-03-08 23:16:35 +00:00
Anna Zaks 6ca4fd5b88 [analyzer] Leaks should be uniqued by the allocation point in the
closest function context (Keychain API).

llvm-svn: 151613
2012-02-28 03:07:06 +00:00
Anna Zaks 06a77fc1b9 [analyzer] Fix Malloc False Positive (PR 12100)
When allocated buffer is passed to CF/NS..NoCopy functions, the
ownership is transfered unless the deallocator argument is set to
'kCFAllocatorNull'.

llvm-svn: 151608
2012-02-28 01:54:22 +00:00
Anna Zaks 18de54b1a6 [analyzer] Run remove dead bindings before each call.
This ensures that we report the bugs associated with symbols going
out of scope in the correct function context.

llvm-svn: 151369
2012-02-24 16:49:46 +00:00
Anna Zaks 4b062cb904 [analyzer] KeyChainAPI: unique the leaks by allocation site.
(Very similar to the previous change in malloc.)

llvm-svn: 151297
2012-02-23 22:53:29 +00:00
Anna Zaks 19a66678d5 [analyzer] Make KeyChainAPI checker inlining-aware.
llvm-svn: 151007
2012-02-21 00:00:44 +00:00
Anna Zaks 719051e1c6 [analyzer] Make KeychainAPI checker less aggressive. radar://10508828
We trigger an error if free is called after a possibly failed allocation. Do not trigger the error if we know that the buffer is not null.

llvm-svn: 145584
2011-12-01 16:41:58 +00:00
Anna Zaks 814adf7752 Fix: Bug 10798 - [analyzer] Crash when analyzing ICU. (A slight improvement on the previous commit.)
llvm-svn: 138762
2011-08-29 20:43:37 +00:00
Anna Zaks cda129eb1c Fix bug 10797: Crash: "cast<Ty>() argument of incompatible type!" assert when analyzing ICU.
Patch by Jean-Daniel Dupas. Thanks for spotting and fixing!

llvm-svn: 138757
2011-08-29 20:05:54 +00:00
Anna Zaks 0897a23517 [analyzer] MacOSKeychainAPIChecker: Add reasoning about functions which MIGHT deallocate the memory region allocated with SecKeychain APIs. Specifically, when the buffer is passed to CFStringCreateWithBytesNoCopy along with a custom deallocator, which might potentially correctly release the memory.
llvm-svn: 138417
2011-08-24 00:06:27 +00:00
Anna Zaks bd7972b49e [analyzer] MacOSKeychainAPIChecker: Users of KeyChain API often use free() to deallocate the password. Catch this error explicitly and generate the error message at the place where free() is called.
llvm-svn: 138296
2011-08-22 23:18:12 +00:00
Anna Zaks a06421a02c MacOSKeychainAPIChecker: Turn it on by default.
llvm-svn: 137740
2011-08-16 20:02:05 +00:00
Anna Zaks 01ae1e1197 MacOSKeychainAPIChecker: Do not report double allocation if first allocation returned an error.
llvm-svn: 137720
2011-08-16 16:30:24 +00:00
Anna Zaks 80319c1702 MacOSKeychainAPIChecker: Test all APIs.
llvm-svn: 137549
2011-08-13 02:10:15 +00:00
Anna Zaks 59d741fec6 MacOSKeychainAPIChecker: If the allocated data address entered as an enclosing function parameter, skip it to avoid false positives.
llvm-svn: 137526
2011-08-12 22:47:22 +00:00
Anna Zaks fdd0aca0a5 MacOSKeychainAPIChecker:
Report errors earlier: on checkDeadSymbols() and clear the state after the symbol we are tracking goes out of scope. 

Also, perform lazy error checking. Instead of forcing the paths to be split depending one the return value of the allocator, make the return symbol depend on the allocated data symbol, which prolongs its life span to the time when the allocated data symbol becomes dead.

llvm-svn: 137523
2011-08-12 21:56:43 +00:00
Anna Zaks c52bed1814 KeychainAPI checker: Generate an error on double allocation. Pull out getAsPointeeMemoryRegion so that it could be reused.
llvm-svn: 136952
2011-08-05 00:37:00 +00:00
Anna Zaks 680be29584 KeychainAPI checker: forgot to commit the test with r136930. This should fix the bot.
llvm-svn: 136938
2011-08-04 22:40:38 +00:00
Anna Zaks 388c18ec7f KeychainAPI checker: Add basic diagnostics. Track MemoryRegion istead of SymbolicRef since the address might not be a symbolic value in some cases, for example in fooOnlyFree() test.
llvm-svn: 136851
2011-08-04 00:26:57 +00:00
Ted Kremenek 3f955e6d89 [analyzer] rename all experimental checker packages to have 'experimental' be the common root package.
llvm-svn: 136835
2011-08-03 23:14:55 +00:00
Anna Zaks 9ab728bb05 KeychainAPI checker: only check the paths on which the allocator function returned noErr. (+ minor cleanup)
llvm-svn: 136694
2011-08-02 17:11:03 +00:00
Anna Zaks 15f496c118 Add a skeleton for the Keychain Services API Checker. Register it as OSX experimental for now. Note, the checker still does not handle tracking of escaped values, taking into account the return value of the allocator functions, nor the actual bug reporting..
llvm-svn: 136659
2011-08-01 22:40:01 +00:00