https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/94983
The patch includes a repro for a case where we were returning a null `FieldDecl`
when calling `getReferencedDecls()` on the `InitListExpr` for a union.
Also, I noticed while working on this that `RecordInitListHelper` has a bug
where it doesn't work correctly for empty unions. This patch also includes a
repro and fix for this bug.
>From d1e0d9b72dc818eab6fc9463c54c0b5d7a61d5dd Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboe...@google.com>
Date: Mon, 10 Jun 2024 14:11:41 +0000
Subject: [PATCH] [clang][nullability] Don't return null fields from
`getReferencedDecls()`.
The patch includes a repro for a case where we were returning a null `FieldDecl`
when calling `getReferencedDecls()` on the `InitListExpr` for a union.
Also, I noticed while working on this that `RecordInitListHelper` has a bug
where it doesn't work correctly for empty unions. This patch also includes a
repro and fix for this bug.
---
clang/docs/tools/clang-formatted-files.txt | 1 +
clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 11 ++-
.../Analysis/FlowSensitive/ASTOpsTest.cpp | 88 +++++++++++++++++++
.../Analysis/FlowSensitive/CMakeLists.txt | 1 +
.../unittests/Analysis/FlowSensitive/BUILD.gn | 1 +
5 files changed, 98 insertions(+), 4 deletions(-)
create mode 100644 clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
diff --git a/clang/docs/tools/clang-formatted-files.txt
b/clang/docs/tools/clang-formatted-files.txt
index dee51e402b687..4866bd4aee634 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -622,6 +622,7 @@ clang/tools/libclang/CXCursor.h
clang/tools/scan-build-py/tests/functional/src/include/clean-one.h
clang/unittests/Analysis/CFGBuildResult.h
clang/unittests/Analysis/MacroExpansionContextTest.cpp
+clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
clang/unittests/Analysis/FlowSensitive/CNFFormula.cpp
clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
index 38b5f51b7b2f0..27d42a7b50856 100644
--- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
@@ -100,7 +100,8 @@ getFieldsForInitListExpr(const InitListT *InitList) {
std::vector<const FieldDecl *> Fields;
if (InitList->getType()->isUnionType()) {
- Fields.push_back(InitList->getInitializedFieldInUnion());
+ if (const FieldDecl *Field = InitList->getInitializedFieldInUnion())
+ Fields.push_back(Field);
return Fields;
}
@@ -137,9 +138,11 @@ RecordInitListHelper::RecordInitListHelper(
// it doesn't do this -- so we create an `ImplicitValueInitExpr` ourselves.
SmallVector<Expr *> InitsForUnion;
if (Ty->isUnionType() && Inits.empty()) {
- assert(Fields.size() == 1);
- ImplicitValueInitForUnion.emplace(Fields.front()->getType());
- InitsForUnion.push_back(&*ImplicitValueInitForUnion);
+ assert(Fields.size() <= 1);
+ if (!Fields.empty()) {
+ ImplicitValueInitForUnion.emplace(Fields.front()->getType());
+ InitsForUnion.push_back(&*ImplicitValueInitForUnion);
+ }
Inits = InitsForUnion;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
new file mode 100644
index 0000000000000..cd1c076ab09e6
--- /dev/null
+++ b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
@@ -0,0 +1,88 @@
+//===- unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
--------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/ASTOps.h"
+#include "TestingSupport.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <memory>
+
+namespace {
+
+using namespace clang;
+using namespace dataflow;
+
+using ast_matchers::cxxRecordDecl;
+using ast_matchers::hasName;
+using ast_matchers::hasType;
+using ast_matchers::initListExpr;
+using ast_matchers::match;
+using ast_matchers::selectFirst;
+using test::findValueDecl;
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+TEST(ASTOpsTest, RecordInitListHelperOnEmptyUnionInitList) {
+ // This is a regression test: The `RecordInitListHelper` used to assert-fail
+ // when called for the `InitListExpr` of an empty union.
+ std::string Code = R"cc(
+ struct S {
+ S() : UField{} {};
+
+ union U {} UField;
+ };
+ )cc";
+ std::unique_ptr<ASTUnit> Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+ auto &ASTCtx = Unit->getASTContext();
+
+ ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ auto *InitList = selectFirst<InitListExpr>(
+ "init",
+ match(initListExpr(hasType(cxxRecordDecl(hasName("U")))).bind("init"),
+ ASTCtx));
+ ASSERT_NE(InitList, nullptr);
+
+ RecordInitListHelper Helper(InitList);
+ EXPECT_THAT(Helper.base_inits(), IsEmpty());
+ EXPECT_THAT(Helper.field_inits(), IsEmpty());
+}
+
+TEST(ASTOpsTest, ReferencedDeclsOnUnionInitList) {
+ // This is a regression test: `getReferencedDecls()` used to return a null
+ // `FieldDecl` in this case (in addition to the correct non-null `FieldDecl`)
+ // because `getInitializedFieldInUnion()` returns null for the syntactic form
+ // of the `InitListExpr`.
+ std::string Code = R"cc(
+ struct S {
+ S() : UField{0} {};
+
+ union U {
+ int I;
+ } UField;
+ };
+ )cc";
+ std::unique_ptr<ASTUnit> Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+ auto &ASTCtx = Unit->getASTContext();
+
+ ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ auto *InitList = selectFirst<InitListExpr>(
+ "init",
+ match(initListExpr(hasType(cxxRecordDecl(hasName("U")))).bind("init"),
+ ASTCtx));
+ ASSERT_NE(InitList, nullptr);
+ auto *IDecl = cast<FieldDecl>(findValueDecl(ASTCtx, "I"));
+
+ EXPECT_THAT(getReferencedDecls(*InitList).Fields,
+ UnorderedElementsAre(IDecl));
+}
+
+} // namespace
diff --git a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
index cfabb80576bc1..12fee5dc2789c 100644
--- a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_unittest(ClangAnalysisFlowSensitiveTests
ArenaTest.cpp
+ ASTOpsTest.cpp
CFGMatchSwitchTest.cpp
ChromiumCheckModelTest.cpp
DataflowAnalysisContextTest.cpp
diff --git
a/llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn
b/llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn
index e16ca31b81a8d..780a69f1f3299 100644
--- a/llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn
@@ -18,6 +18,7 @@ unittest("ClangAnalysisFlowSensitiveTests") {
"//llvm/lib/Testing/Support",
]
sources = [
+ "ASTOpsTest.cpp",
"ArenaTest.cpp",
"CFGMatchSwitchTest.cpp",
"ChromiumCheckModelTest.cpp",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
