Since CallExpr::setNumArgs has been removed, it is now possible to store the
callee expression and the argument expressions of CallExpr in a trailing array.
This saves one pointer per CallExpr, CXXOperatorCallExpr, CXXMemberCallExpr,
CUDAKernelCallExpr and UserDefinedLiteral.
Given that CallExpr is used as a base of the above classes we cannot use
llvm::TrailingObjects. Instead we store the offset in bytes from the this pointer
to the start of the trailing objects and manually do the casts + arithmetic.
Some notes:
1.) I did not try to fit the number of arguments in the bit-fields of Stmt.
This leaves some space for future additions and avoid the discussion about
whether x bits are sufficient to hold the number of arguments.
2.) It would be perfectly possible to recompute the offset to the trailing
objects before accessing the trailing objects. However the trailing objects
are frequently accessed and benchmarks show that it is slightly faster to
just load the offset from the bit-fields. Additionally, because of 1),
we have plenty of space in the bit-fields of Stmt.
Differential Revision: https://reviews.llvm.org/D55771
Reviewed By: rjmccall
llvm-svn: 349910
All of the other constructors already take a reference to the AST context.
This avoids calling Decl::getASTContext in most cases. Additionally move
the definition of the constructor from Expr.h to Expr.cpp since it is calling
DeclRefExpr::computeDependence. NFC.
llvm-svn: 349901
StaticAnalyzer uses the CFG-based RelaxedLiveVariables analysis in order to,
in particular, figure out values of which expressions are still needed.
When the expression becomes "dead", it is garbage-collected during
the dead binding scan.
Expressions that constitute branches/bodies of control flow statements,
eg. `E1' in `if (C1) E1;' but not `E2' in `if (C2) { E2; }', were kept alive
for too long. This caused false positives in MoveChecker because it relies
on cleaning up loop-local variables when they go out of scope, but some of those
live-for-too-long expressions were keeping a reference to those variables.
Fix liveness analysis to correctly mark these expressions as dead.
Add a debug checker, debug.DumpLiveStmts, in order to test expressions liveness.
Differential Revision: https://reviews.llvm.org/D55566
llvm-svn: 349320
Summary:
The pattern is problematic with C++ exceptions, and not as widespread as
scoped locks, but it's still used by some, for example Chromium.
We are a bit stricter here at join points, patterns that are allowed for
scoped locks aren't allowed here. That could still be changed in the
future, but I'd argue we should only relax this if people ask for it.
Fixes PR36162.
Reviewers: aaron.ballman, delesley, pwnall
Reviewed By: delesley, pwnall
Subscribers: pwnall, cfe-commits
Differential Revision: https://reviews.llvm.org/D52578
llvm-svn: 349300
CallGraph previously would just show the normal name of a function,
which gets really confusing when using it on large C++ projects. This
patch switches the printName call to a printQualifiedName, so that the
namespaces are included.
Change-Id: Ie086d863f6b2251be92109ea1b0946825b28b49a
llvm-svn: 348950
It seems the two failing tests can be simply fixed after r348037
Fix 3 cases in Analysis/builtin-functions.cpp
Delete the bad CodeGen/builtin-constant-p.c for now
llvm-svn: 348053
Kept the "indirect_builtin_constant_p" test case in test/SemaCXX/constant-expression-cxx1y.cpp
while we are investigating why the following snippet fails:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};
llvm-svn: 348039
This was reverted in r347656 due to me thinking it caused a miscompile of
Chromium. Turns out it was the Chromium code that was broken.
llvm-svn: 347756
This caused a miscompile in Chrome (see crbug.com/908372) that's
illustrated by this small reduction:
static bool f(int *a, int *b) {
return !__builtin_constant_p(b - a) || (!(b - a));
}
int arr[] = {1,2,3};
bool g() {
return f(arr, arr + 3);
}
$ clang -O2 -S -emit-llvm a.cc -o -
g() should return true, but after r347417 it became false for some reason.
This also reverts the follow-up commits.
r347417:
> Re-Reinstate 347294 with a fix for the failures.
>
> Don't try to emit a scalar expression for a non-scalar argument to
> __builtin_constant_p().
>
> Third time's a charm!
r347446:
> The result of is.constant() is unsigned.
r347480:
> A __builtin_constant_p() returns 0 with a function type.
r347512:
> isEvaluatable() implies a constant context.
>
> Assume that we're in a constant context if we're asking if the expression can
> be compiled into a constant initializer. This fixes the issue where a
> __builtin_constant_p() in a compound literal was diagnosed as not being
> constant, even though it's always possible to convert the builtin into a
> constant.
r347531:
> A "constexpr" is evaluated in a constant context. Make sure this is reflected
> if a __builtin_constant_p() is a part of a constexpr.
llvm-svn: 347656
Summary:
Compound literals, enums, file-scoped arrays, etc. require their
initializers and size specifiers to be constant. Wrap the initializer
expressions in a ConstantExpr so that we can easily check for this later
on.
Reviewers: rsmith, shafik
Reviewed By: rsmith
Subscribers: cfe-commits, jyknight, nickdesaulniers
Differential Revision: https://reviews.llvm.org/D53921
llvm-svn: 346455
Summary:
The test case added in this diff would incorrectly warn that control
flow may fall through without returning. Here's a standalone example:
https://godbolt.org/z/dCwXEi
The same program, but using `return` instead of `co_return`, does not
produce a warning: https://godbolt.org/z/mVldqQ
The issue was in how Clang analysis would structure its representation
of the control-flow graph. Specifically, when constructing the CFG,
`CFGBuilder::Visit` had special handling of a `ReturnStmt`, in which it
would place object destructors in the same CFG block as a `return` statement,
immediately after it. Doing so would allow the logic in
`lib/Sema/AnalysisBasedWarning.cpp` `CheckFallThrough` to work properly in the
program that used `return`, correctly determining that no "plain edges" preceded
the exit block of the function.
Because a `co_return` statement would not enjoy the same treatment when
it was being built into the control-flow graph, object destructors
would not be placed in the same CFG block as the `co_return`, thus
resulting in a "plain edge" preceding the exit block of the function,
and so the warning logic would be triggered.
Add special casing for `co_return` to Clang analysis, thereby
remedying the mistaken warning.
Test Plan: `check-clang`
Reviewers: GorNishanov, tks2103, rsmith
Reviewed By: GorNishanov
Subscribers: EricWF, lewissbaker, cfe-commits
Differential Revision: https://reviews.llvm.org/D54075
llvm-svn: 346074
The size of an os_log buffer is known at any stage of compilation, so making it
a constant expression means that the common idiom of declaring a buffer for it
won't result in a VLA. That allows the compiler to skip saving and restoring
the stack pointer around such buffers.
This also moves the OSLog and other FormatString helpers from
libclangAnalysis to libclangAST to avoid a circular dependency.
llvm-svn: 345971
This patch should not introduce any behavior changes. It consists of
mostly one of two changes:
1. Replacing fall through comments with the LLVM_FALLTHROUGH macro
2. Inserting 'break' before falling through into a case block consisting
of only 'break'.
We were already using this warning with GCC, but its warning behaves
slightly differently. In this patch, the following differences are
relevant:
1. GCC recognizes comments that say "fall through" as annotations, clang
doesn't
2. GCC doesn't warn on "case N: foo(); default: break;", clang does
3. GCC doesn't warn when the case contains a switch, but falls through
the outer case.
I will enable the warning separately in a follow-up patch so that it can
be cleanly reverted if necessary.
Reviewers: alexfh, rsmith, lattner, rtrieu, EricWF, bollu
Differential Revision: https://reviews.llvm.org/D53950
llvm-svn: 345882
The size of an os_log buffer is known at any stage of compilation, so making it
a constant expression means that the common idiom of declaring a buffer for it
won't result in a VLA. That allows the compiler to skip saving and restoring
the stack pointer around such buffers.
This also moves the OSLog helpers from libclangAnalysis to libclangAST
to avoid a circular dependency.
llvm-svn: 345866
A ConstantExpr class represents a full expression that's in a context where a
constant expression is required. This class reflects the path the evaluator
took to reach the expression rather than the syntactic context in which the
expression occurs.
In the future, the class will be expanded to cache the result of the evaluated
expression so that it's not needlessly re-evaluated
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D53475
llvm-svn: 345692
We haven't supported compiling ObjC1 for a long time (and never will again), so
there isn't any reason to keep these separate. This patch replaces
LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC.
Differential revision: https://reviews.llvm.org/D53547
llvm-svn: 345637
Only store the NRVO candidate if needed in ReturnStmt.
A good chuck of all of the ReturnStmt have no NRVO candidate
(more than half when parsing all of Boost). For all of them
this saves one pointer. This has no impact on children().
Differential Revision: https://reviews.llvm.org/D53716
Reviewed By: rsmith
llvm-svn: 345605
Only store the needed data in IfStmt. This cuts the size of IfStmt
by up to 3 pointers + 1 SourceLocation. The order of the children
is intentionally kept the same even though it would be more
convenient to put the optional trailing objects last. Additionally
use the newly available space in the bit-fields of Stmt to store
the location of the "if".
The result of this is that for the common case of an
if statement of the form:
if (some_cond)
some_statement
the size of IfStmt is brought down to 8 bytes + 2 pointers,
instead of 8 bytes + 5 pointers + 2 SourceLocation.
Differential Revision: https://reviews.llvm.org/D53607
Reviewed By: rjmccall
llvm-svn: 345464
Summary:
We unwrap conditional expressions containing try-lock functions.
Additionally we don't acquire on conditional expression branches, since
that is usually not helpful. When joining the branches we would almost
certainly get a warning then.
Hopefully fixes an issue that was raised in D52398.
Reviewers: aaron.ballman, delesley, hokein
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52888
llvm-svn: 343902
Summary:
Instead of only examining call arguments, we also examine constructor
arguments applying the same rules.
That was an opportunity for refactoring the examination procedure to
work with iterators instead of integer indices. For the case of
CallExprs no functional change is intended.
Reviewers: aaron.ballman, delesley
Reviewed By: delesley
Subscribers: JonasToth, cfe-commits
Differential Revision: https://reviews.llvm.org/D52443
llvm-svn: 343831
Summary:
When people are really sure they'll get the lock they sometimes use
__builtin_expect. It's also used by some assertion implementations.
Asserting that try-lock succeeded is basically the same as asserting
that the lock is not held by anyone else (and acquiring it).
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Subscribers: kristina, cfe-commits
Differential Revision: https://reviews.llvm.org/D52398
llvm-svn: 343681
Currently, ProgramPoint::dump calls the out-of-line function ProgramPoint::print. This causes
libraries which include ProgramPoint.h to become dependent on libclangAnalysis, which in turn
causes missing symbol link error when building with -DBUILD_SHARED_LIBS=ON -DLLVM_ENABLE_MODULES=ON.
The breakage was introduced in r343160.
This patch fixes the issues by moving ProgramPoint::dump's declaration out of line.
llvm-svn: 343420
Not used productively, so no observable functional change.
Note that printSCFG doesn't yet work reliably, it seems to crash
sometimes.
llvm-svn: 342790
In the case that `win_t` is an `unsigned short` (e.g. on Windows), we would
previously incorrectly diagnose the conversion because we would immediately
promote the argument type from `wint_t` (aka `unsigned short`) to `int` before
checking if the type matched. This should repair the Windows hosted bots.
llvm-svn: 342565
For function pointers, the FunctionDecl of the callee is unknown, so
getDirectCallee will return nullptr. We have to catch that case to avoid
crashing. We assume there is no attribute then.
llvm-svn: 342519
Summary:
This is a follow up of D52008 and should make the analyzer being able to handle perfect forwardings in real world cases where forwardings are done through multiple layers of function calls with `std::forward`.
Fixes PR38891.
Reviewers: lebedev.ri, JonasToth, george.karpenkov
Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D52120
llvm-svn: 342409
Those are not created in the allocator.
Since they are created fairly rarely, a counter overhead should not
affect the memory consumption.
Differential Revision: https://reviews.llvm.org/D51827
llvm-svn: 342314
Summary:
We used to treat an `Expr` mutated whenever it's passed as non-const
reference argument to a function. This results in false positives in
cases like this:
```
int x;
std::vector<int> v;
v.emplace_back(x); // `x` is passed as non-const reference to `emplace_back`
```
In theory the false positives can be suppressed with
`v.emplace_back(std::as_const(x))` but that's considered overly verbose,
inconsistent with existing code and spammy as diags.
This diff handles such cases by following into the function definition
and see whether the argument is mutated inside.
Reviewers: lebedev.ri, JonasToth, george.karpenkov
Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D52008
llvm-svn: 342271
Summary:
This is 1/2 of moving ExprMutationAnalyzer from clangtidy to
clang/Analysis.
This diff along simply copies the ExprMutationAnalyzer over with trivial
modifications (e.g. include path, namespace)
2/2 will migrate existing usage of ExprMutationAnalyzer and remove the
original copy inside clangtidy.
Reviewers: george.karpenkov
Subscribers: mgorny, xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits, JonasToth
Differential Revision: https://reviews.llvm.org/D51948
llvm-svn: 341994
The analyzer doesn't make use of them anyway and they seem to have
pretty weird AST from time to time, so let's just skip them for now.
Fixes pr37769.
Differential Revision: https://reviews.llvm.org/D50824
llvm-svn: 340975
Bruno Ricci