Pass EvalCallOptions via runCheckersForEvalCall into defaultEvalCall.
Update the AnalysisOrderChecker to support evalCall for testing.
Differential Revision: https://reviews.llvm.org/D82256
Summary:
As discussed previously when landing patch for OpenMP in Flang, the idea is
to share common part of the OpenMP declaration between the different Frontend.
While doing this it was thought that moving to tablegen instead of Macros will also
give a cleaner and more powerful way of generating these declaration.
This first part of a future series of patches is setting up the base .td file for
DirectiveLanguage as well as the OpenMP version of it. The base file is meant to
be used by other directive language such as OpenACC.
In this first patch, the Directive and Clause enums are generated with tablegen
instead of the macros on OMPConstants.h. The next pacth will extend this
to other enum and move the Flang frontend to use it.
Reviewers: jdoerfert, DavidTruby, fghanim, ABataev, jdenny, hfinkel, jhuber6, kiranchandramohan, kiranktp
Reviewed By: jdoerfert, jdenny
Subscribers: arphaman, martong, cfe-commits, mgorny, yaxunl, hiraditya, guansong, jfb, sstefan1, aaron.ballman, llvm-commits
Tags: #llvm, #openmp, #clang
Differential Revision: https://reviews.llvm.org/D81736
Summary:
Bug reports of resource leak are now improved.
If there are multiple resource leak paths for the same stream,
only one wil be reported.
Reviewers: Szelethus, xazax.hun, baloghadamsoftware, NoQ
Reviewed By: Szelethus, NoQ
Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D81407
https://bugs.llvm.org/show_bug.cgi?id=46253
This is an obvious hack because realloc isn't any more affected than other
functions modeled by MallocChecker (or any user of CallDescription really),
but the nice solution will take some time to implement.
Differential Revision: https://reviews.llvm.org/D81745
Summary:
EOF macro token coming from a PCH file on macOS while marked as literal,
doesn't contain any literal data. This causes crash on every project
using PCHs.
This commit doesn't resolve the problem with PCH (maybe it was
designed like this for a purpose) or with `tryExpandAsInteger`, but
rather simply shoots off a crash itself.
Differential Revision: https://reviews.llvm.org/D81916
Summary:
Implemented RangeConstraintManager::getRangeForComparisonSymbol which handles comparison operators.
RangeConstraintManager::getRangeForComparisonSymbol cares about the sanity of comparison expressions sequences helps reasonably to branch an exploded graph.
It can significantly reduce the graph and speed up the analysis. For more details, please, see the differential revision.
This fixes https://bugs.llvm.org/show_bug.cgi?id=13426
Differential Revision: https://reviews.llvm.org/D78933
Summary:
After an escaped FILE* stream handle it is not possible to make
reliable checks on it because any function call can have effect
on it.
Reviewers: Szelethus, baloghadamsoftware, martong, NoQ
Reviewed By: NoQ
Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80699
Summary:
This reverts commit 33fb9cbe21.
That commit violates layering by adding a dependency from StaticAnalyzer/Core
back to StaticAnalyzer/FrontEnd, creating a circular dependency.
I can't see a clean way to fix it except refactoring.
Reviewers: echristo, Szelethus, martong
Subscribers: xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D81752
The thrilling conclusion to the barrage of patches I uploaded lately! This is a
big milestone towards the goal set out in http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html.
I hope to accompany this with a patch where the a coreModeling package is added,
from which package diagnostics aren't allowed either, is an implicit dependency
of all checkers, and the core package for the first time can be safely disabled.
Differential Revision: https://reviews.llvm.org/D78126
Checker dependencies were added D54438 to solve a bug where the checker names
were incorrectly registered, for example, InnerPointerChecker would incorrectly
emit diagnostics under the name MallocChecker, or vice versa [1]. Since the
system over the course of about a year matured, our expectations of what a role
of a dependency and a dependent checker should be crystallized a bit more --
D77474 and its summary, as well as a variety of patches in the stack
demonstrates how we try to keep dependencies to play a purely modeling role. In
fact, D78126 outright forbids diagnostics under a dependency checkers name.
These dependencies ensured the registration order and enabling only when all
dependencies are satisfied. This was a very "strong" contract however, that
doesn't fit the dependency added in D79420. As its summary suggests, this
relation is directly in between diagnostics, not modeling -- we'd prefer a more
specific warning over a general one.
To support this, I added a new dependency kind, weak dependencies. These are not
as strict of a contract, they only express a preference in registration order.
If a weak dependency isn't satisfied, the checker may still be enabled, but if
it is, checker registration, and transitively, checker callback evaluation order
is ensured.
If you are not familiar with the TableGen changes, a rather short description
can be found in the summary of D75360. A lengthier one is in D58065.
[1] https://www.youtube.com/watch?v=eqKeqHRAhQM
Differential Revision: https://reviews.llvm.org/D80905
Retrieving the parameter location of functions was disabled because it
may causes crashes due to the fact that functions may have multiple
declarations and without definition it is difficult to ensure that
always the same declration is used. Now parameters are stored in
`ParamRegions` which are independent of the declaration of the function,
therefore the same parameters always have the same regions,
independently of the function declaration used actually. This allows us
to remove the limitation described above.
Differential Revision: https://reviews.llvm.org/D80286
Currently, parameters of functions without their definition present cannot
be represented as regions because it would be difficult to ensure that the
same declaration is used in every case. To overcome this, we split
`VarRegion` to two subclasses: `NonParamVarRegion` and `ParamVarRegion`.
The latter does not store the `Decl` of the parameter variable. Instead it
stores the index of the parameter which enables retrieving the actual
`Decl` every time using the function declaration of the stack frame. To
achieve this we also removed storing of `Decl` from `DeclRegion` and made
`getDecl()` pure virtual. The individual `Decl`s are stored in the
appropriate subclasses, such as `FieldRegion`, `ObjCIvarRegion` and the
newly introduced `NonParamVarRegion`.
Differential Revision: https://reviews.llvm.org/D80522
Checkers should be able to get the return value under construction for a
`CallEvenet`. This patch adds a function to achieve this which retrieves
the return value from the construction context of the call.
Differential Revision: https://reviews.llvm.org/D80366
Summary: LoopWidening is invalidating references coming from type
aliases which lead to a crash.
Patch by Abbas Sabra!
Differential Revision: https://reviews.llvm.org/D80669
Summary:
See https://bugs.llvm.org/show_bug.cgi?id=46128. The checker does not
yet comprehend constraints involving multiple symbols, so it's possible
to calculate a VLA size that's negative or 0. A LIT is added to catch
regressions, and this change simply bails if a VLA size of 0 or less is
calculated.
Reviewers: balazske, NoQ, martong, baloghadamsoftware, Szelethus, gamesh411
Reviewed By: balazske, NoQ, Szelethus
Subscribers: xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, Charusso, ASDenysPetrov, cfe-commits, dkrupp
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80903
The checker currently supports only a whitelist of block-enumeration
methods which are known to internally clear an autorelease pool.
Extend this checker to detect writes within the scope of explicit
@autoreleasepool statements.
rdar://25301111
Differential Revision: https://reviews.llvm.org/D81072
Idiomatic objc using ARC will generate this expression regularly due to
NSError out-param passing. Providing an implementation for this
expression allows the analyzer to explore many more codepaths in ARC
projects.
The current implementation is not perfect but the differences are hopefully
subtle enough to not cause much problems.
rdar://63918914
Differential Revision: https://reviews.llvm.org/D81071
In the added testfile, the from argument was recognized as
&Element{SymRegion{reg_$0<long * global_a>},-1 S64b,long}
instead of
reg_$0<long * global_a>.
This patch implements matrix index expressions
(matrix[RowIdx][ColumnIdx]).
It does so by introducing a new MatrixSubscriptExpr(Base, RowIdx, ColumnIdx).
MatrixSubscriptExprs are built in 2 steps in ActOnMatrixSubscriptExpr. First,
if the base of a subscript is of matrix type, we create a incomplete
MatrixSubscriptExpr(base, idx, nullptr). Second, if the base is an incomplete
MatrixSubscriptExpr, we create a complete
MatrixSubscriptExpr(base->getBase(), base->getRowIdx(), idx)
Similar to vector elements, it is not possible to take the address of
a MatrixSubscriptExpr.
For CodeGen, a new MatrixElt type is added to LValue, which is very
similar to VectorElt. The only difference is that we may need to cast
the type of the base from an array to a vector type when accessing it.
Reviewers: rjmccall, anemet, Bigcheese, rsmith, martong
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D76791
Summary:
In this patch I am trying to get rid of the `Irrelevant` types from the
signatures of the functions from the standard C library. For that I've
introduced `lookupType()` to be able to lookup arbitrary types in the global
scope. This makes it possible to define the signatures precisely.
Note 1) `fread`'s signature is now fixed to have the proper `FILE *restrict`
type when C99 is the language.
Note 2) There are still existing `Irrelevant` types, but they are all from
POSIX. I am planning to address those together with the missing POSIX functions
(in D79433).
Reviewers: xazax.hun, NoQ, Szelethus, balazske
Subscribers: whisperity, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80016
Summary:
Once we found a matching FunctionDecl for the given summary then we
validate the given constraints against that FunctionDecl. E.g. we
validate that a NotNull constraint is applied only on arguments that
have pointer types.
This is needed because when we matched the signature of the summary we
were working with incomplete function types, i.e. some intricate type
could have been marked as `Irrelevant` in the signature.
Reviewers: NoQ, Szelethus, balazske
Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77658
Summary:
Further develop the buffer size argumentum constraint so it can handle sizes
that we can get by multiplying two variables.
Reviewers: Szelethus, NoQ, steakhal
Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77148
Summary:
Introducing a new argument constraint to confine buffer sizes. It is typical in
C APIs that a parameter represents a buffer and another param holds the size of
the buffer (or the size of the data we want to handle from the buffer).
Reviewers: NoQ, Szelethus, Charusso, steakhal
Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77066
Summary:
New logic tries to narrow possible result values of the remainder operation
based on its operands and their ranges. It also tries to be conservative
with negative operands because according to the standard the sign of
the result is implementation-defined.
rdar://problem/44978988
Differential Revision: https://reviews.llvm.org/D80117
Summary:
Previously the current solver started reasoning about bitwise AND
expressions only when one of the operands is a constant. However,
very similar logic could be applied to ranges. This commit addresses
this shortcoming. Additionally, it refines how we deal with negative
operands.
rdar://problem/54359410
Differential Revision: https://reviews.llvm.org/D79434
Summary:
Previously the current solver started reasoning about bitwise OR
expressions only when one of the operands is a constant. However,
very similar logic could be applied to ranges. This commit addresses
this shortcoming. Additionally, it refines how we deal with negative
operands.
Differential Revision: https://reviews.llvm.org/D79336
Summary:
This change introduces a new component to unite all of the reasoning
we have about operations on ranges in the analyzer's solver.
In many cases, we might conclude that the range for a symbolic operation
is much more narrow than the type implies. While reasoning about
runtime conditions (especially in loops), we need to support more and
more of those little pieces of logic. The new component mostly plays
a role of an organizer for those, and allows us to focus on the actual
reasoning about ranges and not dispatching manually on the types of the
nested symbolic expressions.
Differential Revision: https://reviews.llvm.org/D79232
Summary:
SymIntExpr, IntSymExpr, and SymSymExpr share a big portion of logic
that used to be duplicated across all three classes. New
implementation also adds an easy way of introducing another type of
operands into the mix.
Differential Revision: https://reviews.llvm.org/D79156
Summary:
CompoundLiteralRegions have been properly modeled before, but
'getBindingForElement` was not changed to accommodate this change
properly.
rdar://problem/46144644
Differential Revision: https://reviews.llvm.org/D78990
Summary:
According to the standard, after a `wread` or `fwrite` call the file position
becomes "indeterminate". It is assumable that a next read or write causes
undefined behavior, so a (fatal error) warning is added for this case.
The indeterminate position can be cleared by some operations, for example
`fseek` or `freopen`, not with `clearerr`.
Reviewers: Szelethus, baloghadamsoftware, martong, NoQ, xazax.hun, dcoughlin
Reviewed By: Szelethus
Subscribers: rnkovacs, NoQ, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80018
IE throws errors while using key and mouse navigation through the error path tips.
querySelectorAll method returns NodeList. NodeList belongs to browser API. IE doesn't have forEach among NodeList's methods. At the same time Array is a JavaScript object and can be used instead. The fix is in the converting NodeList into Array and keeps using forEach method as before.
Checked in IE11, Chrome and Opera.
Differential Revision: https://reviews.llvm.org/D80444
If you remember the mail [1] I sent out about how I envision the future of the
already existing checkers to look dependencywise, one my main points was that no
checker that emits diagnostics should be a dependency. This is more problematic
for some checkers (ahem, RetainCount [2]) more than for others, like this one.
The MallocChecker family is a mostly big monolithic modeling class some small
reporting checkers that only come to action when we are constructing a warning
message, after the actual bug was detected. The implication of this is that
NewDeleteChecker doesn't really do anything to depend on, so this change was
relatively simple.
The only thing that complicates this change is that FreeMemAux (MallocCheckers
method that models general memory deallocation) returns after calling a bug
reporting method, regardless whether the report was ever emitted (which may not
always happen, for instance, if the checker responsible for the report isn't
enabled). This return unfortunately happens before cleaning up the maps in the
GDM keeping track of the state of symbols (whether they are released, whether
that release was successful, etc). What this means is that upon disabling some
checkers, we would never clean up the map and that could've lead to false
positives, e.g.:
error: 'warning' diagnostics seen but not expected:
File clang/test/Analysis/NewDelete-intersections.mm Line 66: Potential leak of memory pointed to by 'p'
File clang/test/Analysis/NewDelete-intersections.mm Line 73: Potential leak of memory pointed to by 'p'
File clang/test/Analysis/NewDelete-intersections.mm Line 77: Potential leak of memory pointed to by 'p'
error: 'warning' diagnostics seen but not expected:
File clang/test/Analysis/NewDelete-checker-test.cpp Line 111: Undefined or garbage value returned to caller
File clang/test/Analysis/NewDelete-checker-test.cpp Line 200: Potential leak of memory pointed to by 'p'
error: 'warning' diagnostics seen but not expected:
File clang/test/Analysis/new.cpp Line 137: Potential leak of memory pointed to by 'x'
There two possible approaches I had in mind:
Make bug reporting methods of MallocChecker returns whether they succeeded, and
proceed with the rest of FreeMemAux if not,
Halt execution with a sink node upon failure. I decided to go with this, as
described in the code.
As you can see from the removed/changed test files, before the big checker
dependency effort landed, there were tests to check for all the weird
configurations of enabled/disabled checkers and their messy interactions, I
largely repurposed these.
[1] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html
[2] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063205.html
Differential Revision: https://reviews.llvm.org/D77474
Similarly to other patches of mine, I'm trying to uniformize the checker
interface so that dependency checkers don't emit diagnostics. The checker that
made me most anxious so far was definitely RetainCount, because it is definitely
impacted by backward compatibility concerns, and implements a checker hierarchy
that is a lot different to other examples of similar size. Also, I don't have
authority, nor expertise regarding ObjC related code, so I welcome any
objection/discussion!
Differential Revision: https://reviews.llvm.org/D78099
The `SubEngine` interface is an interface with only one implementation
`EpxrEngine`. Adding other implementations are difficult and very
unlikely in the near future. Currently, if anything from `ExprEngine` is
to be exposed to other classes it is moved to `SubEngine` which
restricts the alternative implementations. The virtual methods are have
a slight perofrmance impact. Furthermore, instead of the `LLVM`-style
inheritance a native inheritance is used here, which renders `LLVM`
functions like e.g. `cast<T>()` unusable here. This patch removes this
interface and allows usage of `ExprEngine` directly.
Differential Revision: https://reviews.llvm.org/D80548
As per http://lists.llvm.org/pipermail/cfe-dev/2019-August/063215.html, lets get rid of this option.
It presents 2 issues that have bugged me for years now:
* OSObject is NOT a boolean option. It in fact has 3 states:
* osx.OSObjectRetainCount is enabled but OSObject it set to false: RetainCount
regards the option as disabled.
* sx.OSObjectRetainCount is enabled and OSObject it set to true: RetainCount
regards the option as enabled.
* osx.OSObjectRetainCount is disabled: RetainCount regards the option as
disabled.
* The hack involves directly modifying AnalyzerOptions::ConfigTable, which
shouldn't even be public in the first place.
This still isn't really ideal, because it would be better to preserve the option
and remove the checker (we want visible checkers to be associated with
diagnostics, and hidden options like this one to be associated with changing how
the modeling is done), but backwards compatibility is an issue.
Differential Revision: https://reviews.llvm.org/D78097
Summary:
This fixes https://bugs.llvm.org/show_bug.cgi?id=41588
RangeSet Negate function shall handle unsigned ranges as well as signed ones.
RangeSet getRangeForMinusSymbol function shall use wider variety of ranges, not only concrete value ranges.
RangeSet Intersect functions shall not produce assertions.
Changes:
Improved safety of RangeSet::Intersect function. Added isEmpty() check to prevent an assertion.
Added support of handling unsigned ranges to RangeSet::Negate and RangeSet::getRangeForMinusSymbol.
Extended RangeSet::getRangeForMinusSymbol to return not only range sets with single value [n,n], but with wide ranges [n,m].
Added unit test for Negate function.
Added regression tests for unsigned values.
Differential Revision: https://reviews.llvm.org/D77802
When loop counter is a function parameter "isPossiblyEscaped" will not find
the variable declaration which lead to hitting "llvm_unreachable".
Parameters of reference type should be escaped like global variables;
otherwise treat them as unescaped.
Patch by Abbas Sabra!
Differential Revision: https://reviews.llvm.org/D80171
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
Summary:
Stream functions `fread` and `fwrite` are evaluated
and preconditions checked.
A new bug type is added for a (non fatal) warning if `fread`
is called in EOF state.
Reviewers: Szelethus, NoQ, dcoughlin, baloghadamsoftware, martong, xazax.hun
Reviewed By: Szelethus
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80015
Exactly what it says on the tin! This is clearly not the end of the road in this
direction, the parameters could be merged far more with the use of CallEvent or
a better value type in the CallDescriptionMap, but this was shockingly difficult
enough on its own. I expect that simplifying the file further will be far easier
moving forward.
The end goal is to research how we could create a more mature checker
interaction infrastructure for more complicated C++ modeling, and I'm pretty
sure that being able successfully split up our giants is the first step in this
direction.
Differential Revision: https://reviews.llvm.org/D75432
The title and the included test file sums everything up -- the only thing I'm
mildly afraid of is whether anyone actually depends on the weird behavior of
HTMLDiagnostics pretending to be TextDiagnostics if an output directory is not
supplied. If it is, I guess we would need to resort to tiptoeing around the
compatibility flag.
Differential Revision: https://reviews.llvm.org/D76510
Party based on this thread:
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.
This patch merges two of CXXAllocatorCall's parameters, so that we are able to
supply a CallEvent object to check::NewAllocatorCall (see the description of
D75430 to see why this would be great).
One of the things mentioned by @NoQ was the following:
I think at this point we might actually do a good job sorting out this
check::NewAllocator issue because we have a "separate" "Environment" to hold
the other SVal, which is "objects under construction"! - so we should probably
simply teach CXXAllocatorCall to extract the value from the
objects-under-construction trait of the program state and we're good.
I had MallocChecker in my crosshair for now, so I admittedly threw together
something as a proof of concept. Now that I know that this effort is worth
pursuing though, I'll happily look for a solution better then demonstrated in
this patch.
Differential Revision: https://reviews.llvm.org/D75431
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
I think anyone who added a checker config wondered why is there a need
to test this. Its just a chore when adding a new config, so I removed
it.
To give some historic insight though, we used to not list **all**
options, but only those explicitly added to AnalyzerOptions, such as the
ones specified on the command line. However, past this change (and
arguably even before that) this line makes little sense.
There is an argument to be made against the entirety of
analyzer-config.c test file, but since this commit fixes some builtbots
and is landing without review, I wouldn't like to be too invasive.
The very essence of MallocChecker lies in 2 overload sets: the FreeMemAux
functions and the MallocMemAux functions. The former houses most of the error
checking as well (aside from leaks), such as incorrect deallocation. There, we
check whether the argument's MemSpaceRegion is the heap or unknown, and if it
isn't, we know we encountered a bug (aside from a corner case patched by
@balazske in D76830), as specified by MEM34-C.
In ReallocMemAux, which really is the combination of FreeMemAux and
MallocMemAux, we incorrectly early returned if the memory argument of realloc is
non-symbolic. The problem is, one of the cases where this happens when we know
precisely what the region is, like an array, as demonstrated in the test file.
So, lets get rid of this false negative :^)
Side note, I dislike the warning message and the associated checker name, but
I'll address it in a later patch.
Differential Revision: https://reviews.llvm.org/D79415
Summary:
Variable-length array (VLA) should have a size that fits into
a size_t value. According to the standard: "std::size_t can
store the maximum size of a theoretically possible object of
any type (including array)" (this is applied to C too).
The size expression is evaluated at the definition of the
VLA type even if this is a typedef.
The evaluation of the size expression in itself might cause
problems if it overflows.
Reviewers: Szelethus, baloghadamsoftware, martong, gamesh411
Reviewed By: Szelethus, martong, gamesh411
Subscribers: whisperity, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79330
One of the pain points in simplifying MallocCheckers interface by gradually
changing to CallEvent is that a variety of C++ allocation and deallocation
functionalities are modeled through preStmt<...> where CallEvent is unavailable,
and a single one of these callbacks can prevent a mass parameter change.
This patch introduces a new CallEvent, CXXDeallocatorCall, which happens after
preStmt<CXXDeleteExpr>, and can completely replace that callback as
demonstrated.
Differential Revision: https://reviews.llvm.org/D75430
Summary:
State of error flags for a stream is handled by having separate flags
that allow combination of multiple error states to be described with one
error state object.
After a failed function the error state is set in the stream state
and must not be determined later based on the last failed function
like before this change. The error state can not always be determined
from the last failed function and it was not the best design.
Reviewers: Szelethus
Reviewed By: Szelethus
Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80009
This operator is intended for casting between
pointers to objects in different address spaces
and follows similar logic as const_cast in C++.
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60193
Summary:
Nonnull attribute can be applied to non-pointers. This caused assertion
failures in NonNullParamChecker when we tried to *assume* such parameters
to be non-zero.
rdar://problem/63150074
Differential Revision: https://reviews.llvm.org/D79843
Summary:
Some function path may lead to crash.
Fixed using local variable outside the scope through a pointer.
Fixed minor misspellings.
Added regression test.
This patch covers a bug https://bugs.llvm.org/show_bug.cgi?id=41485
Reviewed By: baloghadamsoftware
Differential Revision: https://reviews.llvm.org/D78289
Summary:
Objective-C Class objects can be used to do a dynamic dispatch on
class methods. The analyzer had a few places where we tried to overcome
the dynamic nature of it and still guess the actual function that
is being called. That was done mostly using some simple heuristics
covering the most widespread cases (e.g. [[self class] classmethod]).
This solution introduces a way to track types represented by Class
objects and work with that instead of direct AST matching.
rdar://problem/50739539
Differential Revision: https://reviews.llvm.org/D78286
Summary:
Currently we map function summaries to names (i.e. strings). We can
associate more summaries with different signatures to one name, this way
we support overloading. During a call event we check whether the
signature of the summary matches the signature of the callee and we
apply the summary only in that case.
In this patch we change this mapping to associate a summary to a
FunctionDecl. We do lookup operations when the summary map is
initialized. We lookup the given name and we match the signature of the
given summary against the lookup results. If the summary matches the
FunctionDecl (got from the lookup result) then we add that to the
summary map. During a call event we compare FunctionDecl pointers.
Advantages of this new refactor:
- Cleaner mapping and structure for the checker.
- Possibly way more efficient handling of call events.
- A summary is added only if that is relevant for the given TU.
- We can get the concrete FunctionDecl by the time when we create the
summary, this opens up possibilities of further sanity checks
regarding the summary cases and argument constraints.
- Opens up to future work when we'd like to store summaries from IR to a
FunctionDecl (or from the Attributor results of the given
FunctionDecl).
Note, we cannot support old C functions without prototypes.
Reviewers: NoQ, Szelethus, balazske, jdoerfert, sstefan1, uenoku
Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, uenoku, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77641
Static analyzer has a mechanism of clearing redundant nodes when
analysis hits a certain threshold with a number of nodes in exploded
graph (default is 1000). It is similar to GC and aims removing nodes
not useful for analysis. Unfortunately nodes corresponding to array
subscript expressions (that actively participate in data propagation)
get removed during the cleanup. This might prevent the analyzer from
generating useful notes about where it thinks the data came from.
This fix is pretty much consistent with the way analysis works
already. Lvalue "interestingness" stands for the analyzer's
possibility of tracking values through them.
Differential Revision: https://reviews.llvm.org/D78638
We want to trust user type annotations and stop assuming pointers declared
as nonnull still can be null. This functionality is implemented as part
of NonNullParamChecker because it already checks parameter attributes.
Whenever we start analyzing a new function, we assume that all parameters
with 'nonnull' attribute are indeed non-null.
Patch by Valeriy Savchenko!
Differential Revision: https://reviews.llvm.org/D77806
We want to trust user type annotations and stop assuming pointers declared
as _Nonnull still can be null. This functionality is implemented as part
of NullabilityChecker as it already tracks non-null types.
Patch by Valeriy Savchenko!
Differential Revision: https://reviews.llvm.org/D77722
It can be used to avoid passing the begin and end of a range.
This makes the code shorter and it is consistent with another
wrappers we already have.
Differential revision: https://reviews.llvm.org/D78016
Exactly what it says on the tin! The included testfile demonstrates why this is
important -- for C++ dynamic memory operators, we don't always recognize custom,
or even standard-specified new/delete operators as CXXAllocatorCall or
CXXDeallocatorCall.
Differential Revision: https://reviews.llvm.org/D77391
Exactly what it says on the tin! There is no reason I think not to have this.
Also, I added test files for checkers that emit warning under the wrong name.
Differential Revision: https://reviews.llvm.org/D76605
Summary:
I wanted to extend the diagnostics of the CStringChecker with taintedness.
This requires the CStringChecker to be refactored to support a more flexible
reporting mechanism.
This patch does only refactorings, such:
- eliminates always false parameters (like WarnAboutSize)
- reduces the number of parameters
- makes strong types differentiating *source* and *destination* buffers
(same with size expressions)
- binds the argument expression and the index, making diagnostics accurate
and easy to emit
- removes a bunch of default parameters to make it more readable
- remove random const char* warning message parameters, making clear where
and what is going to be emitted
Note that:
- CheckBufferAccess now checks *only* one buffer, this removed about 100 LOC
code duplication
- not every function was refactored to use the /new/ strongly typed API, since
the CString related functions are really closely coupled monolithic beasts,
I will refactor them separately
- all tests are preserved and passing; only the message changed at some places.
In my opinion, these messages are holding the same information.
I would also highlight that this refactoring caught a bug in
clang/test/Analysis/string.c:454 where the diagnostic did not reflect reality.
This catch backs my effort on simplifying this monolithic CStringChecker.
Reviewers: NoQ, baloghadamsoftware, Szelethus, rengolin, Charusso
Reviewed By: NoQ
Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin,
mikhail.ramalho, donat.nagy, dkrupp, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74806
This reverts commit 97aa593a83 as it
causes problems (PR45453) https://reviews.llvm.org/D77574#1966321.
This additionally adds an explicit reference to FrontendOpenMP to
clang-tidy where ASTMatchers is used.
This is hopefully just a temporary solution. The dependence on
`FrontendOpenMP` from `ASTMatchers` should be handled by CMake
implicitly, not us explicitly.
Reviewed By: aheejin
Differential Revision: https://reviews.llvm.org/D77666
Summary:
ASTMatchers is used in various places and it now exposes the
LLVMFrontendOpenMP library to its users without them needing to depend
on it explicitly.
Reviewers: lebedev.ri
Subscribers: mgorny, yaxunl, bollu, guansong, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77574
Constructors and delete operators cannot return a boolean value.
Therefore they cannot possibly follow the NS/CFError-related coding
conventions.
Patch by Valeriy Savchenko!
Differential Revision: https://reviews.llvm.org/D77551
Summary:
Currently we match the summary signature based on the arguments in the CallExpr.
There are a few problems with this approach.
1) Variadic arguments are handled badly. Consider the below code:
int foo(void *stream, const char *format, ...);
void test_arg_constraint_on_variadic_fun() {
foo(0, "%d%d", 1, 2); // CallExpr
}
Here the call expression holds 4 arguments, whereas the function declaration
has only 2 `ParmVarDecl`s. So there is no way to create a summary that
matches the call expression, because the discrepancy in the number of
arguments causes a mismatch.
2) The call expression does not handle the `restrict` type qualifier.
In C99, fwrite's signature is the following:
size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
However, in a call expression, like below, the type of the argument does not
have the restrict qualifier.
void test_fread_fwrite(FILE *fp, int *buf) {
size_t x = fwrite(buf, sizeof(int), 10, fp);
}
This can result in an unmatches signature, so the summary is not applied.
The solution is to match the summary against the referened callee
`FunctionDecl` that we can query from the `CallExpr`.
Further patches will continue with additional refactoring where I am going to
do a lookup during the checker initialization and the signature match will
happen there. That way, we will not check the signature during every call,
rather we will compare only two `FunctionDecl` pointers.
Reviewers: NoQ, Szelethus, gamesh411, baloghadamsoftware
Subscribers: whisperity, xazax.hun, kristof.beyls, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, steakhal, danielkiss, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77410
This is a cleanup and normalization patch that also enables reuse with
Flang later on. A follow up will clean up and move the directive ->
clauses mapping.
Reviewed By: fghanim
Differential Revision: https://reviews.llvm.org/D77112
Summary:
Previously we induced a state split if there were multiple argument
constraints given for a function. This was because we called
`addTransition` inside the for loop.
The fix is to is to store the state and apply the next argument
constraint on that. And once the loop is finished we call `addTransition`.
Reviewers: NoQ, Szelethus, baloghadamsoftware
Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, C
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76790
This is a cleanup and normalization patch that also enables reuse with
Flang later on. A follow up will clean up and move the directive ->
clauses mapping.
Differential Revision: https://reviews.llvm.org/D77112
Summary:
This check was causing a crash in a test case where the 0th argument was
uninitialized ('Assertion `T::isKind(*this)' at line SVals.h:104). This
was happening since the argument was actually undefined, but the castAs
assumes the value is DefinedOrUnknownSVal.
The fix appears to be simply to check for an undefined value and skip
the check allowing the uninitalized value checker to detect the error.
I included a test case that I verified to produce the negative case
prior to the fix, and passes with the fix.
Reviewers: martong, NoQ
Subscribers: xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, Charusso, ASDenysPetrov, baloghadamsoftware, dkrupp, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77012
Since its important to know whether a function frees memory (even if its a
reallocating function!), I used two CallDescriptionMaps to merge all
CallDescriptions into it. MemFunctionInfoTy no longer makes sense, it may never
have, but for now, it would be more of a distraction then anything else.
Differential Revision: https://reviews.llvm.org/D68165
Summary:
Added basic representation and parsing/sema handling of array-shaping
operations. Array shaping expression is an expression of form ([s0]..[sn])base,
where s0, ..., sn must be a positive integer, base - a pointer. This
expression is a kind of cast operation that converts pointer expression
into an array-like kind of expression.
Reviewers: rjmccall, rsmith, jdoerfert
Subscribers: guansong, arphaman, cfe-commits, caomhin, kkwli0
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74144
Summary:
The kernel kmalloc function may return a constant value ZERO_SIZE_PTR
if a zero-sized block is allocated. This special value is allowed to
be passed to kfree and should produce no warning.
This is a simple version but should be no problem. The macro is always
detected independent of if this is a kernel source code or any other
code.
Reviewers: Szelethus, martong
Reviewed By: Szelethus, martong
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76830
Iterator checkers (and planned container checkers) need the option
aggressive-binary-operation-simplification to be enabled. Without this
option they may cause assertions. To prevent such misuse, this patch adds
a preventive check which issues a warning and denies the registartion of
the checker if this option is disabled.
Differential Revision: https://reviews.llvm.org/D75171
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
Originally commited in rG57b8a407493c34c3680e7e1e4cb82e097f43744a, but
it broke the modules bot. This is solved by putting the contructors of
the CheckerManager class to the Frontend library.
Differential Revision: https://reviews.llvm.org/D75360
If an error happens which is related to a container the Container
Modeling checker adds note tags to all the container operations along
the bug path. This may be disturbing if there are other containers
beside the one which is affected by the bug. This patch restricts the
note tags to only the affected container and adjust the debug checkers
to be able to test this change.
Differential Revision: https://reviews.llvm.org/D75514
Container operations such as `push_back()`, `pop_front()`
etc. increment and decrement the abstract begin and end
symbols of containers. This patch introduces note tags
to `ContainerModeling` to track these changes. This helps
the user to better identify the source of errors related
to containers and iterators.
Differential Revision: https://reviews.llvm.org/D73720
Normally clang avoids creating expressions when it encounters semantic
errors, even if the parser knows which expression to produce.
This works well for the compiler. However, this is not ideal for
source-level tools that have to deal with broken code, e.g. clangd is
not able to provide navigation features even for names that compiler
knows how to resolve.
The new RecoveryExpr aims to capture the minimal set of information
useful for the tools that need to deal with incorrect code:
source range of the expression being dropped,
subexpressions of the expression.
We aim to make constructing RecoveryExprs as simple as possible to
ensure writing code to avoid dropping expressions is easy.
Producing RecoveryExprs can result in new code paths being taken in the
frontend. In particular, clang can produce some new diagnostics now and
we aim to suppress bogus ones based on Expr::containsErrors.
We deliberately produce RecoveryExprs only in the parser for now to
minimize the code affected by this patch. Producing RecoveryExprs in
Sema potentially allows to preserve more information (e.g. type of an
expression), but also results in more code being affected. E.g.
SFINAE checks will have to take presence of RecoveryExprs into account.
Initial implementation only works in C++ mode, as it relies on compiler
postponing diagnostics on dependent expressions. C and ObjC often do not
do this, so they require more work to make sure we do not produce too
many bogus diagnostics on the new expressions.
See documentation of RecoveryExpr for more details.
original patch from Ilya
This change is based on https://reviews.llvm.org/D61722
Reviewers: sammccall, rsmith
Reviewed By: sammccall, rsmith
Tags: #clang
Differential Revision: https://reviews.llvm.org/D69330
TableGen and .def files (which are meant to be used with the preprocessor) come
with obvious downsides. One of those issues is that generated switch-case
branches have to be identical. This pushes corner cases either to an outer code
block, or into the generated code.
Inspect the removed code in AnalysisConsumer::DigestAnalyzerOptions. You can see
how corner cases like a not existing output file, the analysis output type being
set to PD_NONE, or whether to complement the output with additional diagnostics
on stderr lay around the preprocessor generated code. This is a bit problematic,
as to how to deal with such errors is not in the hands of the users of this
interface (those implementing output types, like PlistDiagnostics etc).
This patch changes this by moving these corner cases into the generated code,
more specifically, into the called functions. In addition, I introduced a new
output type for convenience purposes, PD_TEXT_MINIMAL, which always existed
conceptually, but never in the actual Analyses.def file. This refactoring
allowed me to move TextDiagnostics (renamed from ClangDiagPathDiagConsumer) to
its own file, which it really deserved.
Also, those that had the misfortune to gaze upon Analyses.def will probably
enjoy the sight that a clang-format did on it.
Differential Revision: https://reviews.llvm.org/D76509
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
Its been a while since my CheckerRegistry related patches landed, allow me to
refresh your memory:
During compilation, TblGen turns
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td into
(build directory)/tools/clang/include/clang/StaticAnalyzer/Checkers/Checkers.inc.
This is a file that contains the full name of the checkers, their options, etc.
The class that is responsible for parsing this file is CheckerRegistry. The job
of this class is to establish what checkers are available for the analyzer (even
from plugins and statically linked but non-tblgen generated files!), and
calculate which ones should be turned on according to the analyzer's invocation.
CheckerManager is the class that is responsible for the construction and storage
of checkers. This process works by first creating a CheckerRegistry object, and
passing itself to CheckerRegistry::initializeManager(CheckerManager&), which
will call the checker registry functions (for example registerMallocChecker) on
it.
The big problem here is that these two classes lie in two different libraries,
so their interaction is pretty awkward. This used to be far worse, but I
refactored much of it, which made things better but nowhere near perfect.
---
This patch changes how the above mentioned two classes interact. CheckerRegistry
is mainly used by CheckerManager, and they are so intertwined, it makes a lot of
sense to turn in into a field, instead of a one-time local variable. This has
additional benefits: much of the information that CheckerRegistry conveniently
holds is no longer thrown away right after the analyzer's initialization, and
opens the possibility to pass CheckerManager in the shouldRegister* function
rather then LangOptions (D75271).
There are a few problems with this. CheckerManager isn't the only user, when we
honor help flags like -analyzer-checker-help, we only have access to a
CompilerInstance class, that is before the point of parsing the AST.
CheckerManager makes little sense without ASTContext, so I made some changes and
added new constructors to make it constructible for the use of help flags.
Differential Revision: https://reviews.llvm.org/D75360
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
Most clients of SourceManager.h need to do things like turning source
locations into file & line number pairs, but this doesn't require
bringing in FileManager.h and LLVM's FS headers.
The main code change here is to sink SM::createFileID into the cpp file.
I reason that this is not performance critical because it doesn't happen
on the diagnostic path, it happens along the paths of macro expansion
(could be hot) and new includes (less hot).
Saves some includes:
309 - /usr/local/google/home/rnk/llvm-project/clang/include/clang/Basic/FileManager.h
272 - /usr/local/google/home/rnk/llvm-project/clang/include/clang/Basic/FileSystemOptions.h
271 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/VirtualFileSystem.h
267 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/FileSystem.h
266 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/Chrono.h
Differential Revision: https://reviews.llvm.org/D75406
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
Most of the getter functions (and a reporter function) in
`CheckerManager` are constant but not marked as `const`. This prevents
functions having only a constant reference to `CheckerManager` using
these member functions. This patch fixes this issue.
Differential Revision: https://reviews.llvm.org/D75839
Nithin Vadukkumchery Rajendrakumar