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
Artem Dergachev