Upon calling one of the functions `std::advance()`, `std::prev()` and
`std::next()` iterators could get out of their valid range which leads
to undefined behavior. If all these funcions are inlined together with
the functions they call internally (e.g. `__advance()` called by
`std::advance()` in some implementations) the error is detected by
`IteratorRangeChecker` but the bug location is inside the STL
implementation. Even worse, if the budget runs out and one of the calls
is not inlined the bug remains undetected. This patch fixes this
behavior: all the bugs are detected at the point of the STL function
invocation.
Differential Revision: https://reviews.llvm.org/D76379
Whenever the analyzer budget runs out just at the point where
`std::advance()`, `std::prev()` or `std::next()` is invoked the function
are not inlined. This results in strange behavior such as
`std::prev(v.end())` equals `v.end()`. To prevent this model these
functions if they were not inlined. It may also happend that although
`std::advance()` is inlined but a function it calls inside (e.g.
`__advance()` in some implementations) is not. This case is also handled
in this patch.
Differential Revision: https://reviews.llvm.org/D76361
Summary:
Currently, ValueRange is very hard to extend with new kind of constraints.
For instance, it forcibly encapsulates relations between arguments and the
return value (ComparesToArgument) besides handling the regular value
ranges (OutOfRange, WithinRange).
ValueRange in this form is not suitable to add new constraints on
arguments like "not-null".
This refactor introduces a new base class ValueConstraint with an
abstract apply function. Descendants must override this. There are 2
descendants: RangeConstraint and ComparisonConstraint. In the following
patches I am planning to add the NotNullConstraint, and additional
virtual functions like `negate()` and `warning()`.
Reviewers: NoQ, Szelethus, balazske, gamesh411, baloghadamsoftware, steakhal
Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74973
This makes life easier for downstream users who maintain exotic
target platforms.
Patch by Vince Bridgers!
Differential Revision: https://reviews.llvm.org/D75529
error: default initialization of an object of const type
'const clang::QualType' without a user-provided
default constructor
Irrelevant; // A placeholder, whenever we do not care about the type.
^
{}
Lambdas creating path notes using NoteTags still take BugReport as their
parameter. Since path notes obviously only appear in PathSensitiveBugReports
it is straightforward that lambdas of NoteTags take PathSensitiveBugReport
as their parameter.
Differential Revision: https://reviews.llvm.org/D75898
Summary:
According to documentations, after an `fclose` call any other stream
operations cause undefined behaviour, regardless if the close failed
or not.
This change adds the check for the opened state before all other
(applicable) operations.
Reviewers: Szelethus
Reviewed By: Szelethus
Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75614
Summary:
Adding PreCall callback.
Argument validity checks are moved into the PreCall callback.
Code is restructured, functions renamed.
There are "pre" and "eval" functions for the file operations.
And additional state check (validate) functions.
Reviewers: Szelethus
Reviewed By: Szelethus
Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75612
Summary:
Intended to be a non-functional change but it turned out CallEvent handles
constructor calls unlike CallExpr which doesn't triggered for constructors.
All in all, this change shouldn't be observable since constructors are not
yet propagating taintness like functions.
In the future constructors should propagate taintness as well.
This change includes:
- NFCi change all uses of the CallExpr to CallEvent
- NFC rename some functions, mark static them etc.
- NFC omit explicit TaintPropagationRule type in switches
- NFC apply some clang-tidy fixits
Reviewers: NoQ, Szelethus, boga95
Reviewed By: Szelethus
Subscribers: martong, whisperity, xazax.hun, baloghadamsoftware, szepet,
a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D72035
Summary:
This patch introduces the `clang_analyzer_isTainted` expression inspection
check for checking taint.
Using this we could query the analyzer whether the expression used as the
argument is tainted or not. This would be useful in tests, where we don't want
to issue warning for all tainted expressions in a given file
(like the `debug.TaintTest` would do) but only for certain expressions.
Example usage:
```lang=c++
int read_integer() {
int n;
clang_analyzer_isTainted(n); // expected-warning{{NO}}
scanf("%d", &n);
clang_analyzer_isTainted(n); // expected-warning{{YES}}
clang_analyzer_isTainted(n + 2); // expected-warning{{YES}}
clang_analyzer_isTainted(n > 0); // expected-warning{{YES}}
int next_tainted_value = n; // no-warning
return n;
}
```
Reviewers: NoQ, Szelethus, baloghadamsoftware, xazax.hun, boga95
Reviewed By: Szelethus
Subscribers: martong, rnkovacs, whisperity, xazax.hun,
baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy,
Charusso, cfe-commits, boga95, dkrupp, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74131
Summary:
Have a description object for the stream functions
that can store different aspects of a single stream operation.
I plan to extend the structure with other members,
for example pre-callback and index of the stream argument.
Reviewers: Szelethus, baloghadamsoftware, NoQ, martong, Charusso, xazax.hun
Reviewed By: Szelethus
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75158
Exactly what it says on the tin! I decided not to merge this with the patch that
changes all these to a CallDescriptionMap object, so the patch is that much more
trivial.
Differential Revision: https://reviews.llvm.org/D68163
Currently, using negative numbers in iterator operations (additions and
subractions) results in advancements with huge positive numbers due to
an error. This patch fixes it.
Differential Revision: https://reviews.llvm.org/D74760
The following series of refactoring patches aim to fix the horrible mess that MallocChecker.cpp is.
I genuinely hate this file. It goes completely against how most of the checkers
are implemented, its by far the biggest headache regarding checker dependencies,
checker options, or anything you can imagine. On top of all that, its just bad
code. Its seriously everything that you shouldn't do in C++, or any other
language really. Bad variable/class names, in/out parameters... Apologies, rant
over.
So: there are a variety of memory manipulating function this checker models. One
aspect of these functions is their AllocationFamily, which we use to distinguish
between allocation kinds, like using free() on an object allocated by operator
new. However, since we always know which function we're actually modeling, in
fact we know it compile time, there is no need to use tricks to retrieve this
information out of thin air n+1 function calls down the line. This patch changes
many methods of MallocChecker to take a non-optional AllocationFamily template
parameter (which also makes stack dumps a bit nicer!), and removes some no
longer needed auxiliary functions.
Differential Revision: https://reviews.llvm.org/D68162
Summary:
PutenvWithAutoChecker.cpp used to include "AllocationState.h" that is present in project root.
This makes build systems like blaze unhappy. Made it include the header relative to source file.
Reviewers: kadircet
Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74906
Summary:
This patch introduces a new checker:
`alpha.security.cert.pos.34c`
This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/x/6NYxBQ
The check warns if `putenv` function is
called with automatic storage variable as an argument.
Differential Revision: https://reviews.llvm.org/D71433
In the path-sensitive vfork() checker that keeps a list of operations
allowed after a successful vfork(), unforget to include execve() in the list.
Patch by Jan Včelák!
Differential Revision: https://reviews.llvm.org/D73629
Summary:
Both EOF and the max value of unsigned char is platform dependent. In this
patch we try our best to deduce the value of EOF from the Preprocessor,
if we can't we fall back to -1.
Reviewers: Szelethus, NoQ
Subscribers: whisperity, xazax.hun, kristof.beyls, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalh
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74473
Summary:
Simplifies the C++11-style "-> decltype(...)" return-type deduction.
Note that you have to be careful about whether the function return type
is `auto` or `decltype(auto)`. The difference is that bare `auto`
strips const and reference, just like lambda return type deduction. In
some cases that's what we want (or more likely, we know that the return
type is a value type), but whenever we're wrapping a templated function
which might return a reference, we need to be sure that the return type
is decltype(auto).
No functional change.
Reviewers: bkramer, MaskRay, martong, shafik
Subscribers: martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74423
STL Algorithms are usually implemented in a tricky for performance
reasons which is too complicated for the analyzer. Furthermore inlining
them is costly. Instead of inlining we should model their behavior
according to the specifications.
This patch is the first step towards STL Algorithm modeling. It models
all the `find()`-like functions in a simple way: the result is either
found or not. In the future it can be extended to only return success if
container modeling is also extended in a way the it keeps track of
trivial insertions and deletions.
Differential Revision: https://reviews.llvm.org/D70818
Summary:
This patch uses the new `DynamicSize.cpp` to serve dynamic information.
Previously it was static and probably imprecise data.
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D69599
Summary:
This patch introduces a placeholder for representing the dynamic size of
regions. It also moves the `getExtent()` method of `SubRegions` to the
`MemRegionManager` as `getStaticSize()`.
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D69540
Iterator modeling depends on container modeling,
but not vice versa. This enables the possibility
to arrange these two modeling checkers into
separate layers.
There are several advantages for doing this: the
first one is that this way we can keep the
respective modeling checkers moderately simple
and small. Furthermore, this enables creation of
checkers on container operations which only
depend on the container modeling. Thus iterator
modeling can be disabled together with the
iterator checkers if they are not needed.
Since many container operations also affect
iterators, container modeling also uses the
iterator library: it creates iterator positions
upon calling the `begin()` or `end()` method of
a containter (but propagation of the abstract
position is left to the iterator modeling),
shifts or invalidates iterators according to the
rules upon calling a container modifier and
rebinds the iterator to a new container upon
`std::move()`.
Iterator modeling propagates the abstract
iterator position, handles the relations between
iterator positions and models iterator
operations such as increments and decrements.
Differential Revision: https://reviews.llvm.org/D73547
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
These are mostly trivial additions as both of them are reusing existing
PThreadLockChecker logic. I only needed to add the list of functions to
check and do some plumbing to make sure that we display the right
checker name in the diagnostic.
Differential Revision: https://reviews.llvm.org/D73376
Summary:
Instead of checking the range manually, changed the checker to use assumeInclusiveRangeDual instead.
This patch was part of D28955.
Reviewers: NoQ
Reviewed By: NoQ
Subscribers: ddcc, xazax.hun, baloghadamsoftware, szepet, a.sidorin, Szelethus, donat.nagy, dkrupp, Charusso, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73062
Summary:
This checker verifies if default placement new is provided with pointers
to sufficient storage capacity.
Noncompliant Code Example:
#include <new>
void f() {
short s;
long *lp = ::new (&s) long;
}
Based on SEI CERT rule MEM54-CPP
https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM54-CPP.+Provide+placement+new+with+properly+aligned+pointe
This patch does not implement checking of the alignment.
Reviewers: NoQ, xazax.hun
Subscribers: mgorny, whisperity, xazax.hun, baloghadamsoftware, szepet,
rnkovacs, a.sidorin, mikhail.ramalho, donat
Tags: #clang
Differential Revision: https://reviews.llvm.org/D71612
This avoids unneeded copies when using a range-based for loops.
This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.
Differential Revision: https://reviews.llvm.org/D70869
Method '-[NSCoder decodeValueOfObjCType:at:]' is not only deprecated
but also a security hazard, hence a loud check.
Differential Revision: https://reviews.llvm.org/D71728
MallocChecker warns when memory is passed into -[NSData initWithBytesNoCopy]
but isn't allocated by malloc(), because it will be deallocated by free().
However, initWithBytesNoCopy has an overload that takes an arbitrary block
for deallocating the object. If such overload is used, it is no longer
necessary to make sure that the memory is allocated by malloc().
This patch introduces the namespaces for the configured functions and
also enables the use of the member functions.
I added an optional Scope field for every configured function. Functions
without Scope match for every function regardless of the namespace.
Functions with Scope will match if the full name of the function starts
with the Scope.
Multiple functions can exist with the same name.
Differential Revision: https://reviews.llvm.org/D70878
This patch introduced additional PointerEscape callbacks after conservative
calls for output parameters. This should not really affect the current
checkers but the upcoming FuchsiaHandleChecker relies on this heavily.
Differential Revision: https://reviews.llvm.org/D71224
The checker was trying to analyze the body of every method in Objective-C
@implementation clause but the sythesized accessor stubs that were introduced
into it by 2073dd2d have no bodies.
While analyzing code `memcmp(a, NULL, n);', where `a' has an unconstrained
symbolic value, the analyzer was emitting a warning about the *first* argument
being a null pointer, even though we'd rather have it warn about the *second*
argument.
This happens because CStringChecker first checks whether the two argument
buffers are in fact the same buffer, in order to take the fast path.
This boils down to assuming `a == NULL' to true. Then the subsequent check
for null pointer argument "discovers" that `a' is null.
Don't take the fast path unless we are *sure* that the buffers are the same.
Otherwise proceed as normal.
Differential Revision: https://reviews.llvm.org/D71322
Sometimes the return value of a comparison operator call is
`UnkownVal`. Since no assumptions can be made on `UnknownVal`,
this leeds to keeping impossible execution paths in the
exploded graph resulting in poor performance and false
positives. To overcome this we replace unknown results of
iterator comparisons by conjured symbols.
Differential Revision: https://reviews.llvm.org/D70244
Debugging the Iterator Modeling checker or any of the iterator checkers
is difficult without being able to see the relations between the
iterator variables and their abstract positions, as well as the abstract
symbols denoting the begin and the end of the container.
This patch adds the checker-specific part of the Program State printing
to the Iterator Modeling checker.
A monolithic checker class is hard to maintain. This patch splits it up
into a modeling part, the three checkers and a debug checker. The common
functions are moved into a library.
Differential Revision: https://reviews.llvm.org/D70320
This patch is the last of the series of patches which allow the user to
annotate their functions with taint propagation rules.
I implemented the use of the configured filtering functions. These
functions can remove taintedness from the symbols which are passed at
the specified arguments to the filters.
Differential Revision: https://reviews.llvm.org/D59516
This patch is motivated by (and factored out from)
https://reviews.llvm.org/D66121 which is a debug info bugfix. Starting
with DWARF 5 all Objective-C methods are nested inside their
containing type, and that patch implements this for synthesized
Objective-C properties.
1. SemaObjCProperty populates a list of synthesized accessors that may
need to inserted into an ObjCImplDecl.
2. SemaDeclObjC::ActOnEnd inserts forward-declarations for all
accessors for which no override was provided into their
ObjCImplDecl. This patch does *not* synthesize AST function
*bodies*. Moving that code from the static analyzer into Sema may
be a good idea though.
3. Places that expect all methods to have bodies have been updated.
I did not update the static analyzer's inliner for synthesized
properties to point back to the property declaration (see
test/Analysis/Inputs/expected-plists/nullability-notes.m.plist), which
I believed to be more bug than a feature.
Differential Revision: https://reviews.llvm.org/D68108
rdar://problem/53782400
For white-box testing correct container and iterator modelling it is essential
to access the internal data structures stored for container and iterators. This
patch introduces a simple debug checkers called debug.IteratorDebugging to
achieve this.
Differential Revision: https://reviews.llvm.org/D67156
- Fix false positive reports of strlcat.
- The return value of strlcat and strlcpy is now correctly calculated.
- The resulting string length of strlcat and strlcpy is now correctly
calculated.
Patch by Daniel Krupp!
Differential Revision: https://reviews.llvm.org/D66049
Summary:
Recognization of function names is done now with the CallDescription
class instead of using IdentifierInfo. This means function name and
argument count is compared too.
A new check for filtering not global-C-functions was added.
Test was updated.
Reviewers: Szelethus, NoQ, baloghadamsoftware, Charusso
Reviewed By: Szelethus, NoQ, Charusso
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy, Charusso, dkrupp, Szelethus, gamesh411, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D67706
The static analyzer is warning about a potential null dereference, but we should be able to use cast<> directly and if not assert will fire for us.
llvm-svn: 374717
Some compilers have trouble converting unique_ptr<PathSensitiveBugReport> to
unique_ptr<BugReport> causing some functions to fail to compile.
Changing the return type of the functions that fail to compile does not
appear to have any issues.
I ran into this issue building with clang 3.8 on Ubuntu 16.04.
llvm-svn: 372668
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
At this point the PathDiagnostic, PathDiagnosticLocation, PathDiagnosticPiece
structures no longer rely on anything specific to Static Analyzer, so we can
move them out of it for everybody to use.
PathDiagnosticConsumers are still to be handed off.
Differential Revision: https://reviews.llvm.org/D67419
llvm-svn: 371661
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
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
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
Allow attaching fixit hints to Static Analyzer BugReports.
Fixits are attached either to the bug report itself or to its notes
(path-sensitive event notes or path-insensitive extra notes).
Add support for fixits in text output (including the default text output that
goes without notes, as long as the fixit "belongs" to the warning).
Add support for fixits in the plist output mode.
Implement a fixit for the path-insensitive DeadStores checker. Only dead
initialization warning is currently covered.
Implement a fixit for the path-sensitive VirtualCall checker when the virtual
method is not pure virtual (in this case the "fix" is to suppress the warning
by qualifying the call).
Both fixits are under an off-by-default flag for now, because they
require more careful testing.
Differential Revision: https://reviews.llvm.org/D65182
llvm-svn: 371257
Most functions that our checkers react upon are not C-style variadic functions,
and therefore they have as many actual arguments as they have formal parameters.
However, it's not impossible to define a variadic function with the same name.
This will crash any checker that relies on CallDescription to check the number
of arguments but silently assumes that the number of parameters is the same.
Change CallDescription to check both the number of arguments and the number of
parameters by default.
If we're intentionally trying to match variadic functions, allow specifying
arguments and parameters separately (possibly omitting any of them).
For now we only have one CallDescription which would make use of those,
namely __builtin_va_start itself.
Differential Revision: https://reviews.llvm.org/D67019
llvm-svn: 371256
There are some functions which can't be given a null pointer as parameter either
because it has a nonnull attribute or it is declared to have undefined behavior
(e.g. strcmp()). Sometimes it is hard to determine from the checker message
which parameter is null at the invocation, so now this information is included
in the message.
This commit fixes https://bugs.llvm.org/show_bug.cgi?id=39358
Reviewed By: NoQ, Szelethus, whisperity
Patch by Tibor Brunner!
Differential Revision: https://reviews.llvm.org/D66333
llvm-svn: 370798
Enables the users to specify an optional flag which would warn for more dead
stores.
Previously it ignored if the dead store happened e.g. in an if condition.
if ((X = generate())) { // dead store to X
}
This patch introduces the `WarnForDeadNestedAssignments` option to the checker,
which is `false` by default - so this change would not affect any previous
users.
I have updated the code, tests and the docs as well. If I missed something, tell
me.
I also ran the analysis on Clang which generated 14 more reports compared to the
unmodified version. All of them seemed reasonable for me.
Related previous patches:
rGf224820b45c6847b91071da8d7ade59f373b96f3
Reviewers: NoQ, krememek, Szelethus, baloghadamsoftware
Reviewed By: Szelethus
Patch by Balázs Benics!
Differential Revision: https://reviews.llvm.org/D66733
llvm-svn: 370767
Range errors (dereferencing or incrementing the past-the-end iterator or
decrementing the iterator of the first element of the range) and access of
invalidated iterators lead to undefined behavior. There is no point to
continue the analysis after such an error on the same execution path, but
terminate it by a sink node (fatal error). This also improves the
performance and helps avoiding double reports (e.g. in case of nested
iterators).
Differential Revision: https://reviews.llvm.org/D62893
llvm-svn: 370314
Write tests for the actual crash that was found. Write comments and refactor
code around 17 style bugs and suppress 3 false positives.
Differential Revision: https://reviews.llvm.org/D66847
llvm-svn: 370246
Summary: EnumCastOutOfRangeChecker should not perform enum range checks on LValueToRValue casts, since this type of cast does not actually change the underlying type. Performing the unnecessary check actually triggered an assertion failure deeper in EnumCastOutOfRange for certain input (which is captured in the accompanying test code).
Reviewers: #clang, Szelethus, gamesh411, NoQ
Reviewed By: Szelethus, gamesh411, NoQ
Subscribers: NoQ, gamesh411, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, bjope, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66014
llvm-svn: 369760
Our SVal hierarchy doesn't allow modeling pointer casts as no-op. The
pointer type is instead encoded into the pointer object. Defer to our
usual pointer casting facility, SValBuilder::evalBinOp().
Fixes a crash.
llvm-svn: 369729
Summary:
This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `CastSets` which are storing the dynamic cast informations of
objects based on memory regions. It could be used to store and check the
casts and prevent infeasible paths.
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D66325
llvm-svn: 369605
Calling a pure virtual method during construction or destruction
is undefined behavior. It's worth it to warn about it by default.
That part is now known as the cplusplus.PureVirtualCall checker.
Calling a normal virtual method during construction or destruction
may be fine, but does behave unexpectedly, as it skips virtual dispatch.
Do not warn about this by default, but let projects opt in into it
by enabling the optin.cplusplus.VirtualCall checker manually.
Give the two parts differentiated warning text:
Before:
Call to virtual function during construction or destruction:
Call to pure virtual function during construction
Call to virtual function during construction or destruction:
Call to virtual function during destruction
After:
Pure virtual method call:
Call to pure virtual method 'X::foo' during construction
has undefined behavior
Unexpected loss of virtual dispatch:
Call to virtual method 'Y::bar' during construction
bypasses virtual dispatch
Also fix checker names in consumers that support them (eg., clang-tidy)
because we now have different checker names for pure virtual calls and
regular virtual calls.
Also fix capitalization in the bug category.
Differential Revision: https://reviews.llvm.org/D64274
llvm-svn: 369449
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
Summary:
Explicitly deleting the copy constructor makes compiling the function
`ento::registerGenericTaintChecker` difficult with some compilers. When we
construct an `llvm::Optional<TaintConfig>`, the optional is constructed with a
const TaintConfig reference which it then uses to invoke the deleted TaintConfig
copy constructor.
I've observered this failing with clang 3.8 on Ubuntu 16.04.
Reviewers: compnerd, Szelethus, boga95, NoQ, alexshap
Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, llvm-commits, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66192
llvm-svn: 368779
When we're tracking a variable that is responsible for a null pointer
dereference or some other sinister programming error, we of course would like to
gather as much information why we think that the variable has that specific
value as possible. However, the newly introduced condition tracking shows that
tracking all values this thoroughly could easily cause an intolerable growth in
the bug report's length.
There are a variety of heuristics we discussed on the mailing list[1] to combat
this, all of them requiring to differentiate in between tracking a "regular
value" and a "condition".
This patch introduces the new `bugreporter::TrackingKind` enum, adds it to
several visitors as a non-optional argument, and moves some functions around to
make the code a little more coherent.
[1] http://lists.llvm.org/pipermail/cfe-dev/2019-June/062613.html
Differential Revision: https://reviews.llvm.org/D64270
llvm-svn: 368777
I feel this is kinda important, because in a followup patch I'm adding different
kinds of interestingness, and propagating the correct kind in BugReporter.cpp is
just one less thing to worry about.
Differential Revision: https://reviews.llvm.org/D65578
llvm-svn: 368755
When I'm new to a file/codebase, I personally find C++'s strong static type
system to be a great aid. BugReporter.cpp is still painful to read however:
function calls are made with mile long parameter lists, seemingly all of them
taken with a non-const reference/pointer. This patch fixes nothing but this:
make a few things const, and hammer it until it compiles.
Differential Revision: https://reviews.llvm.org/D65382
llvm-svn: 368735
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
While we implemented taint propagation rules for several
builtin/standard functions, there's a natural desire for users to add
such rules to custom functions.
A series of patches will implement an option that allows users to
annotate their functions with taint propagation rules through a YAML
file. This one adds parsing of the configuration file, which may be
specified in the commands line with the analyzer config:
alpha.security.taint.TaintPropagation:Config. The configuration may
contain propagation rules, filter functions (remove taint) and sink
functions (give a warning if it gets a tainted value).
I also added a new header for future checkers to conveniently read YAML
files as checker options.
Differential Revision: https://reviews.llvm.org/D59555
llvm-svn: 367190
Summary:
Integer Set Library using retain-count based allocation which is not
modeled in MallocChecker.
Reviewed By: NoQ
Tags: #clang
Differential Revision: https://reviews.llvm.org/D64680
llvm-svn: 366391
Summary:
It models the LLVM casts:
- `cast<>`
- `dyn_cast<>`
- `cast_or_null<>`
- `dyn_cast_or_null<>`
It has a very basic support without checking the `classof()` function.
(It reapplies the reverted 'llvm-svn: 365582' patch with proper test file.)
Reviewed By: NoQ
Tags: #clang
Differential Revision: https://reviews.llvm.org/D64374
llvm-svn: 365585
Summary:
It models the LLVM casts:
- `cast<>`
- `dyn_cast<>`
- `cast_or_null<>`
- `dyn_cast_or_null<>`
It has a very basic support without checking the `classof()` function.
Reviewed By: NoQ
Tags: #clang
Differential Revision: https://reviews.llvm.org/D64374
llvm-svn: 365582
I intend to improve the analyzer's bug reports by tracking condition
expressions.
01 bool b = messyComputation();
02 int i = 0;
03 if (b) // control dependency of the bug site, let's explain why we assume val
04 // to be true
05 10 / i; // warn: division by zero
I'll detail this heuristic in the followup patch, strictly related to this one
however:
* Create the new ControlDependencyCalculator class that uses llvm::IDFCalculator
to (lazily) calculate control dependencies for Clang's CFG.
* A new debug checker debug.DumpControlDependencies is added for lit tests
* Add unittests
Differential Revision: https://reviews.llvm.org/D62619
llvm-svn: 365197
Transform clang::DominatorTree to be able to also calculate post dominators.
* Tidy up the documentation
* Make it clang::DominatorTree template class (similarly to how
llvm::DominatorTreeBase works), rename it to clang::CFGDominatorTreeImpl
* Clang's dominator tree is now called clang::CFGDomTree
* Clang's brand new post dominator tree is called clang::CFGPostDomTree
* Add a lot of asserts to the dump() function
* Create a new checker to test the functionality
Differential Revision: https://reviews.llvm.org/D62551
llvm-svn: 365028
The NonnullGlobalConstants checker models the rule "it doesn't make sense
to make a constant global pointer and initialize it to null"; it makes sure
that whatever it's initialized with is known to be non-null.
Ironically, annotating the type of the pointer as _Nonnull breaks the checker.
Fix handling of the _Nonnull annotation so that it was instead one more reason
to believe that the value is non-null.
Differential Revision: https://reviews.llvm.org/D63956
llvm-svn: 364869
This patch uses the new CDF_MaybeBuiltin flag to handle C library functions.
It's mostly an NFC/refactoring pass, but it does fix a bug in handling memset()
when it expands to __builtin___memset_chk() because the latter has
one more argument and memset() handling code was trying to match
the exact number of arguments. Now the code is deduplicated and there's
less room for mistakes.
Differential Revision: https://reviews.llvm.org/D62557
llvm-svn: 364868
This changes the checker callback signature to use the modern, easy to
use interface. Additionally, this unblocks future work on allowing
checkers to implement evalCall() for calls that don't correspond to any
call-expression or require additional information that's only available
as part of the CallEvent, such as C++ constructors and destructors.
Differential Revision: https://reviews.llvm.org/D62440
llvm-svn: 363893
IIG is a replacement for MIG in DriverKit: IIG is autogenerating C++ code.
Suppress dead store warnings on such code, as the tool seems to be producing
them regularly, and the users of IIG are not in position to address these
warnings, as they don't control the autogenerated code. IIG-generated code
is identified by looking at the comments at the top of the file.
Differential Revision: https://reviews.llvm.org/D63118
llvm-svn: 363892
The `cplusplus.SelfAssignment` checker has a visitor that is added
to every `BugReport` to mark the to branch of the self assignment
operator with e.g. `rhs == *this` and `rhs != *this`. With the new
`NoteTag` feature this visitor is not needed anymore. Instead the
checker itself marks the two branches using the `NoteTag`s.
Differential Revision: https://reviews.llvm.org/D62479
llvm-svn: 361818
Turn it into a variant class instead. This conversion does indeed save some code
but there's a plan to add support for more kinds of terminators that aren't
necessarily based on statements, and with those in mind it becomes more and more
confusing to have CFGTerminators implicitly convertible to a Stmt *.
Differential Revision: https://reviews.llvm.org/D61814
llvm-svn: 361586
This patch refactors begin and end symbol creation by moving symbol
conjuration into the `create...` functions. This way the functions'
responsibilities are clearer and makes possible to add more functions
handling these symbols (e.g. functions for handling the container's
size) without code multiplication.
Differential Revision: https://reviews.llvm.org/D61136
llvm-svn: 361141
Since D57922, the config table contains every checker option, and it's default
value, so having it as an argument for getChecker*Option is redundant.
By the time any of the getChecker*Option function is called, we verified the
value in CheckerRegistry (after D57860), so we can confidently assert here, as
any irregularities detected at this point must be a programmer error. However,
in compatibility mode, verification won't happen, so the default value must be
restored.
This implies something else, other than adding removing one more potential point
of failure -- debug.ConfigDumper will always contain valid values for
checker/package options!
Differential Revision: https://reviews.llvm.org/D59195
llvm-svn: 361042
The checker was crashing when it was trying to assume a structure
to be null or non-null so that to evaluate the effect of the annotation.
Differential Revision: https://reviews.llvm.org/D61958
llvm-svn: 360790
Suppress MIG checker false positives that occur when the programmer increments
the reference count before calling a MIG destructor, and the MIG destructor
literally boils down to decrementing the reference count.
Differential Revision: https://reviews.llvm.org/D61925
llvm-svn: 360737
new expression.
This was voted into C++20 as a defect report resolution, so we
retroactively apply it to all prior language modes (though it can never
actually be used before C++11 mode).
llvm-svn: 360006
https://bugs.llvm.org/show_bug.cgi?id=41741
Pretty much the same as D61246 and D61106, this time for __complex__ types. Upon
further investigation, I realized that we should regard all types
Type::isScalarType returns true for as primitive, so I merged
isMemberPointerType(), isBlockPointerType() and isAnyComplexType()` into that
instead.
I also stumbled across yet another bug,
https://bugs.llvm.org/show_bug.cgi?id=41753, but it seems to be unrelated to
this checker.
Differential Revision: https://reviews.llvm.org/D61569
llvm-svn: 359998
https://bugs.llvm.org/show_bug.cgi?id=41611
Similarly to D61106, the checker ran over an llvm_unreachable for vector types:
struct VectorSizeLong {
VectorSizeLong() {}
__attribute__((__vector_size__(16))) long x;
};
void __vector_size__LongTest() {
VectorSizeLong v;
}
Since, according to my short research,
"The vector_size attribute is only applicable to integral and float scalars,
although arrays, pointers, and function return values are allowed in conjunction
with this construct."
[src: https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Vector-Extensions.html#Vector-Extensions]
vector types are safe to regard as primitive.
Differential Revision: https://reviews.llvm.org/D61246
llvm-svn: 359539
Don't crash when trying to model a call in which the callee is unknown
in compile time, eg. a pointer-to-member call.
Differential Revision: https://reviews.llvm.org/D61285
llvm-svn: 359530
This patch is more of a fix than a real improvement: in checkPostCall()
we should return immediately after finding the right call and handling
it. This both saves unnecessary processing and double-handling calls by
mistake.
Differential Revision: https://reviews.llvm.org/D61134
llvm-svn: 359283
Because RetainCountChecker has custom "local" reasoning about escapes,
it has a separate facility to deal with tracked symbols at end of analysis
and check them for leaks regardless of whether they're dead or not.
This facility iterates over the list of tracked symbols and reports
them as leaks, but it needs to treat the return value specially.
Some custom allocators tend to return the value with an offset, storing
extra metadata at the beginning of the buffer. In this case the return value
would be a non-base region. In order to avoid false positives, we still need to
find the original symbol within the return value, otherwise it'll be unable
to match it to the item in the list of tracked symbols.
Differential Revision: https://reviews.llvm.org/D60991
llvm-svn: 359263
https://bugs.llvm.org/show_bug.cgi?id=41590
For the following code snippet, UninitializedObjectChecker crashed:
struct MyAtomicInt {
_Atomic(int) x;
MyAtomicInt() {}
};
void entry() {
MyAtomicInt b;
}
The problem was that _Atomic types were not regular records, unions,
dereferencable or primitive, making the checker hit the llvm_unreachable at
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:347.
The solution is to regard these types as primitive as well. The test case shows
that with this addition, not only are we able to get rid of the crash, but we
can identify x as uninitialized.
Differential Revision: https://reviews.llvm.org/D61106
llvm-svn: 359230
A compilation warning was in my previous commit which broke the buildbot
because it is using `-Werror` for compilation. This patch fixes this
issue.
llvm-svn: 358955
Currently iterator checkers record comparison of iterator positions
and process them for keeping track the distance between them (e.g.
whether a position is the same as the end position). However this
makes some processing unnecessarily complex and it is not needed at
all: we only need to keep track between the abstract symbols stored
in these iterator positions. This patch changes this and opens the
path to comparisons to the begin() and end() symbols between the
container (e.g. size, emptiness) which are stored as symbols, not
iterator positions. The functionality of the checker is unchanged.
Differential Revision: https://reviews.llvm.org/D53701
llvm-svn: 358951
Implement cplusplus.SmartPtrModeling, a new checker that doesn't
emit any warnings but models methods of smart pointers more precisely.
For now the only thing it does is make `(bool) P` return false when `P`
is a freshly moved pointer. This addresses a false positive in the
use-after-move-checker.
Differential Revision: https://reviews.llvm.org/D60796
llvm-svn: 358944
Moved UninitializedObjectChecker from the 'alpha.cplusplus' to the
'optin.cplusplus' package.
Differential Revision: https://reviews.llvm.org/D58573
llvm-svn: 358797
For the following code snippet:
void builtin_function_call_crash_fixes(char *c) {
__builtin_strncpy(c, "", 6);
__builtin_memset(c, '\0', (0));
__builtin_memcpy(c, c, 0);
}
security.insecureAPI.DeprecatedOrUnsafeBufferHandling caused a regression, as it
didn't recognize functions starting with __builtin_. Fixed exactly that.
I wanted to modify an existing test file, but the two I found didn't seem like
perfect candidates. While I was there, I prettified their RUN: lines.
Differential Revision: https://reviews.llvm.org/D59812
llvm-svn: 358609
Requires making the llvm::MemoryBuffer* stored by SourceManager const,
which in turn requires making the accessors for that return const
llvm::MemoryBuffer*s and updating all call sites.
The original motivation for this was to use it and fix the TODO in
CodeGenAction.cpp's ConvertBackendLocation() by using the UnownedTag
version of createFileID, and since llvm::SourceMgr* hands out a const
llvm::MemoryBuffer* this is required. I'm not sure if fixing the TODO
this way actually works, but this seems like a good change on its own
anyways.
No intended behavior change.
Differential Revision: https://reviews.llvm.org/D60247
llvm-svn: 357724
__builtin_constant_p(x) is a compiler builtin that evaluates to 1 when
its argument x is a compile-time constant and to 0 otherwise. In CodeGen
it is simply lowered to the respective LLVM intrinsic. In the Analyzer
we've been trying to delegate modeling to Expr::EvaluateAsInt, which is
allowed to sometimes fail for no apparent reason.
When it fails, let's conservatively return false. Modeling it as false
is pretty much never wrong, and it is only required to return true
on a best-effort basis, which every user should expect.
Fixes VLAChecker false positives on code that tries to emulate
static asserts in C by constructing a VLA of dynamic size -1 under the
assumption that this dynamic size is actually a constant
in the sense of __builtin_constant_p.
Differential Revision: https://reviews.llvm.org/D60110
llvm-svn: 357557
At least gcc 7.4 complained with
../tools/clang/lib/StaticAnalyzer/Checkers/Taint.cpp:26:53: warning: extra ';' [-Wpedantic]
TaintTagType);
^
llvm-svn: 357461
It is now an inter-checker communication API, similar to the one that
connects MallocChecker/CStringChecker/InnerPointerChecker: simply a set of
setters and getters for a state trait.
Differential Revision: https://reviews.llvm.org/D59861
llvm-svn: 357326
Almost all path-sensitive checkers need to tell the user when something specific
to that checker happens along the execution path but does not constitute a bug
on its own. For instance, a call to operator delete in C++ has consequences
that are specific to a use-after-free bug. Deleting an object is not a bug
on its own, but when the Analyzer finds an execution path on which a deleted
object is used, it'll have to explain to the user when exactly during that path
did the deallocation take place.
Historically such custom notes were added by implementing "bug report visitors".
These visitors were post-processing bug reports by visiting every ExplodedNode
along the path and emitting path notes whenever they noticed that a change that
is relevant to a bug report occurs within the program state. For example,
it emits a "memory is deallocated" note when it notices that a pointer changes
its state from "allocated" to "deleted".
The "visitor" approach is powerful and efficient but hard to use because
such preprocessing implies that the developer first models the effects
of the event (say, changes the pointer's state from "allocated" to "deleted"
as part of operator delete()'s transfer function) and then forgets what happened
and later tries to reverse-engineer itself and figure out what did it do
by looking at the report.
The proposed approach tries to avoid discarding the information that was
available when the transfer function was evaluated. Instead, it allows the
developer to capture all the necessary information into a closure that
will be automatically invoked later in order to produce the actual note.
This should reduce boilerplate and avoid very painful logic duplication.
On the technical side, the closure is a lambda that's put into a special kind of
a program point tag, and a special bug report visitor visits all nodes in the
report and invokes all note-producing closures it finds along the path.
For now it is up to the lambda to make sure that the note is actually relevant
to the report. For instance, a memory deallocation note would be irrelevant when
we're reporting a division by zero bug or if we're reporting a use-after-free
of a different, unrelated chunk of memory. The lambda can figure these thing out
by looking at the bug report object that's passed into it.
A single checker is refactored to make use of the new functionality: MIGChecker.
Its program state is trivial, making it an easy testing ground for the first
version of the API.
Differential Revision: https://reviews.llvm.org/D58367
llvm-svn: 357323
For a rather short code snippet, if debug.ReportStmts (added in this patch) was
enabled, a bug reporter visitor crashed:
struct h {
operator int();
};
int k() {
return h();
}
Ultimately, this originated from PathDiagnosticLocation::createMemberLoc, as it
didn't handle the case where it's MemberExpr typed parameter returned and
invalid SourceLocation for MemberExpr::getMemberLoc. The solution was to find
any related valid SourceLocaion, and Stmt::getBeginLoc happens to be just that.
Differential Revision: https://reviews.llvm.org/D58777
llvm-svn: 356161
Checking whether two regions are the same is a partially decidable problem:
either we know for sure that they are the same or we cannot decide. A typical
case for this are the symbolic regions based on conjured symbols. Two
different conjured symbols are either the same or they are different. Since
we cannot decide this and want to reduce false positives as much as possible
we exclude these regions whenever checking whether two containers are the
same at iterator mismatch check.
Differential Revision: https://reviews.llvm.org/D53754
llvm-svn: 356049
Asserting on invalid input isn't very nice, hence the patch to emit an error
instead.
This is the first of many patches to overhaul the way we handle checker options.
Differential Revision: https://reviews.llvm.org/D57850
llvm-svn: 355704
In D55734, we implemented a far more general way of describing taint propagation
rules for functions, like being able to specify an unlimited amount of
source and destination parameters. Previously, we didn't have a particularly
elegant way of expressing the propagation rules for functions that always return
(either through an out-param or return value) a tainted value. In this patch,
we model these functions similarly to other ones, by assigning them a
TaintPropagationRule that describes that they "create a tainted value out of
nothing".
The socket C function is somewhat special, because for certain parameters (for
example, if we supply localhost as parameter), none of the out-params should
be tainted. For this, we added a general solution of being able to specify
custom taint propagation rules through function pointers.
Patch by Gábor Borsik!
Differential Revision: https://reviews.llvm.org/D59055
llvm-svn: 355703
The gets function has no SrcArgs. Because the default value for isTainted was
false, it didn't mark its DstArgs as tainted.
Patch by Gábor Borsik!
Differential Revision: https://reviews.llvm.org/D58828
llvm-svn: 355396
Under the term "subchecker", I mean checkers that do not have a checker class on
their own, like unix.MallocChecker to unix.DynamicMemoryModeling.
Since a checker object was required in order to retrieve checker options,
subcheckers couldn't possess options on their own.
This patch is also an excuse to change the argument order of getChecker*Option,
it always bothered me, now it resembles the actual command line argument
(checkername:option=value).
Differential Revision: https://reviews.llvm.org/D57579
llvm-svn: 355297
Add more "consuming" functions. For now only vm_deallocate() was supported.
Add a non-zero value that isn't an error; this value is -305 ("MIG_NO_REPLY")
and it's fine to deallocate data when you are returning this error.
Make sure that the mig_server_routine annotation is inherited.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D58397
llvm-svn: 354643
When a MIG server routine argument is released in an automatic destructor,
the Static Analyzer thinks that this happens after the return statement, and so
the violation of the MIG convention doesn't happen.
Of course, it doesn't quite work that way, so this is a false negative.
Add a hack that makes the checker double-check at the end of function
that no argument was released when the routine fails with an error.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D58392
llvm-svn: 354642
Add a BugReporterVisitor for highlighting the events of deallocating a
parameter. All such events are relevant to the emitted report (as long as the
report is indeed emitted), so all of them will get highlighted.
Add a trackExpressionValue visitor for highlighting where does the error return
code come from.
Do not add a trackExpressionValue visitor for highlighting how the deallocated
argument(s) was(were) copied around. This still remains to be implemented.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D58368
llvm-svn: 354641
r354530 has added a new function/block/message attribute "mig_server_routine"
that attracts compiler's attention to functions that need to follow the MIG
server routine convention with respect to deallocating out-of-line data that
was passed to them as an argument.
Teach the checker to identify MIG routines by looking at this attribute,
rather than by making heuristic-based guesses.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/58366
llvm-svn: 354638
This checker detects use-after-free bugs in (various forks of) the Mach kernel
that are caused by errors in MIG server routines - functions called remotely by
MIG clients. The MIG convention forces the server to only deallocate objects
it receives from the client when the routine is executed successfully.
Otherwise, if the server routine exits with an error, the client assumes that
it needs to deallocate the out-of-line data it passed to the server manually.
This means that deallocating such data within the MIG routine and then returning
a non-zero error code is always a dangerous use-after-free bug.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D57558
llvm-svn: 354635
There are certain unsafe or deprecated (since C11) buffer handling
functions which should be avoided in safety critical code. They
could cause buffer overflows. A new checker,
'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' warns for
every occurrence of such functions (unsafe or deprecated printf,
scanf family, and other buffer handling functions, which now have
a secure variant).
Patch by Dániel Kolozsvári!
Differential Revision: https://reviews.llvm.org/D35068
llvm-svn: 353698
oth strlcat and strlcpy cut off their safe bound for the argument value
at sizeof(destination). There's no need to subtract 1 in only one
of these cases.
Differential Revision: https://reviews.llvm.org/D57981
rdar://problem/47873212
llvm-svn: 353583
This patch is an implementation of the ideas discussed on the mailing list[1].
The idea is to somewhat heuristically guess whether the field that was confirmed
to be uninitialized is actually guarded with ifs, asserts, switch/cases and so
on. Since this is a syntactic check, it is very much prone to drastically
reduce the amount of reports the checker emits. The reports however that do not
get filtered out though have greater likelihood of them manifesting into actual
runtime errors.
[1] http://lists.llvm.org/pipermail/cfe-dev/2018-September/059255.html
Differential Revision: https://reviews.llvm.org/D51866
llvm-svn: 352959
Having an incorrect type for a cast causes the checker to incorrectly
dismiss the operation under ARC, leading to a false positive
use-after-release on the test.
rdar://47709885
Differential Revision: https://reviews.llvm.org/D57557
llvm-svn: 352824
This builtin has the same UI as __builtin_object_size, but has the
potential to be evaluated dynamically. It is meant to be used as a
drop-in replacement for libraries that use __builtin_object_size when
a dynamic checking mode is enabled. For instance,
__builtin_object_size fails to provide any extra checking in the
following function:
void f(size_t alloc) {
char* p = malloc(alloc);
strcpy(p, "foobar"); // expands to __builtin___strcpy_chk(p, "foobar", __builtin_object_size(p, 0))
}
This is an overflow if alloc < 7, but because LLVM can't fold the
object size intrinsic statically, it folds __builtin_object_size to
-1. With __builtin_dynamic_object_size, alloc is passed through to
__builtin___strcpy_chk.
rdar://32212419
Differential revision: https://reviews.llvm.org/D56760
llvm-svn: 352665
Provide a more powerful and at the same time more readable way of specifying
taint propagation rules for known functions within the checker.
Now it should be possible to specify an unlimited amount of source and
destination parameters for taint propagation.
No functional change intended just yet.
Patch by Gábor Borsik!
Differential Revision: https://reviews.llvm.org/D55734
llvm-svn: 352572
Track them for ISL/OS objects by default, and for NS/CF under a flag.
rdar://47536377
Differential Revision: https://reviews.llvm.org/D57356
llvm-svn: 352534
That weakens inner invariants, but allows the class to be more generic,
allowing usage in situations where the call expression is not known (or
should not matter).
Differential Revision: https://reviews.llvm.org/D57344
llvm-svn: 352531
This patch effectively fixes the almost decade old checker naming issue.
The solution is to assert when CheckerManager::getChecker is called on an
unregistered checker, and assert when CheckerManager::registerChecker is called
on a checker that is already registered.
Differential Revision: https://reviews.llvm.org/D55429
llvm-svn: 352292
Unfortunately, up until now, the fact that certain checkers depended on one
another was known, but how these actually unfolded was hidden deep within the
implementation. For example, many checkers (like RetainCount, Malloc or CString)
modelled a certain functionality, and exposed certain reportable bug types to
the user. For example, while MallocChecker models many many different types of
memory handling, the actual "unix.MallocChecker" checker the user was exposed to
was merely and option to this modeling part.
Other than this being an ugly mess, this issue made resolving the checker naming
issue almost impossible. (The checker naming issue being that if a checker
registered more than one checker within its registry function, both checker
object recieved the same name) Also, if the user explicitly disabled a checker
that was a dependency of another that _was_ explicitly enabled, it implicitly,
without "telling" the user, reenabled it.
Clearly, changing this to a well structured, declarative form, where the
handling of dependencies are done on a higher level is very much preferred.
This patch, among the detailed things later, makes checkers declare their
dependencies within the TableGen file Checkers.td, and exposes the same
functionality to plugins and statically linked non-generated checkers through
CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies,
makes sure that checkers are added to CheckerManager in the correct order,
and makes sure that if a dependency is disabled, so will be every checker that
depends on it.
In detail:
* Add a new field to the Checker class in CheckerBase.td called Dependencies,
which is a list of Checkers.
* Move unix checkers before cplusplus, as there is no forward declaration in
tblgen :/
* Add the following new checkers:
- StackAddrEscapeBase
- StackAddrEscapeBase
- CStringModeling
- DynamicMemoryModeling (base of the MallocChecker family)
- IteratorModeling (base of the IteratorChecker family)
- ValistBase
- SecuritySyntaxChecker (base of bcmp, bcopy, etc...)
- NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker)
- IvarInvalidationModeling (base of IvarInvalidation checker family)
- RetainCountBase (base of RetainCount and OSObjectRetainCount)
* Clear up and registry functions in MallocChecker, happily remove old FIXMEs.
* Add a new addDependency function to CheckerRegistry.
* Neatly format RUN lines in files I looked at while debugging.
Big thanks to Artem Degrachev for all the guidance through this project!
Differential Revision: https://reviews.llvm.org/D54438
llvm-svn: 352287
The actual implementation of unix.API features a dual-checker: two checkers in
one, even though they don't even interact at all. Split them up, as this is a
problem for establishing dependencies.
I added no new code at all, just merely moved it around.
Since the plist files change (and that's a benefit!) this patch isn't NFC.
Differential Revision: https://reviews.llvm.org/D55425
llvm-svn: 352278
Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions &LO)
function very similarly to ento::register##CHECKERNAME. This will force every
checker to implement this function, but maybe it isn't that bad: I saw a lot of
ObjC or C++ specific checkers that should probably not register themselves based
on some LangOptions (mine too), but they do anyways.
A big benefit of this is that all registry functions now register their checker,
once it is called, registration is guaranteed.
This patch is a part of a greater effort to reinvent checker registration, more
info here: D54438#1315953
Differential Revision: https://reviews.llvm.org/D55424
llvm-svn: 352277
Adam Balogh