Function return: Extra check of safe function return values

cfg/cppcheck-cfg.rng

@@ -132,6 +132,13 @@
                   <data type="positiveInteger"/>
                 </attribute>
               </optional>
+              <optional>
+                <attribute name="safeValues">
+                  <data type="string">
+                    <param name="pattern">all|([0-9]*:[0-9]*)</param>
+                  </data>
+                </attribute>
+              </optional>
               <text/>
             </element>
           </optional>

cfg/std.cfg

@@ -3054,7 +3054,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
   <!-- int rand(void); -->
   <function name="rand">
     <use-retval/>
-    <returnValue type="int"/>
+    <returnValue type="int" safeValues="all"/>
     <noreturn>false</noreturn>
     <leak-ignore/>
   </function>

cli/cmdlineparser.cpp

@@ -186,6 +186,11 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
             else if (std::strcmp(argv[i], "--all-functions-are-safe") == 0)
                 mSettings->allFunctionsAreSafe = true;
 
+            // Experimental: this flag says that all "safe output values" from functions should be checked.
+            // todo: add proper configuration options
+            else if (std::strcmp(argv[i], "--check-all-safe-return-values") == 0)
+                mSettings->checkAllSafeFunctionReturnValues = true;
+
             // Enforce language (--language=, -x)
             else if (std::strncmp(argv[i], "--language=", 11) == 0 || std::strcmp(argv[i], "-x") == 0) {
                 std::string str;

lib/library.cpp

@@ -618,6 +618,11 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
                 mReturnValueType[name] = type;
             if (const char *container = functionnode->Attribute("container"))
                 mReturnValueContainer[name] = std::atoi(container);
+            if (const char *safeValues = functionnode->Attribute("safeValues")) {
+                SafeValues sf{LLONG_MIN, LLONG_MAX};
+                if (std::strcmp(safeValues, "all") == 0)
+                    mReturnSafeValues[name] = sf;
+            }
         } else if (functionnodename == "arg") {
             const char* argNrString = functionnode->Attribute("nr");
             if (!argNrString)
@@ -1195,6 +1200,19 @@ int Library::returnValueContainer(const Token *ftok) const
     return it != mReturnValueContainer.end() ? it->second : -1;
 }
 
+bool Library::returnValueSafeValues(const Token *ftok, int64_t *v1, int64_t *v2) const
+{
+    if (isNotLibraryFunction(ftok))
+        return false;
+    const std::map<std::string, SafeValues>::const_iterator it = mReturnSafeValues.find(getFunctionName(ftok));
+    if (it == mReturnSafeValues.end())
+        return false;
+    *v1 = it->second.minValue;
+    *v2 = it->second.maxValue;
+    return true;
+
+}
+
 bool Library::hasminsize(const Token *ftok) const
 {
     if (isNotLibraryFunction(ftok))

lib/library.h

@@ -181,6 +181,7 @@ public:
     const std::string& returnValue(const Token *ftok) const;
     const std::string& returnValueType(const Token *ftok) const;
     int returnValueContainer(const Token *ftok) const;
+    bool returnValueSafeValues(const Token *ftok, int64_t *v1, int64_t *v2) const;
 
     bool isnoreturn(const Token *ftok) const;
     bool isnotnoreturn(const Token *ftok) const;
@@ -541,6 +542,11 @@ private:
     std::map<std::string, std::string> mReturnValue;
     std::map<std::string, std::string> mReturnValueType;
     std::map<std::string, int> mReturnValueContainer;
+    struct SafeValues {
+        int64_t minValue;
+        int64_t maxValue;
+    };
+    std::map<std::string, SafeValues> mReturnSafeValues;
     std::map<std::string, bool> mReportErrors;
     std::map<std::string, bool> mProcessAfterCode;
     std::set<std::string> mMarkupExtensions; // file extensions of markup files

lib/settings.cpp

@@ -40,6 +40,7 @@ Settings::Settings()
       force(false),
       inconclusive(false),
       allFunctionsAreSafe(false),
+      checkAllSafeFunctionReturnValues(false),
       inlineSuppressions(false),
       jobs(1),
       jointSuppressionReport(false),

lib/settings.h

@@ -149,6 +149,9 @@ public:
     /** @brief Experimental flag that says all functions are "safe" */
     bool allFunctionsAreSafe;
 
+    /** @brief Experimental flag to safety check function output */
+    bool checkAllSafeFunctionReturnValues;
+
     /** @brief Is --inline-suppr given? */
     bool inlineSuppressions;
 

lib/valueflow.cpp

@@ -5393,6 +5393,18 @@ static bool getMinMaxValues(const ValueType *vt, const cppcheck::Platform &platf
     return true;
 }
 
+static bool getMinMaxValues(const std::string &typestr, const Settings *settings, int64_t *minvalue, int64_t *maxvalue)
+{
+    TokenList typeTokens(settings);
+    std::istringstream istr(typestr+";");
+    if (!typeTokens.createTokens(istr))
+        return false;
+    typeTokens.simplifyPlatformTypes();
+    typeTokens.simplifyStdType();
+    const ValueType &vt = ValueType::parseDecl(typeTokens.front(), settings);
+    return getMinMaxValues(&vt, *settings, minvalue, maxvalue);
+}
+
 static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
 {
     if (settings->platformType == cppcheck::Platform::PlatformType::Unspecified)
@@ -5430,6 +5442,38 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold
     }
 }
 
