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 <benicsbal...@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@../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@+2 {{address of stack memory associated with local variable 'a' returned}} diagnosed by -Wreturn-stack-address + // expected-warning@+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@-2 {{Assuming the condition is false}} - // peaceful-note@-3 {{'?' condition is false}} -#else - // peaceful-note@-5 {{Assuming the condition is true}} - // peaceful-note@-6 {{'?' condition is true}} -#endif + // peaceful-note@-1 {{Assuming the condition is true}} + // peaceful-note@-2 {{'?' condition is true}} } // Same thing, but with a switch statement. { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
