diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index b953da7af..106f53902 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2692,6 +2692,10 @@ void CheckMemoryLeakNoVar::check() // Checks if a call to an allocation function like malloc() is made and its return value is not assigned. checkForUnusedReturnValue(scope); + // Checks to see if a function is called with memory allocated for an argument that + // could be leaked if a function called for another argument throws. + checkForUnsafeArgAlloc(scope); + // goto the "}" that ends the executable scope.. const Token *tok = scope->classEnd; @@ -2748,6 +2752,52 @@ void CheckMemoryLeakNoVar::checkForUnusedReturnValue(const Scope *scope) } } +//--------------------------------------------------------------------------- +// Check if an exception could cause a leak in an argument constructed with +// shared_ptr/unique_ptr. For example, in the following code, it is possible +// that if g() throws an exception, the memory allocated by "new int(42)" +// could be leaked. See stackoverflow.com/questions/19034538/ +// why-is-there-memory-leak-while-using-shared-ptr-as-a-function-parameter +// +// void x() { +// f(shared_ptr(new int(42)), g()); +// } +//--------------------------------------------------------------------------- +void CheckMemoryLeakNoVar::checkForUnsafeArgAlloc(const Scope *scope) +{ + // This test only applies to C++ source + if (!_tokenizer->isCPP() || !_settings->inconclusive || !_settings->isEnabled("warning")) + return; + + for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (Token::Match(tok, "%var% (")) { + const Token *endParamToken = tok->next()->link(); + std::string pointerType; + std::string objectType; + std::string functionCalled; + + // Scan through the arguments to the function call + for (const Token *tok2 = tok->tokAt(2); tok2 && tok2 != endParamToken; tok2 = tok2->nextArgument()) { + const Function *pFunc = tok2->function(); + const bool isNothrow = pFunc && (pFunc->isDeclspecNothrow() || pFunc->isAttributeNothrow()); + + if (Token::Match(tok2, "shared_ptr|unique_ptr < %var% > ( new %var%")) { + pointerType = tok2->str(); + objectType = tok2->strAt(6); + } else if (!isNothrow) { + if (Token::Match(tok2, "%var% (")) + functionCalled = tok2->str(); + else if (Token::Match(tok2, "%var% < %var% > (")) + functionCalled = tok2->str() + "<" + tok2->strAt(2) + ">"; + } + } + + if (!pointerType.empty() && !functionCalled.empty()) + unsafeArgAllocError(tok, functionCalled, pointerType, objectType); + } + } +} + void CheckMemoryLeakNoVar::functionCallLeak(const Token *loc, const std::string &alloc, const std::string &functionCall) { reportError(loc, Severity::error, "leakNoVarFunctionCall", "Allocation with " + alloc + ", " + functionCall + " doesn't release it."); @@ -2757,3 +2807,11 @@ void CheckMemoryLeakNoVar::returnValueNotUsedError(const Token *tok, const std:: { reportError(tok, Severity::error, "leakReturnValNotUsed", "Return value of allocation function " + alloc + " is not used."); } + +void CheckMemoryLeakNoVar::unsafeArgAllocError(const Token *tok, const std::string &funcName, const std::string &ptrType, const std::string& objType) +{ + const std::string factoryFunc = ptrType == "shared_ptr" ? "make_shared" : "make_unique"; + reportError(tok, Severity::warning, "leakUnsafeArgAlloc", + "Unsafe allocation. If " + funcName + "() throws, memory could be leaked. Use " + factoryFunc + "<" + objType + ">() instead.", + true); // Inconclusive because funcName may never throw +} diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index b9569a7b8..2d7711dc3 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -446,14 +446,22 @@ private: */ void checkForUnusedReturnValue(const Scope *scope); + /** + * @brief %Check if an exception could cause a leak in an argument constructed with shared_ptr/unique_ptr. + * @param scope The scope of the function to check. + */ + void checkForUnsafeArgAlloc(const Scope *scope); + void functionCallLeak(const Token *loc, const std::string &alloc, const std::string &functionCall); void returnValueNotUsedError(const Token* tok, const std::string &alloc); + void unsafeArgAllocError(const Token *tok, const std::string &funcName, const std::string &ptrType, const std::string &objType); void getErrorMessages(ErrorLogger *e, const Settings *settings) const { CheckMemoryLeakNoVar c(0, settings, e); c.functionCallLeak(0, "funcName", "funcName"); c.returnValueNotUsedError(0, "funcName"); + c.unsafeArgAllocError(0, "funcName", "shared_ptr", "int"); } static std::string myName() { diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 0b5822c6b..d07b413d9 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -6277,6 +6277,8 @@ private: void run() { settings.standards.posix = true; + settings.inconclusive = true; + settings.addEnabled("warning"); LOAD_LIB_2(settings.library, "gtk.cfg"); @@ -6293,8 +6295,12 @@ private: // pass allocated memory to function.. TEST_CASE(functionParameter); + // never use leakable resource TEST_CASE(missingAssignment); + + // pass allocated memory to function using a smart pointer + TEST_CASE(smartPointerFunctionParam); } void functionParameter() { @@ -6441,6 +6447,55 @@ private: "}"); TODO_ASSERT_EQUALS("[test.cpp:7]: (error) Return value of allocation function f is not used.\n", "", errout.str()); } + + void smartPointerFunctionParam() { + check("void x() {\n" + " f(shared_ptr(new int(42)), g());\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If g() throws, memory could be leaked. Use make_shared() instead.\n", errout.str()); + + check("void x() {\n" + " h(12, f(shared_ptr(new int(42)), g()));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If g() throws, memory could be leaked. Use make_shared() instead.\n", errout.str()); + + check("void x() {\n" + " f(unique_ptr(new int(42)), g());\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If g() throws, memory could be leaked. Use make_unique() instead.\n", errout.str()); + + check("void x() {\n" + " f(g(), shared_ptr(new int(42)));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If g() throws, memory could be leaked. Use make_shared() instead.\n", errout.str()); + + check("void x() {\n" + " f(g(), unique_ptr(new int(42)));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If g() throws, memory could be leaked. Use make_unique() instead.\n", errout.str()); + + check("void x() {\n" + " f(shared_ptr(new char), make_unique(32));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If make_unique() throws, memory could be leaked. Use make_shared() instead.\n", errout.str()); + + check("void x() {\n" + " f(g(124), h(\"test\", 234), shared_ptr(new char));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Unsafe allocation. If h() throws, memory could be leaked. Use make_shared() instead.\n", errout.str()); + + check("void g(int x) throw() { }\n" + "void x() {\n" + " f(g(124), shared_ptr(new char));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void __declspec(nothrow) g(int x) { }\n" + "void x() {\n" + " f(g(124), shared_ptr(new char));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestMemleakNoVar)