[clang] [clang][dataflow] Fix crash when `operator=` result type is not destination type. (PR #90898)

via cfe-commits cfe-commits at lists.llvm.org
Sun May 5 23:13:33 PDT 2024


================
@@ -556,14 +556,23 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
 
       copyRecord(*LocSrc, *LocDst, Env);
 
-      // If the expr is a glvalue, we can reasonably assume the operator is
-      // returning T& and thus we can assign it `LocDst`.
-      if (S->isGLValue()) {
+      // The assignment operator can have an arbitrary return type. We model the
+      // return value only if the return type is the same as or a base class of
+      // the destination type.
+      if (S->getType().getCanonicalType().getUnqualifiedType() !=
+          LocDst->getType().getCanonicalType().getUnqualifiedType()) {
+        auto ReturnDecl = S->getType()->getAsCXXRecordDecl();
+        auto DstDecl = LocDst->getType()->getAsCXXRecordDecl();
+        if (ReturnDecl == nullptr || DstDecl == nullptr)
+          return;
+        if (!DstDecl->isDerivedFrom(ReturnDecl))
----------------
martinboehme wrote:

I think it's consistent with our more general approach -- we treat "storage location not set" as meaning "we have no information" [^1]. For example, if above we discover that this isn't a copy or move assignment operator, we bail out without doing anything.

Granted, this case is a bit different in that we have already modeled the assignment itself (by doing the `copyRecord()`) above, but I think it's still consistent to do this but decide we don't know how to model the return value / location.

[^1]: Of course, a fresh storage location that isn't otherwise constrained expresses essentially the same thing, but it's simpler to instead not assign a storage location at all.

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


More information about the cfe-commits mailing list