Commit Graph

327 Commits

Author SHA1 Message Date
Kristof Umann cf6bb77f65 [analyzer][MallocChecker][NFC] Document and reorganize some functions
This patch merely reorganizes some things, and features no functional change.

In detail:

* Provided documentation, or moved existing documentation in more obvious
places.
* Added dividers. (the //===----------===// thing).
* Moved getAllocationFamily, printAllocDeallocName, printExpectedAllocName and
printExpectedDeallocName in the global namespace on top of the file where
AllocationFamily is declared, as they are very strongly related.
* Moved isReleased and MallocUpdateRefState near RefState's definition for the
same reason.
* Realloc modeling was very poor in terms of variable and structure naming, as
well as documentation, so I renamed some of them and added much needed docs.
* Moved function IdentifierInfos to a separate struct, and moved isMemFunction,
isCMemFunction adn isStandardNewDelete inside it. This makes the patch affect
quite a lot of lines, should I extract it to a separate one?
* Moved MallocBugVisitor out of MallocChecker.
* Preferred switches to long else-if branches in some places.
* Neatly organized some RUN: lines.

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

llvm-svn: 349281
2018-12-15 18:34:00 +00:00
Kristof Umann 76a21502fd [analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend
ClangCheckerRegistry is a very non-obvious, poorly documented, weird concept.
It derives from CheckerRegistry, and is placed in lib/StaticAnalyzer/Frontend,
whereas it's base is located in lib/StaticAnalyzer/Core. It was, from what I can
imagine, used to circumvent the problem that the registry functions of the
checkers are located in the clangStaticAnalyzerCheckers library, but that
library depends on clangStaticAnalyzerCore. However, clangStaticAnalyzerFrontend
depends on both of those libraries.

One can make the observation however, that CheckerRegistry has no place in Core,
it isn't used there at all! The only place where it is used is Frontend, which
is where it ultimately belongs.

This move implies that since
include/clang/StaticAnalyzer/Checkers/ClangCheckers.h only contained a single function:

class CheckerRegistry;

void registerBuiltinCheckers(CheckerRegistry &registry);

it had to re purposed, as CheckerRegistry is no longer available to
clangStaticAnalyzerCheckers. It was renamed to BuiltinCheckerRegistration.h,
which actually describes it a lot better -- it does not contain the registration
functions for checkers, but only those generated by the tblgen files.

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

llvm-svn: 349275
2018-12-15 16:23:51 +00:00
Raphael Isemann b23ccecbb0 Misc typos fixes in ./lib folder
Summary: Found via `codespell -q 3 -I ../clang-whitelist.txt -L uint,importd,crasher,gonna,cant,ue,ons,orign,ned`

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: teemperor, jholewinski, jvesely, nhaehnle, whisperity, jfb, cfe-commits

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

llvm-svn: 348755
2018-12-10 12:37:46 +00:00
Kristof Umann 549f9cd46f [analyzer] Evaluate all non-checker config options before analysis
In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.

Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.

However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.

Also, almost all options related class have a similar interface, so uniformity
is also a benefit.

The implementation for errors on invalid input will be commited shorty.

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

llvm-svn: 348031
2018-11-30 20:44:00 +00:00
Artem Dergachev e2b5438a73 [analyzer] MallocChecker: Avoid redundant transitions.
Don't generate a checker-tagged node unconditionally on the first
checkDeadSymbols callback when no pointers are tracked.

This is a tiny performance optimization; it may change the behavior slightly
by making Static Analyzer bail out on max-nodes one node later (which is good)
but any test would either break for no good reason or become useless
every time someone sneezes.

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

llvm-svn: 347955
2018-11-30 03:52:42 +00:00
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
Kristof Umann 0a1f91c80c [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
2018-11-05 03:50:37 +00:00
Artem Dergachev 32c0c85382 [analyzer] pr39348: MallocChecker: Realize that sized delete isn't custom delete.
MallocChecker no longer thinks that operator delete() that accepts the size of
the object to delete (available since C++14 or under -fsized-deallocation)
is some weird user-defined operator. Instead, it handles it like normal delete.

Additionally, it exposes a regression in NewDelete-intersections.mm's
testStandardPlacementNewAfterDelete() test, where the diagnostic is delayed
from before the call of placement new into the code of placement new
in the header. This happens because the check for pass-into-function-after-free
for placement arguments is located in checkNewAllocator(), which happens after
the allocator is inlined, which is too late. Move this use-after-free check
into checkPreCall instead, where it works automagically because the guard
that prevents it from working is useless and can be removed as well.

This commit causes regressions under -analyzer-config
c++-allocator-inlining=false but this option is essentially unsupported
because the respective feature has been enabled by default quite a while ago.

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

llvm-svn: 345802
2018-11-01 00:43:35 +00:00
George Karpenkov d3e7675331 [analyzer] [NFC] Change scanReachableSymbols to use ranges
Remove unused overload. Clean up some usages.

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

llvm-svn: 345101
2018-10-23 23:12:12 +00:00
George Karpenkov c82d457db5 [analyzer] [NFC] Remove unused parameters, as found by -Wunused-parameter
Differential Revision: https://reviews.llvm.org/D52640

llvm-svn: 343353
2018-09-28 18:49:41 +00:00
Reka Kovacs bb2749a594 [analyzer] InnerPointerChecker: improve warning messages and notes.
Differential Revision: https://reviews.llvm.org/D49570

llvm-svn: 339489
2018-08-10 23:56:57 +00:00
Reka Kovacs d9f66ba340 [analyzer] InnerPointerChecker: fix displayed checker name.
For InnerPointerChecker to function properly, both the checker itself
and parts of MallocChecker that handle relevant use-after-free problems
need to be turned on. So far, the latter part has been developed within
MallocChecker's NewDelete sub-checker, often causing warnings to appear
under that name. This patch defines a new CheckKind within MallocChecker
for the inner pointer checking functionality, so that the correct name
is displayed in warnings and in the ExplodedGraph.

Tested on clang-tidy.

Differential Review: https://reviews.llvm.org/D50211

llvm-svn: 339067
2018-08-06 22:03:42 +00:00
Reka Kovacs 122171e235 [analyzer] Detect pointers escaped after ReturnStmt execution in MallocChecker.
Objects local to a function are destroyed right after the statement returning
(part of) them is executed in the analyzer. This patch enables MallocChecker to
warn in these cases.

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

llvm-svn: 338780
2018-08-02 23:02:08 +00:00
Fangrui Song 6907ce2f8f Remove trailing space
sed -Ei 's/[[:space:]]+$//' include/**/*.{def,h,td} lib/**/*.{cpp,h}

llvm-svn: 338291
2018-07-30 19:24:48 +00:00
Reka Kovacs c74cfc4215 [analyzer] Add support for more invalidating functions in InnerPointerChecker.
According to the standard, pointers referring to the elements of a
`basic_string` may be invalidated if they are used as an argument to
any standard library function taking a reference to non-const
`basic_string` as an argument. This patch makes InnerPointerChecker warn
for these cases.

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

llvm-svn: 338259
2018-07-30 15:43:45 +00:00
Reka Kovacs 88ad704b5b [analyzer] Rename DanglingInternalBufferChecker to InnerPointerChecker.
Differential Revision: https://reviews.llvm.org/D49553

llvm-svn: 337559
2018-07-20 15:14:49 +00:00
Reka Kovacs a14a2fed38 [analyzer] Fix memory sanitizer error in MallocChecker.
StringRef's data() returns a string that may be non-null-terminated.
Switch to using StringRefs from const char pointers in visitor notes
to avoid problems.

llvm-svn: 337474
2018-07-19 17:43:09 +00:00
Reka Kovacs 52dd98bd1e [analyzer] Fix disappearing notes in DanglingInternalBufferChecker tests
Correct a mistake of the exact same kind I am writing this checker for.

llvm-svn: 337466
2018-07-19 15:44:46 +00:00
Reka Kovacs c18ecc8489 [analyzer] Add support for more basic_string API in
DanglingInternalBufferChecker.

A pointer referring to the elements of a basic_string may be invalidated
by calling a non-const member function, except operator[], at, front,
back, begin, rbegin, end, and rend. The checker now warns if the pointer
is used after such operations.

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

llvm-svn: 337463
2018-07-19 15:10:06 +00:00
Reka Kovacs e453e60d08 [analyzer] Highlight c_str() call in DanglingInternalBufferChecker.
Add a bug visitor to DanglingInternalBufferChecker that places a note
at the point where the dangling pointer was obtained. The visitor is
handed over to MallocChecker and attached to the report there.

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

llvm-svn: 336495
2018-07-07 19:27:18 +00:00
Reka Kovacs e0dae15d91 [analyzer] Fix -Wcovered-switch-default warning in MallocChecker.
Remove unnecessary default case that caused buildbot failures.

llvm-svn: 336493
2018-07-07 18:37:37 +00:00
Reka Kovacs 8707cd1d1b [analyzer] Highlight container object destruction in MallocChecker.
Extend MallocBugVisitor to place a note at the point where objects with
AF_InternalBuffer allocation family are destroyed.

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

llvm-svn: 336489
2018-07-07 17:22:45 +00:00
George Karpenkov dd18b11b8e [analyzer] [NFC] A convenient getter for getting a current stack frame
Differential Revision: https://reviews.llvm.org/D44756

llvm-svn: 335701
2018-06-27 01:51:55 +00:00
George Karpenkov 70ec1dd14d [analyzer] Do not run visitors until the fixpoint, run only once.
In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the currently processed path
is destroyed, all diagnostics is discarded, and it is regenerated again,
until it's no longer modified.
This pattern has a few negative implications:

 - This loop does not even guarantee to terminate.
   E.g. just imagine two visitors bouncing a diagnostics around.
 - Performance-wise, e.g. for sqlite3 all visitors are being re-run at
   least 10 times for some bugs.
   We have already seen a few reports where it leads to timeouts.
 - If we want to add more computationally intense visitors, this will
   become worse.
 - From architectural standpoint, the current layout requires copying
   visitors, which is conceptually wrong, and can be annoying (e.g. no
   unique_ptr on visitors allowed).

The proposed change is a much simpler architecture: the outer loop
processes nodes upwards, and whenever the visitor is added it only
processes current nodes and above, thus guaranteeing termination.

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

llvm-svn: 335666
2018-06-26 21:12:08 +00:00
Reka Kovacs 18775fc9b7 [analyzer] Add dangling internal buffer check.
This check will mark raw pointers to C++ standard library container internal
buffers 'released' when the objects themselves are destroyed. Such information
can be used by MallocChecker to warn about use-after-free problems.

In this first version, 'std::basic_string's are supported.

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

llvm-svn: 334348
2018-06-09 13:03:49 +00:00
Adrian Prantl 9fc8faf9e6 Remove \brief commands from doxygen comments.
This is similar to the LLVM change https://reviews.llvm.org/D46290.

We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.

Patch produced by

for i in $(git grep -l '\@brief'); do perl -pi -e 's/\@brief //g' $i & done
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done

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

llvm-svn: 331834
2018-05-09 01:00:01 +00:00
Artem Dergachev 806486c781 [analyzer] pr18953: Split C++ zero-initialization from default initialization.
The bindDefault() API of the ProgramState allows setting a default value
for reads from memory regions that were not preceded by writes.

It was used for implementing C++ zeroing constructors (i.e. default constructors
that boil down to setting all fields of the object to 0).

Because differences between zeroing consturctors and other forms of default
initialization have been piling up (in particular, zeroing constructors can be
called multiple times over the same object, probably even at the same offset,
requiring a careful and potentially slow cleanup of previous bindings in the
RegionStore), we split the API in two: bindDefaultInitial() for modeling
initial values and bindDefaultZero() for modeling zeroing constructors.

This fixes a few assertion failures from which the investigation originated.

The imperfect protection from both inability of the RegionStore to support
binding extents and lack of information in ASTRecordLayout has been loosened
because it's, well, imperfect, and it is unclear if it fixing more than it
was breaking.

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

llvm-svn: 331561
2018-05-04 21:56:51 +00:00
Joel E. Denny a6555114c8 [Attr] [NFC] Revert accidental change from r327405
llvm-svn: 329005
2018-04-02 19:43:34 +00:00
Artem Dergachev ff1fc21e8a [analyzer] Suppress more MallocChecker positives in smart pointer destructors.
r326249 wasn't quite enough because we often run out of inlining stack depth
limit and for that reason fail to see the atomics we're looking for.

Add a more straightforward false positive suppression that is based on the name
of the class. I.e. if we're releasing a pointer in a destructor of a "something
shared/intrusive/reference/counting something ptr/pointer something", then any
use-after-free or double-free that occurs later would likely be a false
positive.

rdar://problem/38013606

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

llvm-svn: 328066
2018-03-21 00:49:47 +00:00
Joel E. Denny 8150810556 Reland "[Attr] Fix parameter indexing for several attributes"
Relands r326602 (reverted in r326862) with new test and fix for
PR36620.

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

llvm-svn: 327405
2018-03-13 14:51:22 +00:00
Nico Weber bbf648253d Revert r326602, it caused PR36620.
llvm-svn: 326862
2018-03-07 02:22:41 +00:00
Joel E. Denny 4925445958 [Attr] Fix parameter indexing for several attributes
The patch fixes a number of bugs related to parameter indexing in
attributes:

* Parameter indices in some attributes (argument_with_type_tag,
  pointer_with_type_tag, nonnull, ownership_takes, ownership_holds,
  and ownership_returns) are specified in source as one-origin
  including any C++ implicit this parameter, were stored as
  zero-origin excluding any this parameter, and were erroneously
  printing (-ast-print) and confusingly dumping (-ast-dump) as the
  stored values.

* For alloc_size, the C++ implicit this parameter was not subtracted
  correctly in Sema, leading to assert failures or to silent failures
  of __builtin_object_size to compute a value.

* For argument_with_type_tag, pointer_with_type_tag, and
  ownership_returns, the C++ implicit this parameter was not added
  back to parameter indices in some diagnostics.

This patch fixes the above bugs and aims to prevent similar bugs in
the future by introducing careful mechanisms for handling parameter
indices in attributes.  ParamIdx stores a parameter index and is
designed to hide the stored encoding while providing accessors that
require each use (such as printing) to make explicit the encoding that
is needed.  Attribute declarations declare parameter index arguments
as [Variadic]ParamIdxArgument, which are exposed as ParamIdx[*].  This
patch rewrites all attribute arguments that are processed by
checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp to be declared
as [Variadic]ParamIdxArgument.  The only exception is xray_log_args's
argument, which is encoded as a count not an index.

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

llvm-svn: 326602
2018-03-02 19:03:22 +00:00
Artem Dergachev 5337efc69c [analyzer] MallocChecker: Suppress false positives in shared pointers.
Throw away MallocChecker warnings that occur after releasing a pointer within a
destructor (or its callees) after performing C11 atomic fetch_add or fetch_sub
within that destructor (or its callees).

This is an indication that the destructor's class is likely a
reference-counting pointer. The analyzer is not able to understand that the
original reference count is usually large enough to avoid most use-after-frees.

Even when the smart pointer is a local variable, we still have these false
positives that this patch suppresses, because the analyzer doesn't currently
support atomics well enough.

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

llvm-svn: 326249
2018-02-27 21:19:33 +00:00
Artem Dergachev 9849f595b9 [analyzer] MallocChecker: Fix one more bug category.
Even though most of the inconsistencies in MallocChecker's bug categories were
fixed in r302016, one more was introduced in r301913 which was later missed.

Patch by Henry Wong!

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

llvm-svn: 324680
2018-02-08 23:28:29 +00:00
Artem Dergachev 13b2026ba4 [analyzer] operator new: Add a new checker callback, check::NewAllocator.
The callback runs after operator new() and before the construction and allows
the checker to access the casted return value of operator new() (in the
sense of r322780) which is not available in the PostCall callback for the
allocator call.

Update MallocChecker to use the new callback instead of PostStmt<CXXNewExpr>,
which gets called after the constructor.

Differential Revision: https://reviews.llvm.org/D41406
rdar://problem/12180598

llvm-svn: 322787
2018-01-17 23:46:13 +00:00
George Karpenkov d703ec94a9 [analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the helper is used consistently
In most cases using
`N->getState()->getSVal(E, N->getLocationContext())`
is ugly, verbose, and also opens up more surface area for bugs if an
inconsistent location context is used.

This patch introduces a helper on an exploded node, and ensures
consistent usage of either `ExplodedNode::getSVal` or
`CheckContext::getSVal` across the codebase.
As a result, a large number of redundant lines is removed.

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

llvm-svn: 322753
2018-01-17 20:27:29 +00:00
Gabor Horvath b77bc6bb8b [analyzer] Fix some check's output plist not containing the check name
Differential Revision: https://reviews.llvm.org/D41538

llvm-svn: 321933
2018-01-06 10:51:00 +00:00
Artem Dergachev b6a513d1b2 [analyzer] Fix memory error bug category capitalization.
It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent with
other categories (such as "Logic error").

rdar://problem/31718115

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

llvm-svn: 302016
2017-05-03 11:47:13 +00:00
Daniel Marjamaki a43a8f5c5e [analyzer] Detect bad free of function pointers
Differential Revision: https://reviews.llvm.org/D31650

llvm-svn: 301913
2017-05-02 11:46:12 +00:00
Leslie Zhai e3986c544d [analyzer] Teach the MallocChecker about Glib API for two arguments
Reviewers: zaks.anna, NoQ, danielmarjamaki

Reviewed By: zaks.anna, NoQ, danielmarjamaki

Subscribers: cfe-commits, kalev, pwithnall

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

llvm-svn: 301384
2017-04-26 05:33:14 +00:00
Anna Zaks bbec97ca2c [analyzer] Teach the MallocChecker about about Glib API
A patch by Leslie Zhai!

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

llvm-svn: 297323
2017-03-09 00:01:01 +00:00
Anna Zaks b570195c3a [analyzer] Add LocationContext as a parameter to checkRegionChanges
This patch adds LocationContext to checkRegionChanges and removes
wantsRegionChangeUpdate as it was unused.

A patch by Krzysztof Wiśniewski!

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

llvm-svn: 291869
2017-01-13 00:50:57 +00:00
David Blaikie 0a0c275ffd Migrate PathDiagnosticPiece to std::shared_ptr
Simplifies and makes explicit the memory ownership model rather than
implicitly passing/acquiring ownership.

llvm-svn: 291143
2017-01-05 17:26:53 +00:00
Devin Coughlin e3b75ded98 [analyzer] Fix crash in MallocChecker.
Fix a crash in the MallocChecker when the extent size for the argument
to new[] is not known.

A patch by Abramo Bagnara and Dániel Krupp!

https://reviews.llvm.org/D27849

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

llvm-svn: 289970
2016-12-16 18:41:40 +00:00
Artem Dergachev 85c9211043 [analyzer] Add another exception for Qt in MallocChecker
Treat pointers passed to QObject::connectImpl() as escaping.

rdar://problem/29550440

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

llvm-svn: 289939
2016-12-16 12:21:55 +00:00
Devin Coughlin 684d19d879 Revert "Revert "[analyzer] Make MallocChecker more robust against custom redeclarations""
This reverts commit r284340 to reapply r284335. The bot breakage was due to
an unrelated change in the polybench test suite.

llvm-svn: 284351
2016-10-16 22:19:03 +00:00
Devin Coughlin eeb8d20db4 Revert "[analyzer] Make MallocChecker more robust against custom redeclarations"
This reverts commit r284335.

It appears to be causing test-suite compile-time and execution-time
performance measurements to take longer than expected on several bots.
This is surprising, because r284335 is a static-analyzer-only change.

llvm-svn: 284340
2016-10-16 19:26:07 +00:00
Devin Coughlin 62ce463a35 [analyzer] Make MallocChecker more robust against custom redeclarations
Add additional checking to MallocChecker to avoid crashing when memory
routines have unexpected numbers of arguments. You wouldn't expect to see much
of this in normal code (-Wincompatible-library-redeclaration warns on this),
but, for example, CMake tests can generate these.

This is PR30616.

rdar://problem/28631974

llvm-svn: 284335
2016-10-16 17:26:06 +00:00
Gabor Horvath 7304027c6d [analyzer] Calculate extent size for memory regions allocated by new expression.
ArrayBoundChecker did not detect out of bounds memory access errors in case an
array was allocated by the new expression. This patch resolves this issue.

Patch by Daniel Krupp!

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

llvm-svn: 281934
2016-09-19 20:39:52 +00:00
Gabor Horvath 6ee4f905dc [analyzer] Small cleanups when checkers retrieving statements from exploded
nodes.

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

llvm-svn: 279037
2016-08-18 07:54:50 +00:00
Mehdi Amini 9670f847b8 [NFC] Header cleanup
Summary: Removed unused headers, replaced some headers with forward class declarations

Patch by: Eugene <claprix@yandex.ru>

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

llvm-svn: 275882
2016-07-18 19:02:11 +00:00
David Majnemer 59f7792136 Use more ArrayRefs
No functional change is intended, just a small refactoring.

llvm-svn: 273647
2016-06-24 04:05:48 +00:00
Benjamin Kramer cfeacf56f0 Apply clang-tidy's misc-move-constructor-init throughout Clang.
No functionality change intended, maybe a tiny performance improvement.

llvm-svn: 270996
2016-05-27 14:27:13 +00:00
Anna Zaks 30d4668774 [analyzer] Fix missed leak from MSVC specific allocation functions
Add the wide character strdup variants (wcsdup, _wcsdup) and the MSVC
version of alloca (_alloca) and other differently named function used
by the Malloc checker.

A patch by Alexander Riccio!

Differential Revision: http://reviews.llvm.org/D17688

llvm-svn: 262894
2016-03-08 01:21:51 +00:00
Artem Dergachev 73f018e381 [analyzer] Fix SVal/SymExpr/MemRegion class and enum names for consistency.
The purpose of these changes is to simplify introduction of definition files
for the three hierarchies.

1. For every sub-class C of these classes, its kind in the relevant enumeration
is changed to "CKind" (or C##Kind in preprocessor-ish terms), eg:

  MemRegionKind   -> MemRegionValKind
  RegionValueKind -> SymbolRegionValueKind
  CastSymbolKind  -> SymbolCastKind
  SymIntKind      -> SymIntExprKind

2. MemSpaceRegion used to be inconsistently used as both an abstract base and
a particular region. This region class is now an abstract base and no longer
occupies GenericMemSpaceRegionKind. Instead, a new class, CodeSpaceRegion,
is introduced for handling the unique use case for MemSpaceRegion as
"the generic memory space" (when it represents a memory space that holds all
executable code).

3. BEG_ prefixes in memory region kind ranges are renamed to BEGIN_ for
consisitency with symbol kind ranges.

4. FunctionTextRegion and BlockTextRegion are renamed to FunctionCodeRegion and
BlockCodeRegion, respectively. The term 'code' is less jargony than 'text' and
we already refer to BlockTextRegion as a 'code region' in BlockDataRegion.

Differential Revision: http://reviews.llvm.org/D16062

llvm-svn: 257598
2016-01-13 13:49:29 +00:00
Anna Zaks 03f483353c [analyzer] Fix false warning about memory leak for QApplication::postEvent
According to Qt documentation Qt takes care of memory allocated for QEvent:
http://doc.qt.io/qt-4.8/qcoreapplication.html#postEvent

A patch by Evgeniy Dushistov!

Differential Revision: http://reviews.llvm.org/D14170

llvm-svn: 256887
2016-01-06 00:32:56 +00:00
Alexander Kornienko 9c10490efe Refactor: Simplify boolean conditional return statements in lib/StaticAnalyzer/Checkers
Summary: Use clang-tidy to simplify boolean conditional return values

Reviewers: dcoughlin, krememek

Subscribers: krememek, cfe-commits

Patch by Richard Thomson!

Differential Revision: http://reviews.llvm.org/D10021

llvm-svn: 256491
2015-12-28 13:06:58 +00:00
Anna Zaks fe1eca5169 [analyzer] Assume escape is possible through system functions taking void*
The analyzer assumes that system functions will not free memory or modify the
arguments in other ways, so we assume that arguments do not escape when
those are called. However, this may lead to false positive leak errors. For
example, in code like this where the pointers added to the rb_tree are freed
later on:

		struct alarm_event *e = calloc(1, sizeof(*e));
<snip>

		rb_tree_insert_node(&alarm_tree, e);

Add a heuristic to assume that calls to system functions taking void*
arguments allow for pointer escape.

llvm-svn: 251449
2015-10-27 20:19:45 +00:00
Devin Coughlin 8177173757 [analyzer] Make realloc(ptr, 0) handling equivalent to malloc(0).
Currently realloc(ptr, 0) is treated as free() which seems to be not correct. C
standard (N1570) establishes equivalent behavior for malloc(0) and realloc(ptr,
0): "7.22.3 Memory management functions calloc, malloc, realloc: If the size of
the space requested is zero, the behavior is implementation-defined: either a
null pointer is  returned, or the behavior is as if the size were some nonzero
value, except that the returned pointer shall not be used to access an object."
The patch equalizes the processing of malloc(0) and realloc(ptr,0). The patch
also enables unix.Malloc checker to detect references to zero-allocated memory
returned by realloc(ptr,0) ("Use of zero-allocated memory" warning).

A patch by Антон Ярцев!

Differential Revision: http://reviews.llvm.org/D9040

llvm-svn: 248336
2015-09-22 22:47:14 +00:00
Devin Coughlin e39bd407ba [analyzer] Add generateErrorNode() APIs to CheckerContext.
The analyzer trims unnecessary nodes from the exploded graph before reporting
path diagnostics. However, in some cases it can trim all nodes (including the
error node), leading to an assertion failure (see
https://llvm.org/bugs/show_bug.cgi?id=24184).

This commit addresses the issue by adding two new APIs to CheckerContext to
explicitly create error nodes. Unless the client provides a custom tag, these
APIs tag the node with the checker's tag -- preventing it from being trimmed.
The generateErrorNode() method creates a sink error node, while
generateNonFatalErrorNode() creates an error node for a path that should
continue being explored.

The intent is that one of these two methods should be used whenever a checker
creates an error node.

This commit updates the checkers to use these APIs. These APIs
(unlike addTransition() and generateSink()) do not take an explicit Pred node.
This is because there are not any error nodes in the checkers that were created
with an explicit different than the default (the CheckerContext's Pred node).

It also changes generateSink() to require state and pred nodes (previously
these were optional) to reduce confusion.

Additionally, there were several cases where checkers did check whether a
generated node could be null; we now explicitly check for null in these places.

This commit also includes a test case written by Ying Yi as part of
http://reviews.llvm.org/D12163 (that patch originally addressed this issue but
was reverted because it introduced false positive regressions).

Differential Revision: http://reviews.llvm.org/D12780

llvm-svn: 247859
2015-09-16 22:03:05 +00:00
Ted Kremenek 3a0678e33c [analyzer] Apply whitespace cleanups by Honggyu Kim.
llvm-svn: 246978
2015-09-08 03:50:52 +00:00
David Blaikie 6951e3e406 Wdeprecated: BugReporterVisitors are copied for cloning (BugReporterVisitorImpl), make sure such copies are safe
Make the copy/move ctors defaulted in the base class and make the
derived classes final to avoid any intermediate hierarchy slicing if
these types were further derived.

llvm-svn: 244979
2015-08-13 22:58:37 +00:00
David Blaikie 903c29347a Wdeprecated: CollectReachableSymbolsCallback are move constructed/returned by value, so make sure they're copy/moveable
(return by value is in ExprEngine::processPointerEscapedOnBind and any
other call to the scanReachableSymbols function template used there)

Protect the special members in the base class to avoid slicing, and make
derived classes final so these special members don't accidentally become
public on an intermediate base which would open up the possibility of
slicing again.

llvm-svn: 244975
2015-08-13 22:50:09 +00:00
Aaron Ballman 8d3a7a56a9 Clarify pointer ownership semantics by hoisting the std::unique_ptr creation to the caller instead of hiding it in emitReport. NFC.
llvm-svn: 240400
2015-06-23 13:15:32 +00:00
Anton Yartsev b50f4ba461 [analyzer] This implements potential undefbehavior.ZeroAllocDereference checker.
TODO: support realloc(). Currently it is not possible due to the present realloc() handling. Currently RegionState is not being attached to realloc() in case of a zero Size argument. 
llvm-svn: 234889
2015-04-14 14:18:04 +00:00
Alexander Kornienko 34eb20725d Use 'override/final' instead of 'virtual' for overridden methods
Summary:
The patch is generated using clang-tidy misc-use-override check.

This command was used:

  tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py \
    -checks='-*,misc-use-override' -header-filter='llvm|clang' -j=32 -fix

Reviewers: dblaikie

Reviewed By: dblaikie

Subscribers: klimek, cfe-commits

Differential Revision: http://reviews.llvm.org/D8926

llvm-svn: 234678
2015-04-11 02:00:23 +00:00
Anton Yartsev 2487dd6501 [analyzer] Make getCheckIfTracked() return either leak or regular checker.
llvm-svn: 231863
2015-03-10 22:24:21 +00:00
Anton Yartsev 80fce4e7da [analyzer] Remove unused type (addition to r231540: "Revert changes from r229593")
llvm-svn: 231548
2015-03-07 01:57:31 +00:00
Anton Yartsev 4eb394d6c1 [analyzer] Revert changes from r229593; an enhancement is under discussion
llvm-svn: 231540
2015-03-07 00:31:53 +00:00
Anton Yartsev 9907fc9053 [analyzer] Bug fix: do not report leaks for alloca()
llvm-svn: 231314
2015-03-04 23:18:21 +00:00
Gabor Horvath e40c71c10a [analyzer] Individual configuration options can be specified for checkers.
Reviewed by: Anna Zaks

Original patch by: Aleksei Sidorin

Differential Revision: http://reviews.llvm.org/D7905

llvm-svn: 231266
2015-03-04 17:59:34 +00:00
Anton Yartsev c38d7952b2 [analyzer] unix.Malloc: preserve AllocaRegion bound to __builtin_alloca().
Binding __builtin_alloca() return value to the symbolic value kills previous binding to a AllocaRegion established by the core.BuiltinFunctions checker. Other checkers may rely upon this information. Rollback handling of __builtin_alloca() to the way prior to r229850.

llvm-svn: 231160
2015-03-03 22:58:46 +00:00
Anton Yartsev 5b5c7cec08 [analyzer] Different handling of alloca().
+ separate bug report for "Free alloca()" error to be able to customize checkers responsible for this error.
+ Muted "Free alloca()" error for NewDelete checker that is not responsible for c-allocated memory, turned on for unix.MismatchedDeallocator checker.
+ RefState for alloca() - to be able to detect usage of zero-allocated memory by upcoming ZeroAllocDereference checker.
+ AF_Alloca family to handle alloca() consistently - keep proper family in RefState, handle 'alloca' by getCheckIfTracked() facility, etc.
+ extra tests.

llvm-svn: 229850
2015-02-19 13:36:20 +00:00
Anton Yartsev e5c0c14213 [analyzer] Refactoring: clarified the way the proper check kind is chosen.
llvm-svn: 229593
2015-02-18 00:39:06 +00:00
Anton Yartsev b3fa86d6f3 [analyzer] Pass actual state to alloc/dealloc handling functions.
The state obtained from CheckerContext::getState() may be outdated by the time the alloc/dealloc handling function is called (e.g. the state was modified but the transition was not performed). State argument was added to all alloc/dealloc handling functions in order to get the latest state and to allow sequential calls to those functions.

llvm-svn: 228737
2015-02-10 20:13:08 +00:00
Anna Zaks 486a0ff4b7 [analyzer] Look for allocation site in the parent frames as well as the current one.
Instead of handling edge cases (mostly involving blocks), where we have difficulty finding
an allocation statement, allow the allocation site to be in a parent node.

Previously we assumed that the allocation site can always be found in the same frame
as allocation, but there are scenarios in which an element is leaked in a child
frame but is allocated in the parent.

llvm-svn: 228247
2015-02-05 01:02:53 +00:00
Benjamin Kramer 719772c269 Remove stray enum keywords. MSVC sees this as a redeclaration at global scope.
llvm-svn: 219031
2014-10-03 22:20:30 +00:00
Anna Zaks d79b840716 [analyzer] Make Malloc Checker track memory allocated by if_nameindex
The MallocChecker does currently not track the memory allocated by
if_nameindex. That memory is dynamically allocated and should be freed
by calling if_freenameindex. The attached patch teaches the checker
about these functions.

Memory allocated by if_nameindex is treated as a separate allocation
"family". That way the checker can verify it is freed by the correct
function.

A patch by Daniel Fahlgren!

llvm-svn: 219025
2014-10-03 21:48:59 +00:00
Jordan Rose f69e65f75c [analyzer] Don't crash if malloc() has an unexpected function prototype.
Patch by Daniel Fahlgren!

llvm-svn: 217258
2014-09-05 16:33:51 +00:00
David Blaikie 91e7902622 unique_ptrify BugReporter::visitors
llvm-svn: 217205
2014-09-04 23:54:33 +00:00
David Blaikie d15481ccea unique_ptr-ify PathDiagnosticPiece ownership
llvm-svn: 216751
2014-08-29 18:18:43 +00:00
Anton Yartsev 4e4cb6bc30 [Analyzer] fix for PR19102
Newly-created unconsumed instance is now assumed escaped if an invoked constructor has an argument of a pointer-to-record type.

llvm-svn: 214909
2014-08-05 18:26:05 +00:00
Craig Topper 0dbb783c7b [C++11] Use 'nullptr'. StaticAnalyzer edition.
llvm-svn: 209642
2014-05-27 02:45:47 +00:00
Aaron Ballman a82eaa70f1 Updated the attribute tablegen emitter for variadic arguments to emit a range accessor in addition to the iterators. Updated code using iterators to use range-based for loops.
llvm-svn: 207837
2014-05-02 13:35:42 +00:00
Jordan Rose 6b33c6f234 [analyzer] Handle the M_ZERO and __GFP_ZERO flags in kernel mallocs.
Add M_ZERO awareness to malloc() static analysis in Clang for FreeBSD,
NetBSD, and OpenBSD in a similar fashion to O_CREAT for open(2).
These systems have a three-argument malloc() in the kernel where the
third argument contains flags; the M_ZERO flag will zero-initialize the
allocated buffer.

This should reduce the number of false positives when running static
analysis on BSD kernels.

Additionally, add kmalloc() (Linux kernel malloc()) and treat __GFP_ZERO
like M_ZERO on Linux.

Future work involves a better method of checking for named flags without
hardcoding values.

Patch by Conrad Meyer, with minor modifications by me.

llvm-svn: 204832
2014-03-26 17:05:46 +00:00
Craig Topper fb6b25b5e4 [C++11] Add 'override' keyword to virtual methods that override their base class.
llvm-svn: 203999
2014-03-15 04:29:04 +00:00
Aaron Ballman be22bcb180 [C++11] Replacing DeclBase iterators specific_attr_begin() and specific_attr_end() with iterator_range specific_attrs(). Updating all of the usages of the iterators with range-based for loops.
llvm-svn: 203474
2014-03-10 17:08:28 +00:00
Ahmed Charles b89843299a Replace OwningPtr with std::unique_ptr.
This compiles cleanly with lldb/lld/clang-tools-extra/llvm.

llvm-svn: 203279
2014-03-07 20:03:18 +00:00
Benjamin Kramer 867ea1d426 [C++11] Replace llvm::tie with std::tie.
llvm-svn: 202639
2014-03-02 13:01:17 +00:00
Anton Yartsev 6a61922239 [analyzer] Improved checker naming in CFG dump.
This implements FIXME from Checker.cpp (FIXME: We want to return the package + name of the checker here.) and replaces hardcoded checker names with the new ones obtained via getCheckName().getName().

llvm-svn: 201525
2014-02-17 18:25:34 +00:00
Alexander Kornienko 4aca9b1cd8 Expose the name of the checker producing each diagnostic message.
Summary:
In clang-tidy we'd like to know the name of the checker producing each
diagnostic message. PathDiagnostic has BugType and Category fields, which are
both arbitrary human-readable strings, but we need to know the exact name of the
checker in the form that can be used in the CheckersControlList option to
enable/disable the specific checker.

This patch adds the CheckName field to the CheckerBase class, and sets it in
the CheckerManager::registerChecker() method, which gets them from the
CheckerRegistry.

Checkers that implement multiple checks have to store the names of each check
in the respective registerXXXChecker method.

Reviewers: jordan_rose, krememek

Reviewed By: jordan_rose

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D2557

llvm-svn: 201186
2014-02-11 21:49:21 +00:00
Jordan Rose 6adadb9bc2 [analyzer] Future-proof MallocChecker CFG dumping.
Now we'll get a warning if we miss a case. Refinement of r199800.

llvm-svn: 199868
2014-01-23 03:59:01 +00:00
Anton Yartsev fee669054f [analyzer] The patch prevents the analyzer from crashing during CFG dump.
llvm-svn: 199800
2014-01-22 13:19:48 +00:00
Jordan Rose 2a833ca575 [analyzer] BlockCall shouldn't really be an AnyFunctionCall.
Per discussion with Anna a /long/ time ago, it was way too easy to misuse
BlockCall: because it inherited from AnyFunctionCall (through SimpleCall),
getDecl() was constrained to return a FunctionDecl, and you had to call
getBlockDecl() instead. This goes against the whole point of CallEvent
(to abstract over different ways to invoke bodies of code).

Now, BlockCall just inherits directly from CallEvent. There's a bit of
duplication in getting things out of the origin expression (which is still
known to be a CallExpr), but nothing significant.

llvm-svn: 199321
2014-01-15 17:25:15 +00:00
Jordan Rose 656fdd55dd [analyzer] Warn about double-delete in C++ at the second delete...
...rather somewhere in the destructor when we try to access something and
realize the object has already been deleted. This is necessary because
the destructor is processed before the 'delete' itself.

Patch by Karthik Bhat!

llvm-svn: 198779
2014-01-08 18:46:55 +00:00
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 ef6b007dc5 Only mark dump() function definitions 'used' in debug builds
This has the dual effect of (1) enabling more dead-stripping in release builds
and (2) ensuring that debug helper functions aren't stripped away in debug
builds, as they're intended to be called from the debugger.

Note that the attribute is applied to definitions rather than declarations in
headers going forward because it's now conditional on NDEBUG:

  /// \brief Mark debug helper function definitions like dump() that should not be
  /// stripped from debug builds.

Requires corresponding macro added in LLVM r198456.

llvm-svn: 198489
2014-01-04 13:47:14 +00:00
Aaron Ballman b190f974c9 Fixed a FIXME; created a print method for Selectors that accepts a raw_ostream, and started using it in places it made sense.
No functional changes intended, just API cleanliness.

llvm-svn: 198428
2014-01-03 17:59:55 +00:00
Alp Toker 5faf0c00dc Correct a user-visible static analyzer message typo
llvm-svn: 196062
2013-12-02 03:50:25 +00:00
Richard Smith 852e9ce3dd Remove 'DistinctSpellings' support from Attr.td and change its only user to
look at the attribute spelling instead. The 'ownership_*' attributes should
probably be split into separate *Attr classes, but that's more than I wanted to
do here.

llvm-svn: 195805
2013-11-27 01:46:48 +00:00