[analyzer] Fix uninitialized base class with initializer list when ctor is not declared in the base class (#70464) #70792
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ella Ma (Snape3058) ChangesWhen ctor is not declared in the base class, initializing the base class with the initializer list will not trigger a proper assignment of the base region, as a CXXConstructExpr doing that is not available in the AST. This patch checks whether the init expr is an InitListExpr under a base initializer, and adds a binding if so. Full diff: https://github.com/llvm/llvm-project/pull/70792.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 2e67fb953e45611..78dd1147ce1aa7c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1222,6 +1222,15 @@ void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame);
evalBind(Tmp, Init, Pred, FieldLoc, InitVal, /*isInit=*/true, &PP);
}
+ } else if (BMI->isBaseInitializer() && isa<InitListExpr>(Init)) {
+ // When the base class is initialized with an initialization list, there
+ // will not be a CXXConstructExpr to initialize the base region. Hence, we
+ // need to make the bind for it.
+ StoreManager &StoreMgr = State->getStateManager().getStoreManager();
+ SVal BaseLoc = StoreMgr.evalDerivedToBase(
+ thisVal, QualType(BMI->getBaseClass(), 0), BMI->isBaseVirtual());
+ SVal InitVal = State->getSVal(Init, stackFrame);
+ evalBind(Tmp, Init, Pred, BaseLoc, InitVal, true);
} else {
assert(BMI->isBaseInitializer() || BMI->isDelegatingInitializer());
Tmp.insert(Pred);
diff --git a/clang/test/Analysis/issue-70464.cpp b/clang/test/Analysis/issue-70464.cpp
new file mode 100644
index 000000000000000..0acced7ae102cac
--- /dev/null
+++ b/clang/test/Analysis/issue-70464.cpp
@@ -0,0 +1,69 @@
+// Refer issue 70464 for more details.
+//
+// When the base class does not have a declared constructor, the base
+// initializer in the constructor of the derived class should use the given
+// initializer list to finish the initialization of the base class.
+//
+// Also testing base constructor and delegate constructor to make sure this
+// change will not break these two cases when a CXXConstructExpr is available.
+
+// RUN: %clang_analyze_cc1 %s -verify -analyzer-checker=core,debug.ExprInspection
+
+void clang_analyzer_dump(int);
+
+namespace init_list {
+
+struct foo {
+ int foox;
+};
+
+class bar : public foo {
+public:
+ bar() : foo{42} {
+ // The dereference to this->foox below should be initialized properly.
+ clang_analyzer_dump(this->foox); // expected-warning{{42 S32b}}
+ }
+};
+
+void entry() { bar test; }
+
+} // namespace init_list
+
+namespace base_ctor_call {
+
+void clang_analyzer_dump(int);
+
+struct foo {
+ int foox;
+ foo(int x) : foox(x) {}
+};
+
+class bar : public foo {
+public:
+ bar() : foo{42} {
+ clang_analyzer_dump(this->foox); // expected-warning{{42 S32b}}
+ }
+};
+
+void entry() { bar test; }
+
+} // namespace base_ctor_call
+
+namespace delegate_ctor_call {
+
+void clang_analyzer_dump(int);
+
+struct foo {
+ int foox;
+};
+
+struct bar : foo {
+ bar(int parx) { this->foox = parx; }
+ bar() : bar{42} {
+ clang_analyzer_dump(this->foox); // expected-warning{{42 S32b}}
+ }
+};
+
+void entry() { bar test; }
+
+} // namespace delegate_ctor_call
|
Snape3058
force-pushed
the
issue-70464
branch
from
8600276 to
fa7f1d1
Compare
There was a problem hiding this comment.
Looks great!
FYI when submitting patches to GH try not to force-push to help the UI for following lines having comments. Otherwise, they will be marked as "outdated" and become hard to dig up and relate to new line locations. This was easy this time, as the size of the PR was minimal (as it should be).
After the PR is approved and all conversations resolved; the author can either manually force-push to squash the commits or use the "squash-merge" to let GH do it.
…or is not declared in the base class Fixes llvm#70464 When ctor is not declared in the base class, initializing the base class with the initializer list will not trigger a proper assignment of the base region, as a CXXConstructExpr doing that is not available in the AST. This patch checks whether the init expr is an InitListExpr under a base initializer, and adds a binding if so.
Snape3058
force-pushed
the
issue-70464
branch
from
fa7f1d1 to
afda561
Compare
|
As #59493 is an array, which is different from the test case I provided and the ones in #61919 and #54533, // Additional test case from issue #59493
namespace init_list_array {
struct Base {
int foox[1];
};
class Derived : public Base {
public:
Derived() : Base{{42}} {
// The dereference to this->foox below should be initialized properly.
clang_analyzer_dump(this->foox[0]); // expected-warning{{42 S32b}}
clang_analyzer_dump(foox[0]); // expected-warning{{42 S32b}}
}
};
void entry() { Derived test; }
} // namespace init_list_array |
|
Do I need to
Which operation is correct? (Sorry for not familiar with GitHub) -( |
I'd prefer a new PR, and mentioning the original PR (the fix) and the issue for which it adds the test. |
|
OK, thanks~ |
When ctor is not declared in the base class, initializing the base class with the initializer list will not trigger a proper assignment of the base region, as a CXXConstructExpr doing that is not available in the AST.
This patch checks whether the init expr is an InitListExpr under a base initializer, and adds a binding if so.