[analyzer] Model builtin-like functions as builtin functions #99886
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 72315d0 After that change, the sentiment was that these special funtions should be considered and treated as builtin functions. Fixes #94193 CPP-5358
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balazs Benics (steakhal) ChangesSome template function instantiations don't have a body, even though their templates did have a body. They had bodies before After that change, the sentiment was that these special functions should be considered and treated as builtin functions. Fixes #94193 CPP-5358 Full diff: https://github.com/llvm/llvm-project/pull/99886.diff 7 Files Affected:
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.
{
|
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary: 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 72315d0 After that change, the sentiment was that these special functions should be considered and treated as builtin functions. Fixes #94193 CPP-5358 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251093
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some template function instantiations don't have a body, even though their templates did have a body.
Examples are:
std::move,std::forward,std::addressofetc.They had bodies before
72315d0
After that change, the sentiment was that these special functions should be considered and treated as builtin functions.
Fixes #94193
CPP-5358