Remove redundaunt virtual specifiers (NFC)

Identified with tidy-modernize-use-override.

[clang] Use value instead of getValue (NFC)

[clang] Don't use Optional::hasValue (NFC)

This patch replaces Optional::hasValue with the implicit cast to bool
in conditionals only.

Revert "Don't use Optional::hasValue (NFC)"

This reverts commit aa8feeefd3ac6c78ee8f67bf033976fc7d68bc6d.

Don't use Optional::hasValue (NFC)

[clang] Don't use Optional::hasValue (NFC)

[analyzer][NFC] Prefer using isa<> instead getAs<> in conditions

Depends on D125709

Reviewed By: martong

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

[analyzer][NFC] Remove dead code and modernize surroundings

Thanks @kazu for helping me clean these parts in D127799.

I'm leaving the dump methods, along with the unused visitor handlers and
the fo

[analyzer][NFC] Remove dead code and modernize surroundings

Thanks @kazu for helping me clean these parts in D127799.

I'm leaving the dump methods, along with the unused visitor handlers and
the forwarding methods.

The dead parts actually helped to uncover two bugs, to which I'm going
to post separate patches.

Reviewed By: martong

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

show more ...

[analyzer] Fix Static Analyzer g_memdup false-positive

`g_memdup()` allocates and copies memory, thus we should not assume that
the returned memory region is uninitialized because it might not be th

[analyzer] Fix Static Analyzer g_memdup false-positive

`g_memdup()` allocates and copies memory, thus we should not assume that
the returned memory region is uninitialized because it might not be the
case.

PS: It would be even better to copy the bindings to mimic the actual
content of the buffer, but this works too.

Fixes #53617

Reviewed By: martong

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

show more ...

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

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostrea

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostream.

Also cleaned up all the call sites where this occurs.

Reviewed By: aaron.ballman

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

show more ...

[analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming value

[analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming values to match
pointer widths. Some targets may have different pointer widths depending
upon address space, so this needs to be comprehended.

Reviewed By: steakhal

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

show more ...

[analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This ca

[analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This can
occur when pointer sizes can be of different sizes, depending on address
space for example. See https://reviews.llvm.org/D118050 as an example.

This was uncovered initially in a downstream compiler project, and
tested through those systems tests.

steakhal performed systems testing across a large set of open source
projects.

Co-authored-by: steakhal
Resolves: https://github.com/llvm/llvm-project/issues/53664

Reviewed By: NoQ, steakhal

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

show more ...

[analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

The problem with leak bug reports is that the most interesting event in the code
is likely the one that did not happen --

[analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

The problem with leak bug reports is that the most interesting event in the code
is likely the one that did not happen -- lack of ownership change and lack of
deallocation, which is often present within the same function that the analyzer
inlined anyway, but not on the path of execution on which the bug occured. We
struggle to understand that a function was responsible for freeing the memory,
but failed.

D105819 added a new visitor to improve memory leak bug reports. In addition to
inspecting the ExplodedNodes of the bug pat, the visitor tries to guess whether
the function was supposed to free memory, but failed to. Initially (in D108753),
this was done by checking whether a CXXDeleteExpr is present in the function. If
so, we assume that the function was at least party responsible, and prevent the
analyzer from pruning bug report notes in it. This patch improves this heuristic
by recognizing all deallocator functions that MallocChecker itself recognizes,
by reusing MallocChecker::isFreeingCall.

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

show more ...

[analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

LocationContext::getDecl() isn't useful for obtaining the "farmed" body because
the (synthetic) body statement isn't act

[analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

LocationContext::getDecl() isn't useful for obtaining the "farmed" body because
the (synthetic) body statement isn't actually attached to the (natural-grown)
declaration in the AST.

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

show more ...

[clang] Fix bugprone argument comments (NFC)

Identified with bugprone-argument-comment.

[analyzer][NFC] Separate CallDescription from CallEvent

`CallDescriptions` deserve its own translation unit.
This patch simply moves the corresponding parts.
Also includes the `CallDescription.h` wh

[analyzer][NFC] Separate CallDescription from CallEvent

`CallDescriptions` deserve its own translation unit.
This patch simply moves the corresponding parts.
Also includes the `CallDescription.h` where it's necessary.

Reviewed By: martong, xazax.hun, Szelethus

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

show more ...

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) ||

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) || isa<T2>(x) ...
Will be replaced by:
isa_and_non_null<T1, T2, ...>(x)

Sometimes it caused further simplifications, when it would cause even
more code smell.

Aside from this, keep in mind that within `assert()` or any macro
functions, we need to wrap the isa<> expression within a parenthesis,
due to the parsing of the comma.

Reviewed By: martong

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

show more ...

[Sema, StaticAnalyzer] Use StringRef::contains (NFC)

[analyzer] Fix non-obvious analyzer warning: Use of zero-allocated memory.

Clarify the message provided when the analyzer catches the use of memory
that is allocated with size zero.

Differential Re

[analyzer] Fix non-obvious analyzer warning: Use of zero-allocated memory.

Clarify the message provided when the analyzer catches the use of memory
that is allocated with size zero.

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

show more ...

[analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

D105819 Added NoOwnershipChangeVisitor, but it is on

[analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

D105819 Added NoOwnershipChangeVisitor, but it is only registered when an
off-by-default, hidden checker option was enabled. The reason behind this was
that it grossly overestimated the set of functions that really needed a note:

std::string getTrainName(const Train *T) {
return T->name;
} // note: Retuning without changing the ownership of or deallocating memory
// Umm... I mean duh? Nor would I expect this function to do anything like that...

void foo() {
Train *T = new Train("Land Plane");
print(getTrainName(T)); // note: calling getTrainName / returning from getTrainName
} // warn: Memory leak

This patch adds a heuristic that guesses that any function that has an explicit
operator delete call could have be responsible for deallocating the memory that
ended up leaking. This is waaaay too conservative (see the TODOs in the new
function), but it safer to err on the side of too little than too much, and
would allow us to enable the option by default *now*, and add refinements
one-by-one.

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

show more ...

[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Fix a compilation error due to a missing 'template' keyword.

Different

[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Fix a compilation error due to a missing 'template' keyword.

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

show more ...

Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it"

This reverts commit a375bfb5b729e0f3ca8d5e001f423fa89e74de87.

Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it"

This reverts commit a375bfb5b729e0f3ca8d5e001f423fa89e74de87.

This was causing a bot to crash:

https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/23380/

show more ...

[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

D105553 added NoStateChangeFuncVisitor, an abstract class to aid in cre

[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

D105553 added NoStateChangeFuncVisitor, an abstract class to aid in creating
notes such as "Returning without writing to 'x'", or "Returning without changing
the ownership status of allocated memory". Its clients need to define, among
other things, what a change of state is.

For code like this:

f() {
g();
}

foo() {
f();
h();
}

We'd have a path in the ExplodedGraph that looks like this:

-- <g> -->
/ \
--- <f> --------> --- <h> --->
/ \ / \
-------- <foo> ------ <foo> -->

When we're interested in whether f neglected to change some property,
NoStateChangeFuncVisitor asks these questions:

÷×~
-- <g> -->
ß / \$ @&#*
--- <f> --------> --- <h> --->
/ \ / \
-------- <foo> ------ <foo> -->

Has anything changed in between # and *?
Has anything changed in between & and *?
Has anything changed in between @ and *?
...
Has anything changed in between $ and *?
Has anything changed in between × and ~?
Has anything changed in between ÷ and ~?
...
Has anything changed in between ß and *?
...
This is a rather thorough line of questioning, which is why in D105819, I was
only interested in whether state *right before* and *right after* a function
call changed, and early returned to the CallEnter location:

if (!CurrN->getLocationAs<CallEnter>())
return;
Except that I made a typo, and forgot to negate the condition. So, in this
patch, I'm fixing that, and under the same hood allow all clients to decide to
do this whole-function check instead of the thorough one.

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

show more ...

Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it"

This reverts commit 7d0e62bfb773c68d2bc8831fddcc8536f4613190.