| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In CallAndMessageChecker the initialization of bug types was highly
obfuscated (even compared to other `mutable std::unique_ptr` hacks).
This commit cleans up this situation and removes a totally superfluous
hidded 'modeling' sub-checker that did not have any role apart from
obstructing the normal initialization of bug types.
(Note that if we need to reintroduce CallAndMessageModeling in the
future, we can do it cleanly within the CheckerFamily framework, so we
wouldn't need to re-obfuscate the bug type initialization.)
This change is mostly non-functional, the only visible change is the
removal of the hidden modeling checker.
|
|
This changes a bunch of places which use getAs<TagType>, including
derived types, just to obtain the tag definition.
This is preparation for #155028, offloading all the changes that PR used
to introduce which don't depend on any new helpers.
|
|
This is a major change on how we represent nested name qualifications in
the AST.
* The nested name specifier itself and how it's stored is changed. The
prefixes for types are handled within the type hierarchy, which makes
canonicalization for them super cheap, no memory allocation required.
Also translating a type into nested name specifier form becomes a no-op.
An identifier is stored as a DependentNameType. The nested name
specifier gains a lightweight handle class, to be used instead of
passing around pointers, which is similar to what is implemented for
TemplateName. There is still one free bit available, and this handle can
be used within a PointerUnion and PointerIntPair, which should keep
bit-packing aficionados happy.
* The ElaboratedType node is removed, all type nodes in which it could
previously apply to can now store the elaborated keyword and name
qualifier, tail allocating when present.
* TagTypes can now point to the exact declaration found when producing
these, as opposed to the previous situation of there only existing one
TagType per entity. This increases the amount of type sugar retained,
and can have several applications, for example in tracking module
ownership, and other tools which care about source file origins, such as
IWYU. These TagTypes are lazily allocated, in order to limit the
increase in AST size.
This patch offers a great performance benefit.
It greatly improves compilation time for
[stdexec](https://github.com/NVIDIA/stdexec). For one datapoint, for
`test_on2.cpp` in that project, which is the slowest compiling test,
this patch improves `-c` compilation time by about 7.2%, with the
`-fsyntax-only` improvement being at ~12%.
This has great results on compile-time-tracker as well:
This patch also further enables other optimziations in the future, and
will reduce the performance impact of template specialization resugaring
when that lands.
It has some other miscelaneous drive-by fixes.
About the review: Yes the patch is huge, sorry about that. Part of the
reason is that I started by the nested name specifier part, before the
ElaboratedType part, but that had a huge performance downside, as
ElaboratedType is a big performance hog. I didn't have the steam to go
back and change the patch after the fact.
There is also a lot of internal API changes, and it made sense to remove
ElaboratedType in one go, versus removing it from one type at a time, as
that would present much more churn to the users. Also, the nested name
specifier having a different API avoids missing changes related to how
prefixes work now, which could make existing code compile but not work.
How to review: The important changes are all in
`clang/include/clang/AST` and `clang/lib/AST`, with also important
changes in `clang/lib/Sema/TreeTransform.h`.
The rest and bulk of the changes are mostly consequences of the changes
in API.
PS: TagType::getDecl is renamed to `getOriginalDecl` in this patch, just
for easier to rebasing. I plan to rename it back after this lands.
Fixes #136624
Fixes https://github.com/llvm/llvm-project/issues/43179
Fixes https://github.com/llvm/llvm-project/issues/68670
Fixes https://github.com/llvm/llvm-project/issues/92757
|
|
unnamed bit-field (#145066)
For the following code in C mode: https://godbolt.org/z/3eo1MeGhe
(There is no warning in C++ mode though).
```c++
struct B {
int i : 2;
int : 30; // unnamed bit-field
};
extern void consume_B(struct B);
void bitfield_B_init(void) {
struct B b1;
b1.i = 1; // b1 is initialized
consume_B(b1); // FP: Passed-by-value struct argument contains uninitialized data (e.g., field: '') [core.CallAndMessage]
}
```
|
|
Previously some checkers attached explicitly created program point tags
to some of the exploded graph nodes that they created. In most of the
checkers this ad-hoc tagging only affected the debug dump of the
exploded graph (and they weren't too relevant for debugging) so this
commit removes them.
There were two checkers where the tagging _did_ have a functional role:
- In `RetainCountChecker` the presence of tags were checked by
`RefCountReportVisitor`.
- In `DynamicTypePropagation` the checker sometimes wanted to create two
identical nodes and had to apply an explicit tag on the second one to
avoid "caching out".
In these two situations I preserved the tags but switched to using
`SimpleProgramPointTag` instead of `CheckerProgramPointTag` because
`CheckerProgramPointTag` didn't provide enough benefits to justify its
existence.
Note that this commit depends on the earlier commit "[analyzer] Fix
tagging of PostAllocatorCall" ec96c0c072ef3f78813c378949c00e1c07aa44e5
and would introduce crashes when cherry-picked onto a branch that
doesn't contain that commit.
For more details about the background see the discourse thread
https://discourse.llvm.org/t/role-of-programpointtag-in-the-static-analyzer/
As a tangentially related changes, this commit also adds some comments
to document the surprising behavior of `CheckerContext::addTransition`
and an assertion in the constructor of `PathSensitiveBugReport` to get a
more readable crash dump in the case when the report is constructed with
`nullptr` as the `ErrorNode`. (This can happen due to "caching out".)
|
|
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
|
|
|
|
...because it provides no useful functionality compared to its base
class `BugType`.
A long time ago there were substantial differences between `BugType` and
`BuiltinBug`, but they were eliminated by commit 1bd58233 in 2009 (!).
Since then the only functionality provided by `BuiltinBug` was that it
specified `categories::LogicError` as the bug category and it stored an
extra data member `desc`.
This commit sets `categories::LogicError` as the default value of the
third argument (bug category) in the constructors of BugType and
replaces use of the `desc` field with simpler logic.
Note that `BugType` has a data member `Description` and a non-virtual
method `BugType::getDescription()` which queries it; these are distinct
from the member `desc` of `BuiltinBug` and the identically named method
`BuiltinBug::getDescription()` which queries it. This confusing name
collision was a major motivation for the elimination of `BuiltinBug`.
As this commit touches many files, I avoided functional changes and left
behind FIXME notes to mark minor issues that should be fixed later.
Differential Revision: https://reviews.llvm.org/D158855
|
|
A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up some of the
instances where bool was used, and could be changed to DefaultBool.
Reviewed By: steakhal
Differential Revision: https://reviews.llvm.org/D123464
|
|
iAs listed in the summary D77846, we have 5 different categories of bugs we're
checking for in CallAndMessage. I think the documentation placed in the code
explains my thought process behind my decisions quite well.
A non-obvious change I had here is removing the entry for
CallAndMessageUnInitRefArg. In fact, I removed the CheckerNameRef typed field
back in D77845 (it was dead code), so that checker didn't really exist in any
meaningful way anyways.
Differential Revision: https://reviews.llvm.org/D77866
|
|
The patch aims to use CallEvents interface in a more principled manner, and also
to highlight what this checker really does. It in fact checks for 5 different
kinds of errors (from checkPreCall, that is):
* Invalid function pointer related errors
* Call of methods from an invalid C++ this object
* Function calls with incorrect amount of parameters
* Invalid arguments for operator delete
* Pass of uninitialized values to pass-by-value parameters
In a previous patch I complained that this checker is responsible for emitting
a lot of different diagnostics all under core.CallAndMessage's name, and this
patch shows where we could start to assign different diagnostics to different
entities.
Differential Revision: https://reviews.llvm.org/D77846
|
|
The following series of patches has something similar in mind with D77474, with
the same goal to finally end incorrect checker names for good. Despite
CallAndMessage not suffering from this particular issue, it is a dependency for
many other checkers, which is problematic, because we don't really want
dependencies to also emit diagnostics (reasoning for this is also more detailed
in D77474).
CallAndMessage also has another problem, namely that it is responsible for a lot
of reports. You'll soon learn that this isn't really easy to solve for
compatibility reasons, but that is the topic of followup patches.
Differential Revision: https://reviews.llvm.org/D77845
|
|
functions
Some checkers may not only depend on language options but also analyzer options.
To make this possible this patch changes the parameter of the shouldRegister*
function to CheckerManager to be able to query the analyzer options when
deciding whether the checker should be registered.
Differential Revision: https://reviews.llvm.org/D75271
|
|
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
|
|
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
|
|
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
|
|
function registers no more than 1 checker
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
|
|
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
|
|
trackNullOrUndefValue is a long and confusing name,
and it does not actually reflect what the function is doing.
Give a function a new name, with a relatively clear semantics.
Also remove some dead code.
Differential Revision: https://reviews.llvm.org/D52758
llvm-svn: 345064
|
|
llvm-svn: 344468
|
|
rdar://13729267
Differential Revision: https://reviews.llvm.org/D51323
llvm-svn: 340986
|
|
Differential Revision: https://reviews.llvm.org/D51322
llvm-svn: 340964
|
|
This is a follow-up from r314910. When a checker developer attempts to
dereference a location in memory through ProgramState::getSVal(Loc) or
ProgramState::getSVal(const MemRegion *), without specifying the second
optional QualType parameter for the type of the value he tries to find at this
location, the type is auto-detected from location type. If the location
represents a value beyond a void pointer, we thought that auto-detecting the
type as 'char' is a good idea. However, in most practical cases, the correct
behavior would be to specify the type explicitly, as it is available from other
sources, and the few cases where we actually need to take a 'char' are
workarounds rather than an intended behavior. Therefore, try to fail with an
easy-to-understand assertion when asked to read from a void pointer location.
Differential Revision: https://reviews.llvm.org/D38801
llvm-svn: 320451
|
|
Differential Revision: https://reviews.llvm.org/D30341
llvm-svn: 297283
|
|
The CallAndMessageChecker has an existing check for when a function pointer
is called with too few arguments. Extend this logic to handle the block
case, as well. While we're at it, do a drive-by grammar correction
("less" --> "fewer") on the diagnostic text.
llvm-svn: 287001
|
|
Differential Revision: http://reviews.llvm.org/D18363
llvm-svn: 264164
|
|
This patch should add support for almost all command-line options and
driver tinkering necessary to produce a correct "clang -cc1"
invocation for watchOS and tvOS.
llvm-svn: 251706
|
|
llvm-svn: 249259
|
|
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
|
|
In Objective-C, method calls with nil receivers are essentially no-ops. They
do not fault (although the returned value may be garbage depending on the
declared return type and architecture). Programmers are aware of this
behavior and will complain about a false alarm when the analyzer
diagnoses API violations for method calls when the receiver is known to
be nil.
Rather than require each individual checker to be aware of this behavior
and suppress a warning when the receiver is nil, this commit
changes ExprEngineObjC so that VisitObjCMessage skips calling checker
pre/post handlers when the receiver is definitely nil. Instead, it adds a
new event, ObjCMessageNil, that is only called in that case.
The CallAndMessageChecker explicitly cares about this case, so I've changed it
to add a callback for ObjCMessageNil and moved the logic in PreObjCMessage
that handles nil receivers to the new callback.
rdar://problem/18092611
Differential Revision: http://reviews.llvm.org/D12123
llvm-svn: 247653
|
|
to the caller instead of hiding it in emitReport. NFC.
llvm-svn: 240400
|
|
llvm-svn: 209642
|
|
Passing a pointer to an uninitialized memory buffer is normally okay,
but if the function is declared to take a pointer-to-const then it's
very unlikely it will be modifying the buffer. In this case the analyzer
should warn that there will likely be a read of uninitialized memory.
This doesn't check all elements of an array, only the first one.
It also doesn't yet check Objective-C methods, only C functions and
C++ methods.
This is controlled by a new check: alpha.core.CallAndMessageUnInitRefArg.
Patch by Per Viberg!
llvm-svn: 203822
|
|
iterator_range fields(). Updating all of the usages of the iterators with range-based for loops.
llvm-svn: 203355
|
|
This compiles cleanly with lldb/lld/clang-tools-extra/llvm.
llvm-svn: 203279
|
|
llvm-svn: 202639
|
|
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
|
|
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
|
|
raw_ostream, and started using it in places it made sense.
No functional changes intended, just API cleanliness.
llvm-svn: 198428
|
|
Also add some tests that there is actually a message and that the bug is
actually a hard error. This actually behaved correctly before, because:
- addTransition() doesn't actually add a transition if the new state is null;
it assumes you want to propagate the predecessor forward and does nothing.
- generateSink() is called in order to emit a bug report.
- If at least one new node has been generated, the predecessor node is /not/
propagated forward.
But now it's spelled out explicitly.
Found by Richard Mazorodze, who's working on a patch that may require this.
llvm-svn: 191805
|
|
llvm-svn: 190737
|
|
Patch by Karthik Bhat, modified slightly by me.
llvm-svn: 188043
|
|
Summary:
When processing a call to a function, which got passed less arguments than it
expects, the analyzer would crash.
I've also added a test for that and a analyzer warning which detects these
cases.
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D994
llvm-svn: 184288
|
|
This also allows us to ensure IDC/return null suppression gets triggered in such cases.
llvm-svn: 178686
|
|
reclaimed
The visitor should look for the PreStmt node as the receiver is nil in the PreStmt and this is the node. Also, tag the nil
receiver nodes with a special tag for consistency.
llvm-svn: 178152
|
|
Post-commit CR feedback from Jordan Rose regarding r175594.
llvm-svn: 175679
|
|
See r175462 for another example/more details.
llvm-svn: 175594
|
