Stop tracking retain count of OSObject after escape to void * / other primitive types

Escaping to void * / uint64_t / others non-OSObject * should stop tracking,
as such functions can have heterogen

Stop tracking retain count of OSObject after escape to void * / other primitive types

Escaping to void * / uint64_t / others non-OSObject * should stop tracking,
as such functions can have heterogeneous semantics depending on context,
and can not always be annotated.

rdar://46439133

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

llvm-svn: 348675

show more ...

[analyzer] Move out tracking retain count for OSObjects into a separate checker

Allow enabling and disabling tracking of ObjC/CF objects
separately from tracking of OS objects.

Differential Revisio

[analyzer] Move out tracking retain count for OSObjects into a separate checker

Allow enabling and disabling tracking of ObjC/CF objects
separately from tracking of OS objects.

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

llvm-svn: 348638

show more ...

[analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

The option has no tests, is not used anywhere, and is actually
incorrect: it prints the line number wit

[analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

The option has no tests, is not used anywhere, and is actually
incorrect: it prints the line number without the reference to a file,
which can be outright incorrect.

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

llvm-svn: 348637

show more ...

[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
oper

[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

show more ...

[analyzer] [NFC] Minor refactoring of RetainCountDiagnostics

Move visitors to the implementation file, move a complicated logic into
a function.

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

[analyzer] [NFC] Minor refactoring of RetainCountDiagnostics

Move visitors to the implementation file, move a complicated logic into
a function.

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

llvm-svn: 347946

show more ...

[analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

One of the reasons why AnalyzerOptions is so chaotic is that options can be
retrieved from the comman

[analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

One of the reasons why AnalyzerOptions is so chaotic is that options can be
retrieved from the command line whenever and wherever. This allowed for some
options to be forgotten for a looooooong time. Have you ever heard of
"region-store-small-struct-limit"? In order to prevent this in the future, I'm
proposing to restrict AnalyzerOptions' interface so that only checker options
can be retrieved without special getters. I would like to make every option be
accessible only through a getter, but checkers from plugins are a thing, so I'll
have to figure something out for that.

This also forces developers who'd like to add a new option to register it
properly in the .def file.

This is done by

* making the third checker pointer parameter non-optional, and checked by an
assert to be non-null.
* I added new, but private non-checkers option initializers, meant only for
internal use,
* Renamed these methods accordingly (mind the consistent name for once with
getBooleanOption!):
- getOptionAsString -> getCheckerStringOption,
- getOptionAsInteger -> getCheckerIntegerOption
* The 3 functions meant for initializing data members (with the not very
descriptive getBooleanOption, getOptionAsString and getOptionAsUInt names)
were renamed to be overloads of the getAndInitOption function name.
* All options were in some way retrieved via getCheckerOption. I removed it, and
moved the logic to getStringOption and getCheckerStringOption. This did cause
some code duplication, but that's the only way I could do it, now that checker
and non-checker options are separated. Note that the non-checker version
inserts the new option to the ConfigTable with the default value, but the
checker version only attempts to find already existing entries. This is how
it always worked, but this is clunky and I might end reworking that too, so we
can eventually get a ConfigTable that contains the entire configuration of the
analyzer.

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

llvm-svn: 346113

show more ...

[analyzer][NFC] Fix some incorrect uses of -analyzer-config options

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-con

[analyzer][NFC] Fix some incorrect uses of -analyzer-config options

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.

In this patch, I found some flags that should've been used as checker options,
or have absolutely no mention of in AnalyzerOptions, or are nonexistent.

- NonLocalizedStringChecker now uses its "AggressiveReport" flag as a checker
option
- lib/StaticAnalyzer/Frontend/ModelInjector.cpp now accesses the "model-path"
option through a getter in AnalyzerOptions
- -analyzer-config path-diagnostics-alternate=false is not a thing, I removed it,
- lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp and
lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h are weird, they actually
only contain an option getter. I deleted them, and fixed RetainCountChecker
to get it's "leak-diagnostics-reference-allocation" option as a checker option,
- "region-store-small-struct-limit" has a proper getter now.

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

llvm-svn: 345985

show more ...

Fix clang -Wimplicit-fallthrough warnings across llvm, NFC

This patch should not introduce any behavior changes. It consists of
mostly one of two changes:
1. Replacing fall through comments with the

Fix clang -Wimplicit-fallthrough warnings across llvm, NFC

This patch should not introduce any behavior changes. It consists of
mostly one of two changes:
1. Replacing fall through comments with the LLVM_FALLTHROUGH macro
2. Inserting 'break' before falling through into a case block consisting
of only 'break'.

We were already using this warning with GCC, but its warning behaves
slightly differently. In this patch, the following differences are
relevant:
1. GCC recognizes comments that say "fall through" as annotations, clang
doesn't
2. GCC doesn't warn on "case N: foo(); default: break;", clang does
3. GCC doesn't warn when the case contains a switch, but falls through
the outer case.

I will enable the warning separately in a follow-up patch so that it can
be cleanly reverted if necessary.

Reviewers: alexfh, rsmith, lattner, rtrieu, EricWF, bollu

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

llvm-svn: 345882

show more ...

[analyzer] RetainCountChecker: for now, do not trust the summaries of inlined code

Trusting summaries of inlined code would require a more thorough work,
as the current approach was causing too many

[analyzer] RetainCountChecker: for now, do not trust the summaries of inlined code

Trusting summaries of inlined code would require a more thorough work,
as the current approach was causing too many false positives, as the new
example in test. The culprit lies in the fact that we currently escape
all variables written into a field (but not passed off to unknown
functions!), which can result in inconsistent behavior.

rdar://45655344

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

llvm-svn: 345746

show more ...

[analyzer] Correct modelling of OSDynamicCast: eagerly state split

Previously, OSDynamicCast was modeled as an identity.

This is not correct: the output of OSDynamicCast may be zero even if the
inp

[analyzer] Correct modelling of OSDynamicCast: eagerly state split

Previously, OSDynamicCast was modeled as an identity.

This is not correct: the output of OSDynamicCast may be zero even if the
input was not zero (if the class is not of desired type), and thus the
modeling led to false positives.

Instead, we are doing eager state split:
in one branch, the returned value is identical to the input parameter,
and in the other branch, the returned value is zero.

This patch required a substantial refactoring of canEval infrastructure,
as now it can return different function summaries, and not just true/false.

rdar://45497400

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

llvm-svn: 345338

show more ...

[analyzer] [NFC] Change scanReachableSymbols to use ranges

Remove unused overload. Clean up some usages.

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

llvm-svn: 345101

[analyzer] Do not stop tracking CXX methods touching OSObject.

Trust generalized annotations for OSObject.

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

llvm-svn: 345100

[analyzer] Trust summaries for OSObject::retain and OSObject::release

Refactor the way in which summaries are consumed for safeMetaCast

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

llvm-

[analyzer] Trust summaries for OSObject::retain and OSObject::release

Refactor the way in which summaries are consumed for safeMetaCast

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

llvm-svn: 345099

show more ...

[analyzer] NFC: RetainCountChecker: Don't dump() symbols into program point tags.

We don't need a separate node for every symbol, because whenever the first
symbol leaks, a bug is emitted, the analy

[analyzer] NFC: RetainCountChecker: Don't dump() symbols into program point tags.

We don't need a separate node for every symbol, because whenever the first
symbol leaks, a bug is emitted, the analysis is sinked, and the checker
callback immediately returns due to State variable turning into null,
so we never get to see the second leaking symbol.

Additionally, we are no longer able to break normal analysis while experimenting
with debug dumps.

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

llvm-svn: 344538

show more ...

[analyzer] Avoid unneeded invalidation in RetainCountChecker

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

llvm-svn: 344312

[analyzer] Retain count checker for OSObject: recognize OSDynamicCast

For now, tresting the cast as a no-op, and disregarding the case where
the output becomes null due to the type mismatch.

rdar:/

[analyzer] Retain count checker for OSObject: recognize OSDynamicCast

For now, tresting the cast as a no-op, and disregarding the case where
the output becomes null due to the type mismatch.

rdar://45174557

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

llvm-svn: 344311

show more ...

[analyzer] Process state in checkEndFunction in RetainCountChecker

Modify the RetainCountChecker to perform state "adjustments" in
checkEndFunction, as performing work in PreStmt<ReturnStmt> does no

[analyzer] Process state in checkEndFunction in RetainCountChecker

Modify the RetainCountChecker to perform state "adjustments" in
checkEndFunction, as performing work in PreStmt<ReturnStmt> does not
work with destructors.
The previous version made an implicit assumption that no code runs
after the return statement is executed.

rdar://43945028

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

llvm-svn: 342770

show more ...

[analyzer] [NFC] Prefer make_unique over "new"

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

llvm-svn: 342767

[analyzer] Better retain count rules for OSObjects

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

llvm-svn: 340961

[analyzer] Preliminary version of retain count checking for OSObjects

Has quite a lot of false positives, disabled behind the flag.

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

llvm-svn:

[analyzer] Preliminary version of retain count checking for OSObjects

Has quite a lot of false positives, disabled behind the flag.

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

llvm-svn: 340502

show more ...

[analyzer] [NFC] Fix minor formatting issues in RetainCountChecker

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

llvm-svn: 340378

[analyzer] [NFC] Extract a method for creating RefVal from RetEffect in RetainCountChecker

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

llvm-svn: 340377

[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker - try #2

Turns out it can't be removed from the analyzer since it relies on CallEvent.

Moving to staticAnalyzer/core

Differen

[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker - try #2

Turns out it can't be removed from the analyzer since it relies on CallEvent.

Moving to staticAnalyzer/core

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

llvm-svn: 340247

show more ...

Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"

This reverts commit a786521fa66c72edd308baff0c08961b6d964fb1.

Bots haven't caught up yet, but broke modules build wit

Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"

This reverts commit a786521fa66c72edd308baff0c08961b6d964fb1.

Bots haven't caught up yet, but broke modules build with:

../tools/clang/include/clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h:18:10:
fatal error: cyclic dependency in module 'Clang_StaticAnalyzer_Core':
Clang_StaticAnalyzer_Core -> Clang_Analysis ->
Clang_StaticAnalyzer_Checkers -> Clang_StaticAnalyzer_Core
^

llvm-svn: 340117

show more ...

[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker

ARCMigrator is using code from RetainCountChecker, which is a layering
violation (and it also does it badly, by using a differe

[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker

ARCMigrator is using code from RetainCountChecker, which is a layering
violation (and it also does it badly, by using a different header, and
then relying on implementation being present in a header file).

This change splits up RetainSummaryManager into a separate library in
lib/Analysis, which can be used independently of a checker.

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

llvm-svn: 340114

show more ...