+static void valueFlowSafeFunctionReturn(TokenList *tokenlist, const Settings *settings)
+{
+    if (!settings->checkAllSafeFunctionReturnValues)
+        return;
+    for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
+        if (!tok->astParent() || tok->str() != "(")
+            continue;
+        if (!Token::Match(tok->previous(), "%name%"))
+            continue;
+        int64_t v1,v2;
+        if (!settings->library.returnValueSafeValues(tok->previous(), &v1, &v2))
+            continue;
+
+        // Get min/max values for return type
+        const std::string typestr = settings->library.returnValueType(tok->previous());
+        int64_t minvalue, maxvalue;
+        if (!getMinMaxValues(typestr, settings, &minvalue, &maxvalue))
+            continue;
+
+        auto value = [](int64_t v, int64_t minvalue, int64_t maxvalue) {
+            if (v < minvalue)
+                return ValueFlow::Value(minvalue);
+            if (v > maxvalue)
+                return ValueFlow::Value(maxvalue);
+            return ValueFlow::Value(v);
+        };
+
+        setTokenValue(const_cast<Token *>(tok), value(v1, minvalue, maxvalue), settings);
+        setTokenValue(const_cast<Token *>(tok), value(v2, minvalue, maxvalue), settings);
+    }
+}
+
 ValueFlow::Value::Value(const Token *c, long long val)
     : valueType(INT),
       intvalue(val),
@@ -5496,6 +5540,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
     valueFlowNumber(tokenlist);
     valueFlowString(tokenlist);
     valueFlowArray(tokenlist);
+    valueFlowSafeFunctionReturn(tokenlist, settings);
     valueFlowGlobalConstVar(tokenlist, settings);
     valueFlowGlobalStaticVar(tokenlist, settings);
     valueFlowPointerAlias(tokenlist);

test/testvalueflow.cpp

@@ -3919,6 +3919,20 @@ private:
         ASSERT_EQUALS(-0x8000, values.front().intvalue);
         ASSERT_EQUALS(0x7fff, values.back().intvalue);
     }
+
+
+    void valueFlowSafeFunctionReturn() {
+        const char *code;
+        std::list<ValueFlow::Value> values;
+        Settings s;
+        s.checkAllSafeFunctionReturnValues = true;
+
+        code = "x = rand();";
+        values = tokenValues(code, "(", &s);
+        ASSERT_EQUALS(2, values.size());
+        ASSERT_EQUALS(INT_MIN, values.front().intvalue);
+        ASSERT_EQUALS(INT_MAX, values.back().intvalue);
+    }
 };
 
 REGISTER_TEST(TestValueFlow)