CallGraph visited LambdaExpr by getting the Call Operator from
CXXRecordDecl (LambdaExpr::getCallOperator calls
CXXRecordDecl::getLambdaCallOperator), which replaced generic lambda
call operators with the non-instantiated FunctionDecl. The result was
that the CallGraph would only pick up non-dependent calls.
This patch does a few things:
1- Extend CXXRecordDecl to have a getDependentLambdaCallOperator, which
will get the FunctionTemplateDecl, rather than immediately getting the
TemplateDecl.
2- Define getLambdaCallOperator and getDependentLambdaCallOperator in
terms of a common function.
3- Extend LambdaExpr with a getDependentCallOperator, which just calls
the above function.
4- Changes CallGraph to handle Generic LambdaExprs.
llvm-svn: 373247
-Wtautological-overlap-compare and self-comparison from -Wtautological-compare
relay on detecting the same operand in different locations. Previously, each
warning had it's own operand checker. Now, both are merged together into
one function that each can call. The function also now looks through member
access and array accesses.
Differential Revision: https://reviews.llvm.org/D66045
llvm-svn: 372453
Allow this warning to detect a larger number of constant values, including
negative numbers, and handle non-int types better.
Differential Revision: https://reviews.llvm.org/D66044
llvm-svn: 372448
Summary:
https://bugs.llvm.org/show_bug.cgi?id=43102
In today's edition of "Is this any better now that it isn't crashing?", I'd like to show you a very interesting test case with loop widening.
Looking at the included test case, it's immediately obvious that this is not only a false positive, but also a very bad bug report in general. We can see how the analyzer mistakenly invalidated `b`, instead of its pointee, resulting in it reporting a null pointer dereference error. Not only that, the point at which this change of value is noted at is at the loop, rather then at the method call.
It turns out that `FindLastStoreVisitor` works correctly, rather the supplied explodedgraph is faulty, because `BlockEdge` really is the `ProgramPoint` where this happens.
{F9855739}
So it's fair to say that this needs improving on multiple fronts. In any case, at least the crash is gone.
Full ExplodedGraph: {F9855743}
Reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso, dcoughlin, rnkovacs, TWeaver
Subscribers: JesperAntonsson, uabelho, Ka-Ka, bjope, whisperity, szepet, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66716
llvm-svn: 372269
'-Xclang -triple' doesn't seem to override the default target triple
as reliably as '--target'. This leads to test failing due to
platform-specific checks getting unexpectedly enabled.
llvm-svn: 371781
Short and sweet. Whenever I use -analyzer-list-enabled-checkers, I'm only
interested about the configuration, not about the analysis.
Differential Revision: https://reviews.llvm.org/D66714
llvm-svn: 371756
This reverts r371497 (git commit 3d7e9ab7b9)
Reorder `not` with `env` in these two tests so they pass:
Driver/rewrite-map-in-diagnostics.c
Index/crash-recovery-modules.m.
This will not be necessary after D66531 lands.
llvm-svn: 371552
I see in the history for some of these tests REQUIRES:shell was used as
a way to disable tests on Windows because they are flaky there. I tried
not to re-enable such tests, but it's possible that I missed some and
this will re-enable flaky tests on Windows. If so, we should disable
them with UNSUPPORTED:system-windows and add a comment that they are
flaky there. So far as I can tell, the lit internal shell is capable of
running all of these tests, and we shouldn't use REQUIRES:shell as a
proxy for Windows.
llvm-svn: 371478
Allow attaching fixit hints to Static Analyzer BugReports.
Fixits are attached either to the bug report itself or to its notes
(path-sensitive event notes or path-insensitive extra notes).
Add support for fixits in text output (including the default text output that
goes without notes, as long as the fixit "belongs" to the warning).
Add support for fixits in the plist output mode.
Implement a fixit for the path-insensitive DeadStores checker. Only dead
initialization warning is currently covered.
Implement a fixit for the path-sensitive VirtualCall checker when the virtual
method is not pure virtual (in this case the "fix" is to suppress the warning
by qualifying the call).
Both fixits are under an off-by-default flag for now, because they
require more careful testing.
Differential Revision: https://reviews.llvm.org/D65182
llvm-svn: 371257
Most functions that our checkers react upon are not C-style variadic functions,
and therefore they have as many actual arguments as they have formal parameters.
However, it's not impossible to define a variadic function with the same name.
This will crash any checker that relies on CallDescription to check the number
of arguments but silently assumes that the number of parameters is the same.
Change CallDescription to check both the number of arguments and the number of
parameters by default.
If we're intentionally trying to match variadic functions, allow specifying
arguments and parameters separately (possibly omitting any of them).
For now we only have one CallDescription which would make use of those,
namely __builtin_va_start itself.
Differential Revision: https://reviews.llvm.org/D67019
llvm-svn: 371256
There are some functions which can't be given a null pointer as parameter either
because it has a nonnull attribute or it is declared to have undefined behavior
(e.g. strcmp()). Sometimes it is hard to determine from the checker message
which parameter is null at the invocation, so now this information is included
in the message.
This commit fixes https://bugs.llvm.org/show_bug.cgi?id=39358
Reviewed By: NoQ, Szelethus, whisperity
Patch by Tibor Brunner!
Differential Revision: https://reviews.llvm.org/D66333
llvm-svn: 370798
Enables the users to specify an optional flag which would warn for more dead
stores.
Previously it ignored if the dead store happened e.g. in an if condition.
if ((X = generate())) { // dead store to X
}
This patch introduces the `WarnForDeadNestedAssignments` option to the checker,
which is `false` by default - so this change would not affect any previous
users.
I have updated the code, tests and the docs as well. If I missed something, tell
me.
I also ran the analysis on Clang which generated 14 more reports compared to the
unmodified version. All of them seemed reasonable for me.
Related previous patches:
rGf224820b45c6847b91071da8d7ade59f373b96f3
Reviewers: NoQ, krememek, Szelethus, baloghadamsoftware
Reviewed By: Szelethus
Patch by Balázs Benics!
Differential Revision: https://reviews.llvm.org/D66733
llvm-svn: 370767
initializers.
This has some interesting interactions with our existing extensions to
support C99 designated initializers as an extension in C++. Those are
resolved as follows:
* We continue to permit the full breadth of C99 designated initializers
in C++, with the exception that we disallow a partial overwrite of an
initializer with a non-trivially-destructible type. (Full overwrite
is OK, because we won't run the first initializer at all.)
* The C99 extensions are disallowed in SFINAE contexts and during
overload resolution, where they could change the meaning of valid
programs.
* C++20 disallows reordering of initializers. We only check for that for
the simple cases that the C++20 rules permit (designators of the form
'.field_name =' and continue to allow reordering in other cases).
It would be nice to improve this behavior in future.
* All C99 designated initializer extensions produce a warning by
default in C++20 mode. People are going to learn the C++ rules based
on what Clang diagnoses, so it's important we diagnose these properly
by default.
* In C++ <= 17, we apply the C++20 rules rather than the C99 rules, and
so still diagnose C99 extensions as described above. We continue to
accept designated C++20-compatible initializers in C++ <= 17 silently
by default (but naturally still reject under -pedantic-errors).
This is not a complete implementation of P0329R4. In particular, that
paper introduces new non-C99-compatible syntax { .field { init } }, and
we do not support that yet.
This is based on a previous patch by Don Hinton, though I've made
substantial changes when addressing the above interactions.
Differential Revision: https://reviews.llvm.org/D59754
llvm-svn: 370544
We're building the CFG from bottom to top, so when the return-value expression
has a non-trivial CFG on its own, we need to continue building from the entry
to the return-value expression CFG rather than from the block to which
we've just appended the return statement.
Fixes a false positive warning "control may reach end of non-void function".
llvm-svn: 370406
Range errors (dereferencing or incrementing the past-the-end iterator or
decrementing the iterator of the first element of the range) and access of
invalidated iterators lead to undefined behavior. There is no point to
continue the analysis after such an error on the same execution path, but
terminate it by a sink node (fatal error). This also improves the
performance and helps avoiding double reports (e.g. in case of nested
iterators).
Differential Revision: https://reviews.llvm.org/D62893
llvm-svn: 370314
Respect C++17 copy elision; previously it would generate destructor calls
for elided temporaries, including in initialization and return statements.
Don't generate duplicate destructor calls for statement expressions.
Fix destructors in initialization lists and comma operators.
Improve printing of implicit destructors.
Patch by Nicholas Allegra!
Differential Revision: https://reviews.llvm.org/D66404
llvm-svn: 370247
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
It was known to be a compile-time constant so it wasn't evaluated during
symbolic execution, but it wasn't evaluated as a compile-time constant either.
Differential Revision: https://reviews.llvm.org/D66565
llvm-svn: 370245
If the global variable has an initializer, we'll ignore it because we're usually
not analyzing the program from the beginning, which means that the global
variable may have changed before we start our analysis.
However when we're analyzing main() as the top-level function, we can rely
on global initializers to still be valid. At least in C; in C++ we have global
constructors that can still break this logic.
This patch allows the Static Analyzer to load constant initializers from
global variables if the top-level function of the current analysis is main().
Differential Revision: https://reviews.llvm.org/D65361
llvm-svn: 370244
Summary:
Added basic analysis of map clauses. Only map clauses with to and tofrom
map type must be analyzed since all other map types (alloc, delete, etc.) do not require to use the value of the initial variable, instead they create the new copy of the variable.
Reviewers: NoQ
Subscribers: guansong, cfe-commits, kkwli0, caomhin
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66668
llvm-svn: 370214
According to the SARIF specification, "a text region does not include the character specified by endColumn".
Differential Revision: https://reviews.llvm.org/D65206
llvm-svn: 370060
There are numorous flaws about the name conflict handling, this patch
attempts fixes them. Changes in details:
* HandleNameConflict return with a false DeclarationName
Hitherto we effectively never returned with a NameConflict error, even
if the preceding StructuralMatch indicated a conflict.
Because we just simply returned with the parameter `Name` in
HandleNameConflict and that name is almost always `true` when converted to
`bool`.
* Add tests which indicate wrong NameConflict handling
* Add to ConflictingDecls only if decl kind is different
Note, we might not indicate an ODR error when there is an existing record decl
and a enum is imported with same name. But there are other cases. E.g. think
about the case when we import a FunctionTemplateDecl with name f and we found a
simple FunctionDecl with name f. They overload. Or in case of a
ClassTemplateDecl and CXXRecordDecl, the CXXRecordDecl could be the 'templated'
class, so it would be false to report error. So I think we should report a
name conflict error only when we are 100% sure of that. That is why I think it
should be a general pattern to report the error only if the kind is the same.
* Fix failing ctu test with EnumConstandDecl
In ctu-main.c we have the enum class 'A' which brings in the enum
constant 'x' with value 0 into the global namespace.
In ctu-other.c we had the enum class 'B' which brought in the same name
('x') as an enum constant but with a different enum value (42). This is clearly
an ODR violation in the global namespace. The solution was to rename the
second enum constant.
* Introduce ODR handling strategies
Reviewers: a_sidorin, shafik
Differential Revision: https://reviews.llvm.org/D59692
llvm-svn: 370045
Summary: EnumCastOutOfRangeChecker should not perform enum range checks on LValueToRValue casts, since this type of cast does not actually change the underlying type. Performing the unnecessary check actually triggered an assertion failure deeper in EnumCastOutOfRange for certain input (which is captured in the accompanying test code).
Reviewers: #clang, Szelethus, gamesh411, NoQ
Reviewed By: Szelethus, gamesh411, NoQ
Subscribers: NoQ, gamesh411, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, bjope, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66014
llvm-svn: 369760
Our SVal hierarchy doesn't allow modeling pointer casts as no-op. The
pointer type is instead encoded into the pointer object. Defer to our
usual pointer casting facility, SValBuilder::evalBinOp().
Fixes a crash.
llvm-svn: 369729
This patch concludes my GSoC'19 project by enabling track-conditions by default.
Differential Revision: https://reviews.llvm.org/D66381
llvm-svn: 369616
As discussed on the mailing list, notes originating from the tracking of foreach
loop conditions are always meaningless.
Differential Revision: https://reviews.llvm.org/D66131
llvm-svn: 369613
Summary:
This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `CastSets` which are storing the dynamic cast informations of
objects based on memory regions. It could be used to store and check the
casts and prevent infeasible paths.
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D66325
llvm-svn: 369605
In D65724, I do a pretty thorough explanation about how I'm solving this
problem, I think that summary nails whats happening here ;)
Differential Revision: https://reviews.llvm.org/D65725
llvm-svn: 369596
Exactly what it says on the tin! Note that we're talking about interestingness
in general, hence this isn't a control-dependency-tracking specific patch.
Differential Revision: https://reviews.llvm.org/D65724
llvm-svn: 369589
Can't add much more to the title! This is part 1, the case where the collapse
point isn't in the condition point is the responsibility of ConditionBRVisitor,
which I'm addressing in part 2.
Differential Revision: https://reviews.llvm.org/D65575
llvm-svn: 369574
Add defensive check that prevents a crash when we try to evaluate a destructor
whose this-value is a concrete integer that isn't a null.
Differential Revision: https://reviews.llvm.org/D65349
llvm-svn: 369450
Calling a pure virtual method during construction or destruction
is undefined behavior. It's worth it to warn about it by default.
That part is now known as the cplusplus.PureVirtualCall checker.
Calling a normal virtual method during construction or destruction
may be fine, but does behave unexpectedly, as it skips virtual dispatch.
Do not warn about this by default, but let projects opt in into it
by enabling the optin.cplusplus.VirtualCall checker manually.
Give the two parts differentiated warning text:
Before:
Call to virtual function during construction or destruction:
Call to pure virtual function during construction
Call to virtual function during construction or destruction:
Call to virtual function during destruction
After:
Pure virtual method call:
Call to pure virtual method 'X::foo' during construction
has undefined behavior
Unexpected loss of virtual dispatch:
Call to virtual method 'Y::bar' during construction
bypasses virtual dispatch
Also fix checker names in consumers that support them (eg., clang-tidy)
because we now have different checker names for pure virtual calls and
regular virtual calls.
Also fix capitalization in the bug category.
Differential Revision: https://reviews.llvm.org/D64274
llvm-svn: 369449
This patch improves Clang call graph analysis by adding in expressions
that are not found in regular function bodies, such as default arguments
or member initializers.
Patch by Joshua Cranmer!
Differential Revision: https://reviews.llvm.org/D65453
llvm-svn: 369321
Summary:
Code to import "ctor initializers" at import of functions
is moved to be after the flags in the newly created function
are imported. This fixes an error when the already created but
incomplete (flags are not set) function declaration is accessed.
Reviewers: martong, shafik, a_sidorin, a.sidorin
Reviewed By: shafik
Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65935
llvm-svn: 369098
Summary:
This patch introduces a new `analyzer-config` configuration:
`-analyzer-config silence-checkers`
which could be used to silence the given checkers.
It accepts a semicolon separated list, packed into quotation marks, e.g:
`-analyzer-config silence-checkers="core.DivideZero;core.NullDereference"`
It could be used to "disable" core checkers, so they model the analysis as
before, just if some of them are too noisy it prevents to emit reports.
This patch also adds support for that new option to the scan-build.
Passing the option `-disable-checker core.DivideZero` to the scan-build
will be transferred to `-analyzer-config silence-checkers=core.DivideZero`.
Reviewed By: NoQ, Szelethus
Differential Revision: https://reviews.llvm.org/D66042
llvm-svn: 369078
...because we're working with a BugReporterVisitor, and the non-evaluated part
of the condition isn't in the bugpath.
Differential Revision: https://reviews.llvm.org/D65290
llvm-svn: 368853
Erich Keane