From b925806e216dcdbb359852ba6fef59268f6d5fe5 Mon Sep 17 00:00:00 2001 From: miaozhiyuan Date: Fri, 1 Mar 2024 22:45:20 +0800 Subject: [PATCH 1/3] [clang][ExprEngineCXX] Fix crash on dereference invalid return value of getAdjustedParameterIndex() --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 9 +++++++-- .../Analysis/engine/expr-engine-cxx-crash.cpp | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/engine/expr-engine-cxx-crash.cpp diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 504fd7f05e0f9..dc72945d68d56 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -354,8 +354,13 @@ SVal ExprEngine::computeObjectUnderConstruction( // Operator arguments do not correspond to operator parameters // because this-argument is implemented as a normal argument in // operator call expressions but not in operator declarations. - const TypedValueRegion *TVR = Caller->getParameterLocation( - *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount()); + std::optional Index = + Caller->getAdjustedParameterIndex(Idx); + if (!Index) { + return std::nullopt; + } + const TypedValueRegion *TVR = + Caller->getParameterLocation(*Index, BldrCtx->blockCount()); if (!TVR) return std::nullopt; diff --git a/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp b/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp new file mode 100644 index 0000000000000..a7d2e32db6eb6 --- /dev/null +++ b/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core.DivideZero -std=c++23 -verify %s +// expected-no-diagnostics + +struct S +{ + constexpr auto operator==(this auto, S) + { + return true; + } +}; + +int main() +{ + return S {} == S {}; +} From 5ac52a8fda78fd8308b95c244a9a4b46d9d8121a Mon Sep 17 00:00:00 2001 From: miaozhiyuan Date: Tue, 5 Mar 2024 23:24:07 +0800 Subject: [PATCH 2/3] fix testcase and fix root reason about crash with tomasz-kaminski-sonarsource's help --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 2 +- .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 9 ++------- .../Analysis/engine/expr-engine-cxx-crash.cpp | 18 +++++++----------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 0ac1d91b79beb..bc14aea27f673 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -1409,7 +1409,7 @@ CallEventManager::getSimpleCall(const CallExpr *CE, ProgramStateRef State, if (const auto *OpCE = dyn_cast(CE)) { const FunctionDecl *DirectCallee = OpCE->getDirectCallee(); if (const auto *MD = dyn_cast(DirectCallee)) - if (MD->isInstance()) + if (MD->isImplicitObjectMemberFunction()) return create(OpCE, State, LCtx, ElemRef); } else if (CE->getCallee()->getType()->isBlockPointerType()) { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index dc72945d68d56..504fd7f05e0f9 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -354,13 +354,8 @@ SVal ExprEngine::computeObjectUnderConstruction( // Operator arguments do not correspond to operator parameters // because this-argument is implemented as a normal argument in // operator call expressions but not in operator declarations. - std::optional Index = - Caller->getAdjustedParameterIndex(Idx); - if (!Index) { - return std::nullopt; - } - const TypedValueRegion *TVR = - Caller->getParameterLocation(*Index, BldrCtx->blockCount()); + const TypedValueRegion *TVR = Caller->getParameterLocation( + *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount()); if (!TVR) return std::nullopt; diff --git a/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp b/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp index a7d2e32db6eb6..2dfeac8392266 100644 --- a/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp +++ b/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp @@ -1,15 +1,11 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core.DivideZero -std=c++23 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core -std=c++23 -verify %s // expected-no-diagnostics -struct S -{ - constexpr auto operator==(this auto, S) - { - return true; - } +struct S { + bool operator==(this auto, S) { + return true; + } }; - -int main() -{ - return S {} == S {}; +int use_deducing_this() { + return S{} == S{}; } From f9779b3a0a818b159cd9dc54fb5fd01d5b6ccaf6 Mon Sep 17 00:00:00 2001 From: miaozhiyuan Date: Wed, 6 Mar 2024 22:58:59 +0800 Subject: [PATCH 3/3] move test into cxx2b-deducing-this.cpp --- clang/test/Analysis/cxx2b-deducing-this.cpp | 11 +++++++++++ clang/test/Analysis/engine/expr-engine-cxx-crash.cpp | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) delete mode 100644 clang/test/Analysis/engine/expr-engine-cxx-crash.cpp diff --git a/clang/test/Analysis/cxx2b-deducing-this.cpp b/clang/test/Analysis/cxx2b-deducing-this.cpp index d22a897097bec..2ec9e96bf0f84 100644 --- a/clang/test/Analysis/cxx2b-deducing-this.cpp +++ b/clang/test/Analysis/cxx2b-deducing-this.cpp @@ -60,3 +60,14 @@ void top() { s.c(); s.c(11); } + + +struct S2 { + bool operator==(this auto, S2) { + return true; + } +}; +void use_deducing_this() { + int result = S2{} == S2{}; // no-crash + clang_analyzer_dump(result); // expected-warning {{1 S32b}} +} diff --git a/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp b/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp deleted file mode 100644 index 2dfeac8392266..0000000000000 --- a/clang/test/Analysis/engine/expr-engine-cxx-crash.cpp +++ /dev/null @@ -1,11 +0,0 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core -std=c++23 -verify %s -// expected-no-diagnostics - -struct S { - bool operator==(this auto, S) { - return true; - } -}; -int use_deducing_this() { - return S{} == S{}; -}