NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
In C++17, when class `C` has large alignment value, a special case of overload
resolution rule kicks in for expression `new C` that causes the aligned version
of `operator new()` to be called. The aligned `new` has two arguments: size and
alignment. However, the new-expression has only one argument: the
construct-expression for `C()`. This causes a false positive in
`core.CallAndMessage`'s check for matching number of arguments and number of
parameters.
Update `CXXAllocatorCall`, which is a `CallEvent` sub-class for operator new
calls within new-expressions, so that the number of arguments always matched
the number of parameters.
Dunno, maybe we should instead abandon the idea of reserving a whole
argument/parameter index for each of those implicit arguments that aren't even
represented by an expression in the AST.
Side note: Ugh, we never supported passing size into `operator new()` calls,
even though it's known in compile time. And now also alignment. They are
currently symbolic (unconstrained) within allocator calls.
Repository:
rC Clang
https://reviews.llvm.org/D52957
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Core/CallEvent.cpp
test/Analysis/new-aligned.cpp
Index: test/Analysis/new-aligned.cpp
===================================================================
--- /dev/null
+++ test/Analysis/new-aligned.cpp
@@ -0,0 +1,14 @@
+//RUN: %clang_analyze_cc1 -std=c++17 -analyze -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+// Notice the weird alignment.
+struct alignas(1024) S {};
+
+void foo() {
+ // Operator new() here is the C++17 aligned new that takes two arguments:
+ // size and alignment. Size is passed implicitly as usual, and alignment
+ // is passed implicitly in a similar manner.
+ S *s = new S; // no-warning
+ delete s;
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -503,10 +503,14 @@
const ParmVarDecl *ParamDecl = *I;
assert(ParamDecl && "Formal parameter has no decl?");
+ // TODO: Support allocator calls.
if (Call.getKind() != CE_CXXAllocator)
if (Call.isArgumentConstructedDirectly(Idx))
continue;
+ // TODO: Allocators should receive the correct size and possibly alignment,
+ // determined in compile-time but not represented as arg-expressions,
+ // which makes getArgSVal() fail and return UnknownVal.
SVal ArgVal = Call.getArgSVal(Idx);
if (!ArgVal.isUnknown()) {
Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx));
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -921,15 +921,28 @@
return getOriginExpr()->getOperatorNew();
}
+ // Size and maybe implicit alignment in C++17. Instead of size, the AST
+ // contains the construct-expression. Alignment is always hidden.
+ // We pretend that argument 0 is size and argument 1 is alignment (if passed
+ // implicitly) and the rest are placement args. This makes sure that the
+ // number of arguments is always the same as the number of parameters.
+ unsigned getNumImplicitArgs() const {
+ return getOriginExpr()->passAlignment() ? 2 : 1;
+ }
+
unsigned getNumArgs() const override {
- return getOriginExpr()->getNumPlacementArgs() + 1;
+ return getOriginExpr()->getNumPlacementArgs() + getNumImplicitArgs();
}
const Expr *getArgExpr(unsigned Index) const override {
// The first argument of an allocator call is the size of the allocation.
- if (Index == 0)
+ if (Index < getNumImplicitArgs())
return nullptr;
- return getOriginExpr()->getPlacementArg(Index - 1);
+ return getOriginExpr()->getPlacementArg(Index - getNumImplicitArgs());
+ }
+
+ const Expr *getPlacementArgExpr(unsigned Index) const {
+ return getOriginExpr()->getPlacementArg(Index);
}
Kind getKind() const override { return CE_CXXAllocator; }
Index: test/Analysis/new-aligned.cpp
===================================================================
--- /dev/null
+++ test/Analysis/new-aligned.cpp
@@ -0,0 +1,14 @@
+//RUN: %clang_analyze_cc1 -std=c++17 -analyze -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+// Notice the weird alignment.
+struct alignas(1024) S {};
+
+void foo() {
+ // Operator new() here is the C++17 aligned new that takes two arguments:
+ // size and alignment. Size is passed implicitly as usual, and alignment
+ // is passed implicitly in a similar manner.
+ S *s = new S; // no-warning
+ delete s;
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -503,10 +503,14 @@
const ParmVarDecl *ParamDecl = *I;
assert(ParamDecl && "Formal parameter has no decl?");
+ // TODO: Support allocator calls.
if (Call.getKind() != CE_CXXAllocator)
if (Call.isArgumentConstructedDirectly(Idx))
continue;
+ // TODO: Allocators should receive the correct size and possibly alignment,
+ // determined in compile-time but not represented as arg-expressions,
+ // which makes getArgSVal() fail and return UnknownVal.
SVal ArgVal = Call.getArgSVal(Idx);
if (!ArgVal.isUnknown()) {
Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx));
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -921,15 +921,28 @@
return getOriginExpr()->getOperatorNew();
}
+ // Size and maybe implicit alignment in C++17. Instead of size, the AST
+ // contains the construct-expression. Alignment is always hidden.
+ // We pretend that argument 0 is size and argument 1 is alignment (if passed
+ // implicitly) and the rest are placement args. This makes sure that the
+ // number of arguments is always the same as the number of parameters.
+ unsigned getNumImplicitArgs() const {
+ return getOriginExpr()->passAlignment() ? 2 : 1;
+ }
+
unsigned getNumArgs() const override {
- return getOriginExpr()->getNumPlacementArgs() + 1;
+ return getOriginExpr()->getNumPlacementArgs() + getNumImplicitArgs();
}
const Expr *getArgExpr(unsigned Index) const override {
// The first argument of an allocator call is the size of the allocation.
- if (Index == 0)
+ if (Index < getNumImplicitArgs())
return nullptr;
- return getOriginExpr()->getPlacementArg(Index - 1);
+ return getOriginExpr()->getPlacementArg(Index - getNumImplicitArgs());
+ }
+
+ const Expr *getPlacementArgExpr(unsigned Index) const {
+ return getOriginExpr()->getPlacementArg(Index);
}
Kind getKind() const override { return CE_CXXAllocator; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
