[analyzer] Report violations of the "returns_nonnull" attribute (#106048)

Make sure code respects the GNU-extension `__attribute__((returns_nonnull))`.

Extend the NullabilityChecker to check that

[analyzer] Report violations of the "returns_nonnull" attribute (#106048)

Make sure code respects the GNU-extension `__attribute__((returns_nonnull))`.

Extend the NullabilityChecker to check that a function returns_nonnull
does not return a nullptr.

This commit also reverts an old hack introduced by
49bd58f1ebe28d97e4949e9c757bc5dfd8b2d72f
because it is no longer needed

CPP-4741

show more ...

[NFC][Clang] Improve const correctness for IdentifierInfo (#79365)

The IdentifierInfo isn't typically modified. Use 'const' wherever
possible.

[clang] Use StringRef::{starts,ends}_with (NFC) (#75149)

This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}:

[clang] Use StringRef::{starts,ends}_with (NFC) (#75149)

This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.

I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.

show more ...

[analyzer] Fix false negative when accessing a nonnull property from … (#67563)

```
@interface A : NSObject
@property (nonnull, nonatomic, strong) NSString *name;
+ (nullable instancetype)shared;

[analyzer] Fix false negative when accessing a nonnull property from … (#67563)

```
@interface A : NSObject
@property (nonnull, nonatomic, strong) NSString *name;
+ (nullable instancetype)shared;
@end

@[[A shared].name];
```
Consider the code above, the nullability of the name property should
depend on the result of the shared method. A warning is expected because
of adding a nullable object to array.
ObjCMessageExpr gets the actual type through
Sema::getMessageSendResultType, instead of using the return type of
MethodDecl directly. The final type is generated by considering the
nullability of receiver and MethodDecl together.
Thus, the RetType in NullabilityChecker should all be replaced with
M.getOriginExpr()->getType().

Co-authored-by: tripleCC <triplecc@gmail.com>

show more ...

[analyzer] Fix false negative when pass implicit cast nil to nonnull

We should look through implicit casts before determining the type of the arguments, and only allow explicit cast to _Nonnull to s

[analyzer] Fix false negative when pass implicit cast nil to nonnull

We should look through implicit casts before determining the type of the arguments, and only allow explicit cast to _Nonnull to suppress warning

```
void foo(NSString *_Nonnull);

foo((NSString * _Nonnull)nil); // no-warning
id obj = nil;
foo(obj); // should warning here (implicit cast id to NSString *_Nonnull)

```

Reviewed By: xazax.hun, steakhal

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

show more ...

[analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradu

[analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradually
move forward.

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

show more ...

[analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

If a parameter has a nullability annotation, the nullability information
of the parameter should

[analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

If a parameter has a nullability annotation, the nullability information
of the parameter should be set as a NullabilityState trait at the
beginning of the function.

Patch By tripleCC!

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

show more ...

[StaticAnalyzer] Fix block pointer type nullability check

This patch fixes a false negative when the property type
is an objective-c block pointer.

Patch By tripleCC!

Differential Revision: https:

[StaticAnalyzer] Fix block pointer type nullability check

This patch fixes a false negative when the property type
is an objective-c block pointer.

Patch By tripleCC!

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

show more ...

[analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

The NullabilityChecker has a very early policy decision that non-inlined
property accesses will be inferred as return

[analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

The NullabilityChecker has a very early policy decision that non-inlined
property accesses will be inferred as returning nonnull, despite nullability
annotations to the contrary. This decision eliminates false positives related
to very common code patterns that look like this:

if (foo.prop) {
[bar doStuffWithNonnull:foo.prop];
}

While this probably represents a correct nil-check, the analyzer can't
determine correctness without gaining visibility into the property
implementation.

Unfortunately, inferring nullable properties as nonnull comes at the cost of
significantly reduced code coverage. My goal here is to enable detection of
many property-related nullability violations without a large increase
in false positives.

The approach is to introduce a heuristic: after accessing the value of
a property, if the analyzer at any time proves that the property value is
nonnull (which would happen in particular due to a nil-check conditional),
then subsequent property accesses on that code path will be *inferred*
as nonnull. This captures the pattern described above, which I believe
to be the dominant source of false positives in real code.

https://reviews.llvm.org/D131655

show more ...

[analyzer] Prefer wrapping SymbolicRegions by ElementRegions

It turns out that in certain cases `SymbolRegions` are wrapped by
`ElementRegions`; in others, it's not. This discrepancy can cause the
a

[analyzer] Prefer wrapping SymbolicRegions by ElementRegions

It turns out that in certain cases `SymbolRegions` are wrapped by
`ElementRegions`; in others, it's not. This discrepancy can cause the
analyzer not to recognize if the two regions are actually referring to
the same entity, which then can lead to unreachable paths discovered.

Consider this example:

```lang=C++
struct Node { int* ptr; };
void with_structs(Node* n1) {
Node c = *n1; // copy
Node* n2 = &c;
clang_analyzer_dump(*n1); // lazy...
clang_analyzer_dump(*n2); // lazy...
clang_analyzer_dump(n1->ptr); // rval(n1->ptr): reg_$2<int * SymRegion{reg_$0<struct Node * n1>}.ptr>
clang_analyzer_dump(n2->ptr); // rval(n2->ptr): reg_$1<int * Element{SymRegion{reg_$0<struct Node * n1>},0 S64b,struct Node}.ptr>
clang_analyzer_eval(n1->ptr != n2->ptr); // UNKNOWN, bad!
(void)(*n1);
(void)(*n2);
}
```

The copy of `n1` will insert a new binding to the store; but for doing
that it actually must create a `TypedValueRegion` which it could pass to
the `LazyCompoundVal`. Since the memregion in question is a
`SymbolicRegion` - which is untyped, it needs to first wrap it into an
`ElementRegion` basically implementing this untyped -> typed conversion
for the sake of passing it to the `LazyCompoundVal`.
So, this is why we have `Element{SymRegion{.}, 0,struct Node}` for `n1`.

The problem appears if the analyzer evaluates a read from the expression
`n1->ptr`. The same logic won't apply for `SymbolRegionValues`, since
they accept raw `SubRegions`, hence the `SymbolicRegion` won't be
wrapped into an `ElementRegion` in that case.

Later when we arrive at the equality comparison, we cannot prove that
they are equal.

For more details check the corresponding thread on discourse:
https://discourse.llvm.org/t/are-symbolicregions-really-untyped/64406

---

In this patch, I'm eagerly wrapping each `SymbolicRegion` by an
`ElementRegion`; basically canonicalizing to this form.
It seems reasonable to do so since any object can be thought of as a single
array of that object; so this should not make much of a difference.

The tests also underpin this assumption, as only a few were broken by
this change; and actually fixed a FIXME along the way.

About the second example, which does the same copy operation - but on
the heap - it will be fixed by the next patch.

Reviewed By: martong

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

show more ...

[clang] Qualify auto in range-based for loops (NFC)

[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up s

[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up some of the
instances where bool was used, and could be changed to DefaultBool.

Reviewed By: steakhal

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

show more ...

[analyzer] Add forwarding `addVisitor` method

The majority of all `addVisitor` callers follow the same pattern:
addVisitor(std::make_unique<SomeVisitor>(arg1, arg2, ...));

This patches introduces

[analyzer] Add forwarding `addVisitor` method

The majority of all `addVisitor` callers follow the same pattern:
addVisitor(std::make_unique<SomeVisitor>(arg1, arg2, ...));

This patches introduces additional overload for `addVisitor` to simplify
that pattern:
addVisitor<SomeVisitor>(arg1, arg2, ...);

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

show more ...

[analyzer][Nullability] Don't emit under the checker name NullabilityBase

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

[analyzer] Do not report NSError null dereference for _Nonnull params.

We want to trust user type annotations and stop assuming pointers declared
as _Nonnull still can be null. This functionality is

[analyzer] Do not report NSError null dereference for _Nonnull params.

We want to trust user type annotations and stop assuming pointers declared
as _Nonnull still can be null. This functionality is implemented as part
of NullabilityChecker as it already tracks non-null types.

Patch by Valeriy Savchenko!

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

show more ...

[analyzer] Use IgnoreImpCasts() instead of reimplementing it.

No intended behavior change.

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

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patc

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patch changes the parameter of the shouldRegister*
function to CheckerManager to be able to query the analyzer options when
deciding whether the checker should be registered.

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

show more ...

[analyzer] Nullability: Don't infer nullable when passing as nullable parameter.

You can't really infer anything from that.

[analyzer][NFC] Fix inconsistent references to checkers as "checks"

Traditionally, clang-tidy uses the term check, and the analyzer uses checker,
but in the very early years, this wasn't the case, a

[analyzer][NFC] Fix inconsistent references to checkers as "checks"

Traditionally, clang-tidy uses the term check, and the analyzer uses checker,
but in the very early years, this wasn't the case, and code originating from the
early 2010's still incorrectly refer to checkers as checks.

This patch attempts to hunt down most of these, aiming to refer to checkers as
checkers, but preserve references to callback functions (like checkPreCall) as
checks.

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

llvm-svn: 371760

show more ...

[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

These static functions deal with ExplodedNodes which is something we don't want
the PathDiagnostic interface to know anyth

[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

These static functions deal with ExplodedNodes which is something we don't want
the PathDiagnostic interface to know anything about, as it's planned to be
moved out of libStaticAnalyzerCore.

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

llvm-svn: 371659

show more ...

[analyzer] NFC: Re-implement stack hints as a side map in BugReport.

That's one of the few random entities in the PathDiagnostic interface that
are specific to the Static Analyzer. By moving them ou

[analyzer] NFC: Re-implement stack hints as a side map in BugReport.

That's one of the few random entities in the PathDiagnostic interface that
are specific to the Static Analyzer. By moving them out we could let
everybody use path diagnostics without linking against Static Analyzer.

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

llvm-svn: 371658

show more ...

[analyzer] NFC: Introduce sub-classes for path-sensitive and basic reports.

Checkers are now required to specify whether they're creating a
path-sensitive report or a path-insensitive report by cons

[analyzer] NFC: Introduce sub-classes for path-sensitive and basic reports.

Checkers are now required to specify whether they're creating a
path-sensitive report or a path-insensitive report by constructing an
object of the respective type.

This makes BugReporter more independent from the rest of the Static Analyzer
because all Analyzer-specific code is now in sub-classes.

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

llvm-svn: 371450

show more ...

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
o

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.

Differential revision: https://reviews.llvm.org/D66259

llvm-svn: 368942

show more ...

[analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef

find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagn

[analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef

find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagnosticPieceRef/g' {} \;
git diff -U3 --no-color HEAD^ | clang-format-diff-6.0 -p1 -i

Just as C++ is meant to be refactored, right?

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

llvm-svn: 368717

show more ...

Fix parameter name comments using clang-tidy. NFC.

This patch applies clang-tidy's bugprone-argument-comment tool
to LLVM, clang and lld source trees. Here is how I created this
patch:

$ git clone

Fix parameter name comments using clang-tidy. NFC.

This patch applies clang-tidy's bugprone-argument-comment tool
to LLVM, clang and lld source trees. Here is how I created this
patch:

$ git clone https://github.com/llvm/llvm-project.git
$ cd llvm-project
$ mkdir build
$ cd build
$ cmake -GNinja -DCMAKE_BUILD_TYPE=Debug \
-DLLVM_ENABLE_PROJECTS='clang;lld;clang-tools-extra' \
-DCMAKE_EXPORT_COMPILE_COMMANDS=On -DLLVM_ENABLE_LLD=On \
-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ ../llvm
$ ninja
$ parallel clang-tidy -checks='-*,bugprone-argument-comment' \
-config='{CheckOptions: [{key: StrictMode, value: 1}]}' -fix \
::: ../llvm/lib/**/*.{cpp,h} ../clang/lib/**/*.{cpp,h} ../lld/**/*.{cpp,h}

llvm-svn: 366177

show more ...