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

[Gábor Horváth via llvm-branch-commits]

================
@@ -274,6 +289,21 @@ void FactsGenerator::VisitMemberExpr(const MemberExpr *ME) {
         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");
+    OriginNode *NewUsedOrigins =
+        doesDeclHaveStorage(FD) ? Dst->getPointeeChild() : Dst;
+    UF->setUsedOrigins(NewUsedOrigins);
+    UseFacts[ME] = UF;
+    UseFacts.erase(ME->getBase());
----------------
Xazax-hun wrote:

I think this hack of having to erase the base would go away if we tracked uses based on value categories. 

I.e., if we only considered a use when there is an lvalue to rvalue conversion, I suspect that when we have a field access like `auto x = foo.bar` we would only have a reference to `foo` as an lvalue, and we would have an lvalue to rvalue conversion for `bar` only. 

I think it is fine to leave this as is for now but it looks like some places are popping up where the value category based liveness would simplify thing a bit.

@usx95 what do you think?

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