Add inconclusive warning

Author: Your Name

lib/forwardanalyzer.cpp

@@ -745,13 +745,25 @@ namespace {
                                 return Break(Analyzer::Terminate::Bail);
                             if (updateScope(thenBranch.endBlock, depth - 1) == Progress::Break)
                                 return Break();
+                            // The branch was entered because of the tracked value; if it might not
+                            // return (it ends in a call to an unknown, possibly noreturn function)
+                            // then the value might not flow past the branch.
+                            if (!condTok->hasKnownIntValue() &&
+                                !isEscapeScope(thenBranch.endBlock, thenBranch.escapeUnknown) &&
+                                thenBranch.escapeUnknown && !analyzer->lowerToInconclusive())
+                                return Break(Analyzer::Terminate::Bail);
                         } else if (elseBranch.check) {
                             // Likewise the skipped then block could still modify the value
                             if (!condTok->hasKnownIntValue() && analyzeScope(thenBranch.endBlock).isModified() &&
                                 !analyzer->lowerToPossible())
                                 return Break(Analyzer::Terminate::Bail);
                             if (elseBranch.endBlock && updateScope(elseBranch.endBlock, depth - 1) == Progress::Break)
                                 return Break();
+                            // Same as above: an else branch that might not return
+                            if (elseBranch.endBlock && !condTok->hasKnownIntValue() &&
+                                !isEscapeScope(elseBranch.endBlock, elseBranch.escapeUnknown) &&
+                                elseBranch.escapeUnknown && !analyzer->lowerToInconclusive())
+                                return Break(Analyzer::Terminate::Bail);
                         } else {
                             const bool conditional = stopOnCondition(condTok);
                             // The value only flows into the then-branch when the condition can split

lib/valueflow.cpp

@@ -4650,6 +4650,13 @@ struct ConditionHandler {
         });
     }
 
+    static void lowerToInconclusive(std::list<ValueFlow::Value>& values) {
+        for (ValueFlow::Value& v : values) {
+            if (!v.isImpossible())
+                v.setInconclusive();
+        }
+    }
+
     void afterCondition(TokenList& tokenlist,
                         const SymbolDatabase& symboldatabase,
                         ErrorLogger& errorLogger,
@@ -4882,10 +4889,13 @@ struct ConditionHandler {
                 else if (!dead_if)
                     dead_if = isReturnScope(after, settings.library, &unknownFunction);
 
+                // If the taken branch might not return (it ends in a call to an unknown,
+                // possibly noreturn function) then its values might not flow past the
+                // conditional code -> lower them to inconclusive.
                 if (!dead_if && unknownFunction) {
                     if (settings.debugwarnings)
                         bailout(tokenlist, errorLogger, unknownFunction, "possible noreturn scope");
-                    return;
+                    lowerToInconclusive(thenValues);
                 }
 
                 if (Token::simpleMatch(after, "} else {")) {
@@ -4896,7 +4906,7 @@ struct ConditionHandler {
                     if (!dead_else && unknownFunction) {
                         if (settings.debugwarnings)
                             bailout(tokenlist, errorLogger, unknownFunction, "possible noreturn scope");
-                        return;
+                        lowerToInconclusive(elseValues);
                     }
                 }
 
@@ -4912,11 +4922,15 @@ struct ConditionHandler {
                     std::copy_if(thenValues.cbegin(),
                                  thenValues.cend(),
                                  std::back_inserter(values),
-                                 std::mem_fn(&ValueFlow::Value::isPossible));
+                                 [](const ValueFlow::Value& v) {
+                        return v.isPossible() || v.isInconclusive();
+                    });
                     std::copy_if(elseValues.cbegin(),
                                  elseValues.cend(),
                                  std::back_inserter(values),
-                                 std::mem_fn(&ValueFlow::Value::isPossible));
+                                 [](const ValueFlow::Value& v) {
+                        return v.isPossible() || v.isInconclusive();
+                    });
                 }
 
                 if (values.empty())

test/testnullpointer.cpp

@@ -4468,6 +4468,59 @@ class TestNullPointer : public TestFixture {
             "[test.cpp:3:13]: (warning) If resource allocation fails, then there is a possible null pointer dereference: fid [nullPointerOutOfResources]\n"
             "[test.cpp:4:12]: (warning) If resource allocation fails, then there is a possible null pointer dereference: fid [nullPointerOutOfResources]\n",
             errout_str());
+
+        // the guard might call an unknown, possibly noreturn function -> no warning
+        check("void f() {\n"
+              "    FILE* fid = fopen(\"x.txt\", \"w\");\n"
+              "    if (fid == NULL)\n"
+              "        g();\n"
+              "    fclose(fid);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        // .. but an inconclusive warning is reported with --inconclusive
+        check("void f() {\n"
+              "    FILE* fid = fopen(\"x.txt\", \"w\");\n"
+              "    if (fid == NULL)\n"
+              "        g();\n"
+              "    fclose(fid);\n"
+              "}\n",
+              dinit(CheckOptions, $.inconclusive = true));
+        ASSERT_EQUALS(
+            "[test.cpp:5:12]: (warning, inconclusive) If resource allocation fails, then there is a possible null pointer dereference: fid [nullPointerOutOfResources]\n",
+            errout_str());
+
+        check("int f(const int* p) {\n"
+              "    if (p == nullptr)\n"
+              "        g();\n"
+              "    return *p;\n"
+              "}\n",
+              dinit(CheckOptions, $.inconclusive = true));
+        ASSERT_EQUALS(
+            "[test.cpp:2:11] -> [test.cpp:4:13]: (warning, inconclusive) Either the condition 'p==nullptr' is redundant or there is possible null pointer dereference: p. [nullPointerRedundantCheck]\n",
+            errout_str());
+
+        check("void f() {\n"
+              "    FILE* fid = fopen(\"x.txt\", \"w\");\n"
+              "    if (fid != NULL)\n"
+              "        ;\n"
+              "    else\n"
+              "        g();\n"
+              "    fclose(fid);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        // guard function is known to return -> warning
+        check("void g() {}\n"
+              "void f() {\n"
+              "    FILE* fid = fopen(\"x.txt\", \"w\");\n"
+              "    if (fid == NULL)\n"
+              "        g();\n"
+              "    fclose(fid);\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:6:12]: (warning) If resource allocation fails, then there is a possible null pointer dereference: fid [nullPointerOutOfResources]\n",
+            errout_str());
     }
 
     void functioncalllibrary() {