| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This patch replaces SmallSet<T *, N> with SmallPtrSet<T *, N>. Note
that SmallSet.h "redirects" SmallSet to SmallPtrSet for pointer
element types:
template <typename PointeeType, unsigned N>
class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N>
{};
We only have 30 instances that rely on this "redirection", with about
half of them under clang/. Since the redirection doesn't improve
readability, this patch replaces SmallSet with SmallPtrSet for pointer
element types.
I'm planning to remove the redirection eventually.
|
|
SourceLocation (#145711)
Introduce a type alias for the commonly used `std::pair<FileID,
unsigned>` to improve code readability, and make it easier for future
updates (64-bit source locations).
|
|
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".)
|
|
|
|
DynamicRecursiveASTVisitor (#115144)
This pr refactors all recursive AST visitors in `Sema`, `Analyze`, and
`StaticAnalysis` to inherit from DRAV instead. This is over half of the
visitors that inherit from RAV directly.
See also #115132, #110040, #93462
LLVM Compile-Time Tracker link for this branch:
https://llvm-compile-time-tracker.com/compare.php?from=5adb5c05a2e9f31385fbba8b0436cbc07d91a44d&to=b58e589a86c06ba28d4d90613864d10be29aa5ba&stat=instructions%3Au
|
|
|
|
I'm planning to remove StringRef::equals in favor of
StringRef::operator==.
- StringRef::operator==/!= outnumber StringRef::equals by a factor of
24 under clang/ in terms of their usage.
- The elimination of StringRef::equals brings StringRef closer to
std::string_view, which has operator== but not equals.
- S == "foo" is more readable than S.equals("foo"), especially for
!Long.Expression.equals("str") vs Long.Expression != "str".
|
|
The IdentifierInfo isn't typically modified. Use 'const' wherever
possible.
|
|
|
|
Cleanup most of the lazy-init `BugType` legacy.
Some will be preserved, as those are slightly more complicated to
refactor.
Notice, that the default category for `BugType` is `LogicError`. I
omitted setting this explicitly where I could.
Please, actually have a look at the diff. I did this manually, and we
rarely check the bug type descriptions and stuff in tests, so the
testing might be shallow on this one.
|
|
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
|
|
I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradually
move forward.
Differential Revision: https://reviews.llvm.org/D154325
|
|
This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.
I'll post a separate patch to actually replace llvm::Optional with
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
|
|
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
|
|
Since https://reviews.llvm.org/D120334 we shouldn't pass temporary LangOptions to Lexer.
This change fixes stack-use-after-scope UB in LocalizationChecker found by sanitizer-x86_64-linux-fast buildbot
and resolve similar issue in HeaderIncludes.
|
|
|
|
Modify the IfStmt node to suppoort constant evaluated expressions.
Add a new ExpressionEvaluationContext::ImmediateFunctionContext to
keep track of immediate function contexts.
This proved easier/better/probably more efficient than walking the AST
backward as it allows diagnosing nested if consteval statements.
|
|
Update clang/lib/StaticAnalyzer to stop relying on a `MemoryBuffer*`,
using the `MemoryBufferRef` from `getBufferOrNone` or the
`Optional<MemoryBufferRef>` from `getBufferOrFake`, depending on whether
there's logic for checking validity of the buffer. The change to
clang/lib/StaticAnalyzer/Core/IssueHash.cpp is potentially a
functionality change, since the logic was wrong (it checked for
`nullptr`, which was never returned by the old API), but if that was
reachable the new behaviour should be better.
Differential Revision: https://reviews.llvm.org/D89414
|
|
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
|
|
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.
|
|
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
|
|
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
|
|
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
|
|
std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef
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
|
|
llvm-svn: 365901
|
|
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
|
|
llvm::MemoryBuffer*
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
|
|
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
|
|
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
|
|
have to be registered
One of the reasons why AnalyzerOptions is so chaotic is that options can be
retrieved from the command line whenever and wherever. This allowed for some
options to be forgotten for a looooooong time. Have you ever heard of
"region-store-small-struct-limit"? In order to prevent this in the future, I'm
proposing to restrict AnalyzerOptions' interface so that only checker options
can be retrieved without special getters. I would like to make every option be
accessible only through a getter, but checkers from plugins are a thing, so I'll
have to figure something out for that.
This also forces developers who'd like to add a new option to register it
properly in the .def file.
This is done by
* making the third checker pointer parameter non-optional, and checked by an
assert to be non-null.
* I added new, but private non-checkers option initializers, meant only for
internal use,
* Renamed these methods accordingly (mind the consistent name for once with
getBooleanOption!):
- getOptionAsString -> getCheckerStringOption,
- getOptionAsInteger -> getCheckerIntegerOption
* The 3 functions meant for initializing data members (with the not very
descriptive getBooleanOption, getOptionAsString and getOptionAsUInt names)
were renamed to be overloads of the getAndInitOption function name.
* All options were in some way retrieved via getCheckerOption. I removed it, and
moved the logic to getStringOption and getCheckerStringOption. This did cause
some code duplication, but that's the only way I could do it, now that checker
and non-checker options are separated. Note that the non-checker version
inserts the new option to the ConfigTable with the default value, but the
checker version only attempts to find already existing entries. This is how
it always worked, but this is clunky and I might end reworking that too, so we
can eventually get a ConfigTable that contains the entire configuration of the
analyzer.
Differential Revision: https://reviews.llvm.org/D53483
llvm-svn: 346113
|
|
I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.
In this patch, I found some flags that should've been used as checker options,
or have absolutely no mention of in AnalyzerOptions, or are nonexistent.
- NonLocalizedStringChecker now uses its "AggressiveReport" flag as a checker
option
- lib/StaticAnalyzer/Frontend/ModelInjector.cpp now accesses the "model-path"
option through a getter in AnalyzerOptions
- -analyzer-config path-diagnostics-alternate=false is not a thing, I removed it,
- lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp and
lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h are weird, they actually
only contain an option getter. I deleted them, and fixed RetainCountChecker
to get it's "leak-diagnostics-reference-allocation" option as a checker option,
- "region-store-small-struct-limit" has a proper getter now.
Differential Revision: https://reviews.llvm.org/D53276
llvm-svn: 345985
|
|
Differential Revision: https://reviews.llvm.org/D52640
llvm-svn: 343353
|
|
In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the currently processed path
is destroyed, all diagnostics is discarded, and it is regenerated again,
until it's no longer modified.
This pattern has a few negative implications:
- This loop does not even guarantee to terminate.
E.g. just imagine two visitors bouncing a diagnostics around.
- Performance-wise, e.g. for sqlite3 all visitors are being re-run at
least 10 times for some bugs.
We have already seen a few reports where it leads to timeouts.
- If we want to add more computationally intense visitors, this will
become worse.
- From architectural standpoint, the current layout requires copying
visitors, which is conceptually wrong, and can be annoying (e.g. no
unique_ptr on visitors allowed).
The proposed change is a much simpler architecture: the outer loop
processes nodes upwards, and whenever the visitor is added it only
processes current nodes and above, thus guaranteeing termination.
Differential Revision: https://reviews.llvm.org/D47856
llvm-svn: 335666
|
|
Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:
archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
methode
nd
optin
ot
pres
statics
te
thru
Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few
files that have dubious fixes reverted.)
Differential revision: https://reviews.llvm.org/D44188
llvm-svn: 329399
|
|
helper is used consistently
In most cases using
`N->getState()->getSVal(E, N->getLocationContext())`
is ugly, verbose, and also opens up more surface area for bugs if an
inconsistent location context is used.
This patch introduces a helper on an exploded node, and ensures
consistent usage of either `ExplodedNode::getSVal` or
`CheckContext::getSVal` across the codebase.
As a result, a large number of redundant lines is removed.
Differential Revision: https://reviews.llvm.org/D42155
llvm-svn: 322753
|
|
There was already a returns_localized_nsstring annotation to indicate
that the return value could be passed to UIKit methods that would
display them. However, those UIKit methods were hard-coded, and it was
not possible to indicate that other classes/methods in a code-base would
do the same.
The takes_localized_nsstring annotation can be put on function
parameters and selector parameters to indicate that those will also show
the string to the user.
Differential Revision: https://reviews.llvm.org/D35186
llvm-svn: 308012
|
|
Add support for new methods that were added in macOS High Sierra & iOS 11
and require a localized string.
Patch by Kulpreet Chilana!
rdar://problem/32795210
Differential Revision: https://reviews.llvm.org/D34266
llvm-svn: 305896
|
|
Simplifies and makes explicit the memory ownership model rather than
implicitly passing/acquiring ownership.
llvm-svn: 291143
|
|
Fix a crash when relexing the underlying memory buffer to find incorrect
arguments to NSLocalizedString(). With precompiled headers, the raw
buffer may be NULL. Instead, use the source manager to get the buffer,
which will lazily create the buffer for precompiled headers.
rdar://problem/27429091
llvm-svn: 280174
|
|
Add new APIs that require localized strings and remove two APIs that were
incorrectly marked as requiring a user-facing string.
A patch by Kulpreet Chilana!
Differential Revision: https://reviews.llvm.org/D22926
llvm-svn: 277273
|
|
Summary: Removed unused headers, replaced some headers with forward class declarations
Patch by: Eugene <claprix@yandex.ru>
Differential Revision: https://reviews.llvm.org/D20100
llvm-svn: 275882
|
|
Add a path note indicating the location of the non-localized string
literal in NonLocalizedStringChecker.
rdar://problem/25981525
llvm-svn: 267924
|
|
r260925 introduced a version of the *trim methods which is preferable
when trimming a single kind of character. Update all users in clang.
llvm-svn: 260927
|
|
If the class or method name case-insensitively contains the term "debug",
suppress warnings about string constants flowing to user-facing UI APIs.
llvm-svn: 259875
|
|
This commit creates a new 'optin' top-level checker package and moves several of
the localizability checkers into it.
This package is for checkers that are not alpha and that would normally be on by
default but where the driver does not have enough information to determine when
they are applicable. The localizability checkers fit this criterion because the
driver cannot determine whether a project is localized or not -- this is best
determined at the IDE or build-system level.
This new package is *not* intended for checkers that are too noisy to be on by
default.
The hierarchy under 'optin' mirrors that in 'alpha': checkers under 'optin'
should be organized in the hierarchy they would have had if they were truly top
level (e.g., optin.osx.cocoa.MyOptInChecker).
Differential Revision: http://reviews.llvm.org/D14303
llvm-svn: 252080
|
|
Various improvements to the localization checker:
* Adjusted copy to be consistent with diagnostic text in other Apple
API checkers.
* Added in ~150 UIKit / AppKit methods that require localized strings in
UnlocalizedStringsChecker.
* UnlocalizedStringChecker now checks for UI methods up the class hierarchy and
UI methods that conform for a certain Objective-C protocol.
* Added in alpha version of PluralMisuseChecker and some regression tests. False
positives are still not ideal.
(This is the second attempt, with the memory issues on Linux resolved.)
A patch by Kulpreet Chilana!
Differential Revision: http://reviews.llvm.org/D12417
llvm-svn: 248432
|
