NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus,
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet.
Herald added a project: clang.
NoQ retitled this revision from "[analyzer] CallDescription: Check number of
parameters as well as number of arguments." to "[analyzer] pr43179:
CallDescription: Check number of parameters as well as number of arguments.".
NoQ edited the summary of this revision.
This is https://bugs.llvm.org/show_bug.cgi?id=43179.
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 then 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.
Repository:
rC Clang
https://reviews.llvm.org/D67019
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/test/Analysis/cast-value-weird.cpp
Index: clang/test/Analysis/cast-value-weird.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/cast-value-weird.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling -verify %s
+
+// expected-no-diagnostics
+
+namespace llvm {
+template <typename>
+void cast(...);
+void a() { cast<int>(int()); } // no-crash
+} // namespace llvm
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -368,7 +368,8 @@
if (CD.Flags & CDF_MaybeBuiltin) {
return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
- (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs());
+ (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
}
if (!CD.IsLookupDone) {
@@ -407,7 +408,8 @@
return false;
}
- return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs());
+ return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams == parameters().size());
}
SVal CallEvent::getArgSVal(unsigned Index) const {
Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -116,7 +116,9 @@
// vswprintf is the wide version of vsnprintf,
// vsprintf has no wide version
{{"vswscanf", 3}, 2}};
-const CallDescription ValistChecker::VaStart("__builtin_va_start", 2),
+
+const CallDescription
+ ValistChecker::VaStart("__builtin_va_start", /*Args=*/2, /*Params=*/1),
ValistChecker::VaCopy("__builtin_va_copy", 2),
ValistChecker::VaEnd("__builtin_va_end", 1);
} // end anonymous namespace
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1064,8 +1064,19 @@
// e.g. "{a, b}" represent the qualified names, like "a::b".
std::vector<const char *> QualifiedName;
Optional<unsigned> RequiredArgs;
+ Optional<size_t> RequiredParams;
int Flags;
+ // A constructor helper.
+ static Optional<size_t> readRequiredParams(Optional<unsigned> RequiredArgs,
+ Optional<size_t> RequiredParams) {
+ if (RequiredParams)
+ return RequiredParams;
+ if (RequiredArgs)
+ return static_cast<size_t>(*RequiredArgs);
+ return None;
+ }
+
public:
/// Constructs a CallDescription object.
///
@@ -1078,14 +1089,17 @@
/// call. Omit this parameter to match every occurrence of call with a given
/// name regardless the number of arguments.
CallDescription(int Flags, ArrayRef<const char *> QualifiedName,
- Optional<unsigned> RequiredArgs = None)
+ Optional<unsigned> RequiredArgs = None,
+ Optional<size_t> RequiredParams = None)
: QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
+ RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
Flags(Flags) {}
/// Construct a CallDescription with default flags.
CallDescription(ArrayRef<const char *> QualifiedName,
- Optional<unsigned> RequiredArgs = None)
- : CallDescription(0, QualifiedName, RequiredArgs) {}
+ Optional<unsigned> RequiredArgs = None,
+ Optional<size_t> RequiredParams = None)
+ : CallDescription(0, QualifiedName, RequiredArgs, RequiredParams) {}
/// Get the name of the function that this object matches.
StringRef getFunctionName() const { return QualifiedName.back(); }
Index: clang/test/Analysis/cast-value-weird.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/cast-value-weird.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling -verify %s
+
+// expected-no-diagnostics
+
+namespace llvm {
+template <typename>
+void cast(...);
+void a() { cast<int>(int()); } // no-crash
+} // namespace llvm
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -368,7 +368,8 @@
if (CD.Flags & CDF_MaybeBuiltin) {
return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
- (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs());
+ (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
}
if (!CD.IsLookupDone) {
@@ -407,7 +408,8 @@
return false;
}
- return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs());
+ return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams == parameters().size());
}
SVal CallEvent::getArgSVal(unsigned Index) const {
Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -116,7 +116,9 @@
// vswprintf is the wide version of vsnprintf,
// vsprintf has no wide version
{{"vswscanf", 3}, 2}};
-const CallDescription ValistChecker::VaStart("__builtin_va_start", 2),
+
+const CallDescription
+ ValistChecker::VaStart("__builtin_va_start", /*Args=*/2, /*Params=*/1),
ValistChecker::VaCopy("__builtin_va_copy", 2),
ValistChecker::VaEnd("__builtin_va_end", 1);
} // end anonymous namespace
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1064,8 +1064,19 @@
// e.g. "{a, b}" represent the qualified names, like "a::b".
std::vector<const char *> QualifiedName;
Optional<unsigned> RequiredArgs;
+ Optional<size_t> RequiredParams;
int Flags;
+ // A constructor helper.
+ static Optional<size_t> readRequiredParams(Optional<unsigned> RequiredArgs,
+ Optional<size_t> RequiredParams) {
+ if (RequiredParams)
+ return RequiredParams;
+ if (RequiredArgs)
+ return static_cast<size_t>(*RequiredArgs);
+ return None;
+ }
+
public:
/// Constructs a CallDescription object.
///
@@ -1078,14 +1089,17 @@
/// call. Omit this parameter to match every occurrence of call with a given
/// name regardless the number of arguments.
CallDescription(int Flags, ArrayRef<const char *> QualifiedName,
- Optional<unsigned> RequiredArgs = None)
+ Optional<unsigned> RequiredArgs = None,
+ Optional<size_t> RequiredParams = None)
: QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
+ RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
Flags(Flags) {}
/// Construct a CallDescription with default flags.
CallDescription(ArrayRef<const char *> QualifiedName,
- Optional<unsigned> RequiredArgs = None)
- : CallDescription(0, QualifiedName, RequiredArgs) {}
+ Optional<unsigned> RequiredArgs = None,
+ Optional<size_t> RequiredParams = None)
+ : CallDescription(0, QualifiedName, RequiredArgs, RequiredParams) {}
/// Get the name of the function that this object matches.
StringRef getFunctionName() const { return QualifiedName.back(); }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
