From 3e095035616159a6f98a54a36d3beb5fea804ee6 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Thu, 16 Jun 2022 12:26:36 -0500
Subject: [PATCH] Fix 11087: false negative: usage of reference to member of
 temporary object not detected (#4217)

* Fix 11087: false negative: usage of reference to member of temporary object not detected

* Format

* Add another test case

* Fix FP with pointer

* Format
---
 lib/valueflow.cpp          | 36 ++++++++++++++++++++++++-
 test/testautovariables.cpp | 55 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index bd52004f3..0aac6a1a4 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -3247,6 +3247,31 @@ std::vector<ValueFlow::Value> getLifetimeObjValues(const Token* tok, bool inconc
     return result;
 }
 
+static bool hasUniqueOwnership(const Token* tok)
+{
+    if (astIsPointer(tok))
+        return false;
+    if (astIsUniqueSmartPointer(tok))
+        return true;
+    if (astIsContainerOwned(tok))
+        return true;
+    return false;
+}
+
+// Check if dereferencing an object that doesn't have unique ownership
+static bool derefShared(const Token* tok)
+{
+    if (!tok)
+        return false;
+    if (tok->str() == "." && tok->originalName() != "->") {
+        return false;
+    } else if (!tok->isUnaryOp("*") && tok->str() == "[") {
+        return false;
+    }
+    const Token* ptrTok = tok->astOperand1();
+    return !hasUniqueOwnership(ptrTok);
+}
+
 ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
 {
     std::vector<ValueFlow::Value> values = getLifetimeObjValues(tok, inconclusive);
@@ -3320,6 +3345,7 @@ static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
                     const Variable* argvar = argvarTok->variable();
                     if (!argvar)
                         continue;
+                    const Token* argTok = nullptr;
                     if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) {
                         int n = getArgumentPos(argvar, f);
                         if (n < 0)
@@ -3328,9 +3354,17 @@ static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
                         // TODO: Track lifetimes of default parameters
                         if (n >= args.size())
                             return std::vector<LifetimeToken> {};
-                        const Token* argTok = args[n];
+                        argTok = args[n];
                         lt.errorPath.emplace_back(returnTok, "Return reference.");
                         lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'.");
+                    } else if (Token::Match(tok->tokAt(-2), ". %name% (") && !derefShared(tok->tokAt(-2)) &&
+                               exprDependsOnThis(argvarTok)) {
+                        argTok = tok->tokAt(-2)->astOperand1();
+                        lt.errorPath.emplace_back(returnTok, "Return reference that depends on 'this'.");
+                        lt.errorPath.emplace_back(tok->previous(),
+                                                  "Calling member function on '" + argTok->expressionString() + "'.");
+                    }
+                    if (argTok) {
                         std::vector<LifetimeToken> arglts = LifetimeToken::setInconclusive(
                             getLifetimeTokens(argTok, escape, std::move(lt.errorPath), pred, depth - returns.size()),
                             returns.size() > 1);
diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp
index 9a465f13f..055d65174 100644
--- a/test/testautovariables.cpp
+++ b/test/testautovariables.cpp
@@ -1940,6 +1940,61 @@ private:
               "    g(std::move(v));\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #11087
+        check("struct S1 {\n"
+              "    int& get() { return val; }\n"
+              "    int val{42};\n"
+              "};\n"
+              "void f() {\n"
+              "    int& v = S1().get();\n"
+              "    v += 1;\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:6] -> [test.cpp:2] -> [test.cpp:6] -> [test.cpp:7]: (error) Using reference to dangling temporary.\n",
+            errout.str());
+
+        check("struct A {\n"
+              "    const int& g() const { return i; }\n"
+              "    int i;\n"
+              "};\n"
+              "A* a();\n"
+              "int f() {\n"
+              "    const int& i = a()->g();\n"
+              "    return i;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("struct A {\n"
+              "    const int& g() const { return i; }\n"
+              "    int i;\n"
+              "};\n"
+              "std::unique_ptr<A> a();\n"
+              "int f() {\n"
+              "    const int& i = a()->g();\n"
+              "    return i;\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:7] -> [test.cpp:2] -> [test.cpp:7] -> [test.cpp:8]: (error) Using reference to dangling temporary.\n",
+            errout.str());
+
+        check("struct S1 {\n"
+              "    auto get() -> auto& { return val; }\n"
+              "    int val{42};\n"
+              "};\n"
+              "struct S2 {\n"
+              "    auto get() -> S1 { return s; }\n"
+              "    S1 s;\n"
+              "};\n"
+              "auto main() -> int {\n"
+              "    S2 c{};\n"
+              "    auto& v = c.get().get();\n"
+              "    v += 1;\n"
+              "    return c.s.val;\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:11] -> [test.cpp:2] -> [test.cpp:11] -> [test.cpp:12]: (error) Using reference to dangling temporary.\n",
+            errout.str());
     }
 
     void testglobalnamespace() {