[clang] [analyzer] Model builtin-like functions as builtin functions (PR #99886)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 07:38:45 PDT 2024
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/99886
Some template function instantiations don't have a body, even though their templates did have a body.
Examples are: `std::move`, `std::forward`, `std::addressof` etc.
They had bodies before
https://github.com/llvm/llvm-project/commit/72315d02c432a0fe0acae9c96c69eac8d8e1a9f6
After that change, the sentiment was that these special functions should be considered and treated as builtin functions.
Fixes #94193
CPP-5358
>From 3f25761041417de42330b348d46f35ac5af99426 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Mon, 22 Jul 2024 15:59:23 +0200
Subject: [PATCH] [analyzer] Model builtin-like functions as builtin functions
Some template function instantiations don't have a body, even though
their templates did have a body.
Examples are: `std::move`, `std::forward`, `std::addressof` etc.
They had bodies before
https://github.com/llvm/llvm-project/commit/72315d02c432a0fe0acae9c96c69eac8d8e1a9f6
After that change, the sentiment was that these special funtions
should be considered and treated as builtin functions.
Fixes #94193
CPP-5358
---
clang/docs/ReleaseNotes.rst | 4 ++
.../Checkers/BuiltinFunctionChecker.cpp | 38 +++++++++++++++++
.../Inputs/system-header-simulator-cxx.h | 14 ++++---
clang/test/Analysis/builtin-functions.cpp | 20 +++++++++
.../diagnostics/explicit-suppression.cpp | 2 +-
clang/test/Analysis/issue-94193.cpp | 41 +++++++++++++++++++
clang/test/Analysis/use-after-move.cpp | 9 +---
7 files changed, 114 insertions(+), 14 deletions(-)
create mode 100644 clang/test/Analysis/issue-94193.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 61629ff30aeeb..4ddb6baa49824 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1326,6 +1326,10 @@ Crash and bug fixes
- Z3 crosschecking (aka. Z3 refutation) is now bounded, and can't consume
more total time than the eymbolic execution itself. (#GH97298)
+- ``std::addressof``, ``std::as_const``, ``std::forward``,
+ ``std::forward_like``, ``std::move``, ``std::move_if_noexcept``, are now
+ modeled just like their builtin counterpart. (#GH94193)
+
Improvements
^^^^^^^^^^^^
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 1a75d7b52ad6e..b198b1c2ff4d1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -18,6 +18,7 @@
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
@@ -30,8 +31,40 @@ namespace {
class BuiltinFunctionChecker : public Checker<eval::Call> {
public:
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+ // From: clang/include/clang/Basic/Builtins.def
+ // C++ standard library builtins in namespace 'std'.
+ const CallDescriptionSet BuiltinLikeStdFunctions{
+ {CDM::SimpleFunc, {"std", "addressof"}}, //
+ {CDM::SimpleFunc, {"std", "__addressof"}}, //
+ {CDM::SimpleFunc, {"std", "as_const"}}, //
+ {CDM::SimpleFunc, {"std", "forward"}}, //
+ {CDM::SimpleFunc, {"std", "forward_like"}}, //
+ {CDM::SimpleFunc, {"std", "move"}}, //
+ {CDM::SimpleFunc, {"std", "move_if_noexcept"}}, //
+ };
+
+ bool isBuiltinLikeFunction(const CallEvent &Call) const;
};
+} // namespace
+
+bool BuiltinFunctionChecker::isBuiltinLikeFunction(
+ const CallEvent &Call) const {
+ const auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+ if (!FD || FD->getNumParams() != 1)
+ return false;
+
+ if (QualType RetTy = FD->getReturnType();
+ !RetTy->isPointerType() && !RetTy->isReferenceType())
+ return false;
+
+ if (QualType ParmTy = FD->getParamDecl(0)->getType();
+ !ParmTy->isPointerType() && !ParmTy->isReferenceType())
+ return false;
+
+ return BuiltinLikeStdFunctions.contains(Call);
}
bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
@@ -44,6 +77,11 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
const LocationContext *LCtx = C.getLocationContext();
const Expr *CE = Call.getOriginExpr();
+ if (isBuiltinLikeFunction(Call)) {
+ C.addTransition(state->BindExpr(CE, LCtx, Call.getArgSVal(0)));
+ return true;
+ }
+
switch (FD->getBuiltinID()) {
default:
return false;
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index 29326ec1f9280..a379a47515668 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -263,11 +263,13 @@ namespace std {
template< class T > struct remove_reference<T&> {typedef T type;};
template< class T > struct remove_reference<T&&> {typedef T type;};
- template<class T>
- typename remove_reference<T>::type&& move(T&& a) {
- typedef typename remove_reference<T>::type&& RvalRef;
- return static_cast<RvalRef>(a);
- }
+ template<typename T> typename remove_reference<T>::type&& move(T&& a);
+ template <typename T> T *__addressof(T &x);
+ template <typename T> T *addressof(T &x);
+ template <typename T> const T& as_const(T& x);
+ template <typename T> T&& forward(T&& x);
+ // FIXME: Declare forward_like
+ // FIXME: Declare move_if_noexcept
template< class T >
using remove_reference_t = typename remove_reference<T>::type;
@@ -754,7 +756,7 @@ namespace std {
template<class InputIter, class OutputIter>
OutputIter __copy(InputIter II, InputIter IE, OutputIter OI) {
while (II != IE)
- *OI++ = *II++;
+ *OI++ = *II++; // #system_header_simulator_cxx_std_copy_impl_loop
return OI;
}
diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp
index 8719193e405c4..f7bafabd23cbc 100644
--- a/clang/test/Analysis/builtin-functions.cpp
+++ b/clang/test/Analysis/builtin-functions.cpp
@@ -1,6 +1,16 @@
// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify
// RUN: %clang_analyze_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace std {
+// Intentionally not using an "auto" return type here, as such must also have a definition.
+template <typename T, typename U> constexpr int&& forward_like(U&& x) noexcept;
+template <typename T> const T& move_if_noexcept(T& x) noexcept;
+} // namespace std
+
+template <typename T> void clang_analyzer_dump_ref(T&&);
+template <typename T> void clang_analyzer_dump_ptr(T*);
void clang_analyzer_eval(bool);
void clang_analyzer_warnIfReached();
@@ -8,6 +18,16 @@ void testAddressof(int x) {
clang_analyzer_eval(&x == __builtin_addressof(x)); // expected-warning{{TRUE}}
}
+void testStdBuiltinLikeFunctions(int x) {
+ clang_analyzer_dump_ptr(std::addressof(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ptr(std::__addressof(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ref(std::as_const(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ref(std::forward<int &>(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ref(std::forward_like<int &>(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ref(std::move(x)); // expected-warning{{&x}}
+ clang_analyzer_dump_ref(std::move_if_noexcept(x)); // expected-warning{{&x}}
+}
+
void testSize() {
struct {
int x;
diff --git a/clang/test/Analysis/diagnostics/explicit-suppression.cpp b/clang/test/Analysis/diagnostics/explicit-suppression.cpp
index 24586e37fe207..3a904dac1c0fe 100644
--- a/clang/test/Analysis/diagnostics/explicit-suppression.cpp
+++ b/clang/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@ class C {
void testCopyNull(C *I, C *E) {
std::copy(I, E, (C *)0);
#ifndef SUPPRESSED
- // expected-warning at ../Inputs/system-header-simulator-cxx.h:757 {{Called C++ object pointer is null}}
+ // expected-warning@#system_header_simulator_cxx_std_copy_impl_loop {{Called C++ object pointer is null}}
#endif
}
diff --git a/clang/test/Analysis/issue-94193.cpp b/clang/test/Analysis/issue-94193.cpp
new file mode 100644
index 0000000000000..97acfdd8685be
--- /dev/null
+++ b/clang/test/Analysis/issue-94193.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_analyze_cc1 %s -verify -analyzer-checker=core
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+
+namespace GH94193 {
+template<typename T> class optional {
+ union {
+ char x;
+ T uvalue;
+ };
+ bool holds_value = false;
+public:
+ optional() = default;
+ optional(const optional&) = delete;
+ optional(optional&&) = delete;
+ template <typename U = T> explicit optional(U&& value) : holds_value(true) {
+ new (static_cast<void*>(std::addressof(uvalue))) T(std::forward<U>(value));
+ }
+ optional& operator=(const optional&) = delete;
+ optional& operator=(optional&&) = delete;
+ explicit operator bool() const {
+ return holds_value;
+ }
+ T& unwrap() & {
+ return uvalue; // no-warning: returns a valid value
+ }
+};
+
+int top1(int x) {
+ optional<int> opt{x}; // note: Ctor was inlined.
+ return opt.unwrap(); // no-warning: returns a valid value
+}
+
+std::string *top2() {
+ std::string a = "123";
+ // expected-warning at +2 {{address of stack memory associated with local variable 'a' returned}} diagnosed by -Wreturn-stack-address
+ // expected-warning at +1 {{Address of stack memory associated with local variable 'a' returned to caller [core.StackAddressEscape]}}
+ return std::addressof(a);
+}
+} // namespace GH94193
diff --git a/clang/test/Analysis/use-after-move.cpp b/clang/test/Analysis/use-after-move.cpp
index 33980e6ea2b8b..24d5dd8a8b3d2 100644
--- a/clang/test/Analysis/use-after-move.cpp
+++ b/clang/test/Analysis/use-after-move.cpp
@@ -570,13 +570,8 @@ void differentBranchesTest(int i) {
{
A a;
a.foo() > 0 ? a.foo() : A(std::move(a)).foo();
-#ifdef DFS
- // peaceful-note at -2 {{Assuming the condition is false}}
- // peaceful-note at -3 {{'?' condition is false}}
-#else
- // peaceful-note at -5 {{Assuming the condition is true}}
- // peaceful-note at -6 {{'?' condition is true}}
-#endif
+ // peaceful-note at -1 {{Assuming the condition is true}}
+ // peaceful-note at -2 {{'?' condition is true}}
}
// Same thing, but with a switch statement.
{
More information about the cfe-commits
mailing list