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
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
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
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
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
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 ®istry);
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
Especially with pointees, a lot of meaningless reports came from uninitialized
regions that were already reported. This is fixed by storing all reported fields
to the GDM.
Differential Revision: https://reviews.llvm.org/D51531
llvm-svn: 347153
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
As rightly pointed out by @NoQ, nonloc::LazyCompoundVals were only used to acquire a constructed object's region, which isn't what LazyCompoundVal was made for.
Differential Revision: https://reviews.llvm.org/D51300
llvm-svn: 344879
I've added a new functionality, the checker is now able to
detect and report fields pointing to themselves. I figured
this would fit well into the checker as there's no reason
for a pointer to point to itself instead of being nullptr.
Differential Revision: https://reviews.llvm.org/D51305
llvm-svn: 344242
Since I plan to add a number of new flags, it made sense to encapsulate
them in a new struct, in order not to pollute FindUninitializedFields's
constructor with new boolean options with super long names.
This revision practically reverts D50508, since FindUninitializedFields
now accesses the pedantic flag anyways.
Differential Revision: https://reviews.llvm.org/D51679
llvm-svn: 342219
Some of the comments are incorrect, imprecise, or simply nonexistent.
Since I have a better grasp on how the analyzer works, it makes sense
to update most of them in a single swoop.
I tried not to flood the code with comments too much, this amount
feels just right to me.
Differential Revision: https://reviews.llvm.org/D51417
llvm-svn: 342215
iThis patch aims to fix derefencing, which has been debated for months now.
Instead of working with SVals, the function now relies on TypedValueRegion.
Differential Revision: https://reviews.llvm.org/D51057
llvm-svn: 342213
For the following example:
struct Base {
int x;
};
// In a different translation unit
struct Derived : public Base {
Derived() {}
};
For a call to Derived::Derived(), we'll receive a note that
this->x is uninitialized. Since x is not a direct field of Derived,
it could be a little confusing. This patch aims to fix this, as well
as the case when the derived object has a field that has the name as
an inherited uninitialized data member:
struct Base {
int x; // note: uninitialized field 'this->Base::x'
};
struct Derived : public Base {
int x = 5;
Derived() {}
};
Differential Revision: https://reviews.llvm.org/D50905
llvm-svn: 340272
Now that it has it's own file, it makes little sense for
isPointerOrReferenceUninit to be this large, so I moved
dereferencing to a separate function.
Differential Revision: https://reviews.llvm.org/D50509
llvm-svn: 340265
Before this patch, FieldChainInfo used a spaghetti: it took care of way too many cases,
even though it was always meant as a lightweight wrapper around
ImmutableList<const FieldRegion *>.
This problem is solved by introducing a lightweight polymorphic wrapper around const
FieldRegion *, FieldNode. It is an interface that abstracts away special cases like
pointers/references, objects that need to be casted to another type for a proper note
messages.
Changes to FieldChainInfo:
* Now wraps ImmutableList<const FieldNode &>.
* Any pointer/reference related fields and methods were removed
* Got a new add method. This replaces it's former constructors as a way to create a
new FieldChainInfo objects with a new element.
Changes to FindUninitializedField:
* In order not to deal with dynamic memory management, when an uninitialized field is
found, the note message for it is constructed and is stored instead of a
FieldChainInfo object. (see doc around addFieldToUninits).
Some of the test files are changed too, from now on uninitialized pointees of references
always print "uninitialized pointee" instead of "uninitialized field" (which should've
really been like this from the beginning).
I also updated every comment according to these changes.
Differential Revision: https://reviews.llvm.org/D50506
llvm-svn: 339599
In this patch, the following classes and functions have been moved to a header file:
FieldChainInfo
FindUninitializedFields
isPrimitiveType
This also meant that they moved from anonymous namespace to clang::ento.
Code related to pointer chasing now relies in its own file.
There's absolutely no functional change in this patch -- its literally just copy pasting.
Differential Revision: https://reviews.llvm.org/D50504
llvm-svn: 339595
Kristof Umann