fix(aggr): fix aggr decl with ExprWithCleanups

qqiangwu · qqiangwu · commit 0c346156203b · 2025-01-29T20:34:01.000+08:00
diff --git a/.github/workflows/ci-macos-local.yml b/.github/workflows/ci-macos-local.yml
@@ -12,7 +12,7 @@ jobs:
     strategy:
       matrix:
         build_type: [ Release ]
-        os: [macos-12]
+        os: [macos-latest]
         compiler:
           - cc: cc
             cxx: c++
@@ -32,6 +32,10 @@ jobs:
       with:
         fetch-depth: 0
 
+    - uses: actions/setup-python@v5
+      with:
+        python-version: '3.13'
+
     - name: Install llvm@17
       run: |
         brew install llvm@17
diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml
@@ -16,7 +16,7 @@ jobs:
     strategy:
       matrix:
         build_type: [ Release ]
-        os: [macos-12]
+        os: [macos-13, macos-14, macos-15]
         compiler:
           - cc: cc
             cxx: c++
@@ -34,6 +34,10 @@ jobs:
     steps:
     - uses: actions/checkout@v3
 
+    - uses: actions/setup-python@v5
+      with:
+        python-version: '3.13'
+
     - name: Install conan
       run: pip3 install conan
 
diff --git a/README.md b/README.md
@@ -70,6 +70,8 @@ Note that cppsafe will detect system includes via `c++`, you can override it via
 CXX=/opt/homebrew/opt/llvm/bin/clang cppsafe example.cpp -- -std=c++20
 ```
 
+> Note: cppship should be used with std17 or above, since cpp17 has changed the rule for temporaries.
+
 ### With compile\_commands.json
 Generally, you should use cppsafe with compile\_commands.json.
 
diff --git a/include/cppsafe/lifetime/LifetimePsetBuilder.h b/include/cppsafe/lifetime/LifetimePsetBuilder.h
@@ -32,6 +32,8 @@ class PSBuilder {
 
     DISALLOW_COPY_AND_MOVE(PSBuilder);
 
+    virtual const Expr* ignoreTransparentExprs(const Expr* E, bool IgnoreLValueToRValue = false) const = 0;
+
     virtual void setPSet(const Expr* E, const PSet& PS) = 0;
     virtual void setPSet(const PSet& LHS, PSet RHS, SourceRange Range) = 0;
 
diff --git a/integration_test/case/elements_view_cxx17.cpp b/integration_test/case/elements_view_cxx17.cpp
@@ -124,12 +124,12 @@ int main()
 
     __lifetime_type_category<T1>();  // expected-warning {{lifetime type category is Pointer with pointee const int}}
     __lifetime_type_category<T1_iter>();  // expected-warning {{lifetime type category is Pointer with pointee const int}}
-    __lifetime_type_category<T2>();  // expected-warning {{lifetime type category is Pointer with pointee std::string}}
-    __lifetime_type_category<T2_iter>();  // expected-warning {{lifetime type category is Pointer with pointee std::string}}
+    __lifetime_type_category<T2>();  // expected-warning {{lifetime type category is Pointer with pointee}}
+    __lifetime_type_category<T2_iter>();  // expected-warning {{lifetime type category is Pointer with pointee}}
     __lifetime_type_category<T3>();  // expected-warning {{lifetime type category is Pointer with pointee const int}}
     __lifetime_type_category<T3_iter>();  // expected-warning {{lifetime type category is Pointer with pointee const int}}
-    __lifetime_type_category<T4>();  // expected-warning {{lifetime type category is Pointer with pointee std::string}}
-    __lifetime_type_category<T4_iter>();  // expected-warning {{lifetime type category is Pointer with pointee std::string}}
+    __lifetime_type_category<T4>();  // expected-warning {{lifetime type category is Pointer with pointee}}
+    __lifetime_type_category<T4_iter>();  // expected-warning {{lifetime type category is Pointer with pointee}}
 
     map<int, string> mp{{1, "one"}, {2, "two"}, {3, "three"}, {4, "four"}};
 
diff --git a/integration_test/feature/structureal_binding.cpp b/integration_test/feature/structureal_binding.cpp
@@ -36,7 +36,7 @@ int main()
 
         const auto& [x3, y3] = get1();
         __lifetime_pset(x3);  // expected-warning {{(unknown)}}
-        __lifetime_pset(y3);  // expected-warning {{(unknown)}}
+        __lifetime_pset(y3);  // expected-warning {{(global)}}
 
         auto& [x4, y4] = get2(d);
         __lifetime_pset(x4);  // expected-warning {{(unknown)}}
diff --git a/integration_test/safety/contract_return.cpp b/integration_test/safety/contract_return.cpp
@@ -19,4 +19,17 @@ void test()
     Derive d;
     auto* p2 = d.foo(&x);
     __lifetime_pset(p2);  // expected-warning {{pset(p2) = (x)}}
+}
+
+int* get()
+{
+    static int x = 0;
+    return &x;
+}
+
+int* get2()
+{
+    static int x;
+    static int* y = &x;
+    return y;
 }
diff --git a/integration_test/safety/contract_return_aggr.cpp b/integration_test/safety/contract_return_aggr.cpp
@@ -10,7 +10,12 @@ struct A {
     Inner i;
 };
 
+struct B {
+    int* x;
+};
+
 A foo(int* x);
+B get();
 
 void test_contract_return()
 {
@@ -27,4 +32,72 @@ void test_contract_return()
     __lifetime_pset(m.y);  // expected-warning {{pset(m.y) = ((global))}}
     __lifetime_pset(m.i);  // expected-warning {{pset(m.i) = (m.i)}}
     __lifetime_pset(m.i.m);  // expected-warning {{pset(m.i.m) = (x)}}
+
+    A n = foo(get().x);
+    __lifetime_pset(n);  // expected-warning {{pset(n) = (n)}}
+    __lifetime_pset(n.x);  // expected-warning {{pset(n.x) = ((global))}}
+    __lifetime_pset(n.y);  // expected-warning {{pset(n.y) = ((global))}}
+    __lifetime_pset(n.i);  // expected-warning {{pset(n.i) = (n.i)}}
+    __lifetime_pset(n.i.m);  // expected-warning {{pset(n.i.m) = ((global))}}
+
+    int y = 0;
+    m = foo(&y);
+    __lifetime_pset(m);  // expected-warning {{pset(m) = (m)}}
+    __lifetime_pset(m.x);  // expected-warning {{pset(m.x) = (y)}}
+    __lifetime_pset(m.y);  // expected-warning {{pset(m.y) = ((global))}}
+    __lifetime_pset(m.i);  // expected-warning {{pset(m.i) = (m.i)}}
+    __lifetime_pset(m.i.m);  // expected-warning {{pset(m.i.m) = (y)}}
+
+    n = foo(B{&y}.x);
+    __lifetime_pset(n);  // expected-warning {{pset(n) = (n)}}
+    __lifetime_pset(n.x);  // expected-warning {{pset(n.x) = (y)}}
+    __lifetime_pset(n.y);  // expected-warning {{pset(n.y) = ((global))}}
+    __lifetime_pset(n.i);  // expected-warning {{pset(n.i) = (n.i)}}
+    __lifetime_pset(n.i.m);  // expected-warning {{pset(n.i.m) = (y)}}
+
+    n = foo(B{}.x);
+    __lifetime_pset(n);  // expected-warning {{pset(n) = (n)}}
+    __lifetime_pset(n.x);  // expected-warning {{pset(n.x) = ((global))}}
+    __lifetime_pset(n.y);  // expected-warning {{pset(n.y) = ((global))}}
+    __lifetime_pset(n.i);  // expected-warning {{pset(n.i) = (n.i)}}
+    __lifetime_pset(n.i.m);  // expected-warning {{pset(n.i.m) = ((global))}}
+
+    int* z = B{}.x;
+    __lifetime_pset(z);  // expected-warning {{pset(z) = ((null))}}
+}
+
+B createB()
+{
+    const B b { .x = nullptr };
+    return b;
+}
+
+B createStaticB()
+{
+    static int t = 0;
+    static const B b { .x = &t };
+    __lifetime_pmap();
+    return b;
+}
+
+struct [[gsl::Owner(char)]] String {};
+struct [[gsl::Pointer(char)]] StringView {
+    StringView(const String&);
+
+    void Foo();
+};
+
+struct Wrapper {
+    StringView s1;
+    StringView s2;
+};
+
+Wrapper get(StringView);
+
+void test_string_view()
+{
+    auto s = get(String{});  // expected-note {{temporary was destroyed at the end of the full expression}}
+    __lifetime_pset(s.s1);  // expected-warning {{pset(s.s1) = ((invalid))}}
+    __lifetime_pset(s.s2);  // expected-warning {{pset(s.s2) = ((invalid))}}
+    s.s1.Foo();  // expected-warning {{passing a dangling pointer as argument}}
 }
diff --git a/integration_test/safety/type_aggr_init.cpp b/integration_test/safety/type_aggr_init.cpp
@@ -51,6 +51,14 @@ void test_default_init()
     __lifetime_pset(b.c);  // expected-warning {{pset(b.c) = (*b.c)}}
 }
 
+void test_list_init()
+{
+    int t = 0;
+    Aggr a = {.y = &t };
+    __lifetime_pset(a);  // expected-warning {{pset(a) = (a)}}
+    __lifetime_pset(a.y);  // expected-warning {{pset(a.y) = (t)}}
+}
+
 void test_copy_init()
 {
     int m = 0;
diff --git a/lib/lifetime/LifetimePsetBuilder.cpp b/lib/lifetime/LifetimePsetBuilder.cpp
@@ -107,7 +107,7 @@ class PSetsBuilder final : public ConstStmtVisitor<PSetsBuilder, void>, public P
     /// Does not ignore MaterializeTemporaryExpr as Expr::IgnoreParenImpCasts
     /// would.
     // NOLINTBEGIN(readability-else-after-return)
-    static const Expr* ignoreTransparentExprs(const Expr* E, bool IgnoreLValueToRValue = false)
+    const Expr* ignoreTransparentExprs(const Expr* E, bool IgnoreLValueToRValue = false) const override
     {
         while (true) {
             E = E->IgnoreParens();
@@ -150,7 +150,7 @@ class PSetsBuilder final : public ConstStmtVisitor<PSetsBuilder, void>, public P
         // NOLINTEND(readability-else-after-return)
     }
 
-    static bool isIgnoredStmt(const Stmt* S)
+    bool isIgnoredStmt(const Stmt* S)
     {
         const Expr* E = dyn_cast<Expr>(S);
         return E && ignoreTransparentExprs(E) != E;
@@ -1068,14 +1068,19 @@ class PSetsBuilder final : public ConstStmtVisitor<PSetsBuilder, void>, public P
     // NOLINTNEXTLINE(readability-identifier-naming): required by parent
     void VisitVarDecl(const VarDecl* VD)
     {
-        if (const auto* DD = dyn_cast<DecompositionDecl>(VD)) {
+        if (const auto* DD = dyn_cast_if_present<DecompositionDecl>(VD)) {
             visitDecompositionDecl(DD);
             return;
         }
 
         const Expr* Initializer = VD->getInit();
         const SourceRange Range = VD->getSourceRange();
 
+        // NB. clang don't generage ExprWithCleanups node in CFG
+        if (const auto* E = dyn_cast_if_present<ExprWithCleanups>(Initializer)) {
+            Initializer = E->getSubExpr();
+        }
+
         switch (classifyTypeCategory(VD->getType())) {
         case TypeCategory::Pointer: {
             PSet PS;
diff --git a/lib/lifetime/type/Aggregate.cpp b/lib/lifetime/type/Aggregate.cpp
@@ -164,20 +164,21 @@ void handleAggregateCopy(const Expr* LHS, const Expr* RHS, PSBuilder& Builder)
     const auto OtherPS = Builder.getPSet(RHS);
     CPPSAFE_ASSERT(OtherPS.vars().size() <= 1);
 
-    const auto* Other = OtherPS.vars().empty() ? nullptr : &*OtherPS.vars().begin();
+    // if PSet(RHS) = {}, use it as a placeholder to derive members
+    const Variable Candidate(Builder.ignoreTransparentExprs(RHS));
+    const auto* Other = OtherPS.vars().empty() ? &Candidate : &*OtherPS.vars().begin();
     const auto* RD = LHS->getType()->getAsCXXRecordDecl();
     const Variable Base(LHS);
     expandAggregate(Base, RD,
         [&Base, &Builder, Other, &OtherPS](const Variable& LhsSubVar, const SubVarPath& Path, TypeClassification TC) {
-            if (!Other) {
-                Builder.setVarPSet(LhsSubVar, OtherPS);
-                return;
-            }
-
             const Variable RhsSubVar(Other->chainFields(Path));
             if (!TC.isPointer()) {
                 return;
             }
+            if (OtherPS.containsGlobal()) {
+                Builder.setVarPSet(LhsSubVar, PSet::globalVar());
+                return;
+            }
 
             auto PS = Builder.getVarPSet(RhsSubVar);
             CPPSAFE_ASSERT(PS);

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,14 @@ void test_default_init()`
`51`	`51`	`__lifetime_pset(b.c); // expected-warning {{pset(b.c) = (*b.c)}}`
`52`	`52`	`}`
`53`	`53`
	`54`	`+void test_list_init()`
	`55`	`+{`
	`56`	`+ int t = 0;`
	`57`	`+ Aggr a = {.y = &t };`
	`58`	`+ __lifetime_pset(a); // expected-warning {{pset(a) = (a)}}`
	`59`	`+ __lifetime_pset(a.y); // expected-warning {{pset(a.y) = (t)}}`
	`60`	`+}`
	`61`	`+`
`54`	`62`	`void test_copy_init()`
`55`	`63`	`{`
`56`	`64`	`int m = 0;`