[llvm-branch-commits] [clang] [LifetimeSafety] Track per-field origins for record types (PR #195603)

[Zhijie Wang via llvm-branch-commits]

================
@@ -273,6 +284,19 @@ void FactsGenerator::VisitMemberExpr(const MemberExpr *ME) {
     CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
         Dst->getOriginID(), Src->getOriginID(),
         /*Kill=*/true));
+
+    // Narrow the UseFact's liveness coverage to the accessed field's
+    // subtree.
+    //
+    // E.g., for `(void)s.inner`, without narrowing, the UseFact at `s`
+    // would keep `s.v`'s subtree live and falsely flag a UAF when a loan
+    // held by `s.v` has already expired.
+    if (UseFact *UF = UseFacts.lookup(ME->getBase())) {
+      assert(!UseFacts.contains(ME) && "ME already has a UseFact");
+      UF->setUsedOrigins(Dst);
+      UseFacts[ME] = UF;
+      UseFacts.erase(ME->getBase());
+    }
----------------
aeft wrote:

The corresponding tests are `struct_field_safe` and `struct_field_safe2`.   
  
But it seems like this is not a good and general solution. The core problem here is that I extended the UseFact transfer function from pointee chain traversal to tree traversal. And in some cases (one example is `derived_field_safe`), the remedy doesn't work. I may add a more specific fix here (e.g., fix `derived_field_safe`), but this is more like a band-aid than a principled design. I am thinking whether the long-term solution is that we carefully decide which UseFact to emit.  
  
For example, to avoid FP in `struct_field_safe`, instead of emitting one broad UseFact at `s` and relying on this narrow logic to rewrite it to `s.inner`, each visitor would emit its own narrow UseFact: `VisitDeclRefExpr` emits one at `s`'s origin, `VisitMemberExpr(s.inner)` emits another at `s.inner`'s origin, and the transfer function stops recursing into the subtree. The two UseFacts coexist; siblings of `s.inner` are never targeted by any UseFact, so the FP disappears by construction.

https://github.com/llvm/llvm-project/pull/195603