Find reference to dangling unique ptr (#3344)

This commit is contained in:
Paul Fultz II 2021-07-20 14:30:27 -05:00 committed by GitHub
parent b409d4a598
commit 8efe1d4ab4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 137 additions and 44 deletions

View File

@ -80,14 +80,18 @@
<container id="boostDeque" startPattern="boost :: deque &lt;" inherits="stdDeque"/> <container id="boostDeque" startPattern="boost :: deque &lt;" inherits="stdDeque"/>
<!-- ########## Boost smart pointers ########## --> <!-- ########## Boost smart pointers ########## -->
<!-- https://www.boost.org/doc/libs/1_70_0/libs/smart_ptr/doc/html/smart_ptr.html --> <!-- https://www.boost.org/doc/libs/1_70_0/libs/smart_ptr/doc/html/smart_ptr.html -->
<smart-pointer class-name="boost::scoped_ptr"/> <smart-pointer class-name="boost::scoped_ptr">
<unique/>
</smart-pointer>
<!-- <smart-pointer class-name="boost::scoped_array"/> Already configured as container --> <!-- <smart-pointer class-name="boost::scoped_array"/> Already configured as container -->
<smart-pointer class-name="boost::shared_ptr"/> <smart-pointer class-name="boost::shared_ptr"/>
<smart-pointer class-name="boost::weak_ptr"/> <smart-pointer class-name="boost::weak_ptr"/>
<smart-pointer class-name="boost::intrusive_ptr"/> <smart-pointer class-name="boost::intrusive_ptr"/>
<smart-pointer class-name="boost::local_shared_ptr"/> <smart-pointer class-name="boost::local_shared_ptr"/>
<!-- https://www.boost.org/doc/libs/1_70_0/doc/html/boost/movelib/unique_ptr.html --> <!-- https://www.boost.org/doc/libs/1_70_0/doc/html/boost/movelib/unique_ptr.html -->
<smart-pointer class-name="boost::movelib::unique_ptr"/> <smart-pointer class-name="boost::movelib::unique_ptr">
<unique/>
</smart-pointer>
<!-- ########## Boost functions ########## --> <!-- ########## Boost functions ########## -->
<!-- https://www.boost.org/doc/libs/1_69_0/doc/html/boost/algorithm/join.html --> <!-- https://www.boost.org/doc/libs/1_69_0/doc/html/boost/algorithm/join.html -->
<!-- template<typename SequenceSequenceT, typename Range1T> <!-- template<typename SequenceSequenceT, typename Range1T>

View File

@ -487,6 +487,11 @@
<element name="smart-pointer"> <element name="smart-pointer">
<attribute name="class-name"><ref name="DATA-EXTNAME"/></attribute> <attribute name="class-name"><ref name="DATA-EXTNAME"/></attribute>
<optional>
<element name="unique">
<empty/>
</element>
</optional>
</element> </element>
<element name="type-checks"> <element name="type-checks">

View File

@ -5227,8 +5227,12 @@
<smart-pointer class-name="QSharedPointer"/> <smart-pointer class-name="QSharedPointer"/>
<smart-pointer class-name="QWeakPointer"/> <smart-pointer class-name="QWeakPointer"/>
<smart-pointer class-name="QPointer"/> <smart-pointer class-name="QPointer"/>
<smart-pointer class-name="QScopedPointer"/> <smart-pointer class-name="QScopedPointer">
<smart-pointer class-name="QScopedArrayPointer"/> <unique/>
</smart-pointer>
<smart-pointer class-name="QScopedArrayPointer">
<unique/>
</smart-pointer>
<!-- Internal Smart Pointers --> <!-- Internal Smart Pointers -->
<smart-pointer class-name="QtPatternist::AutoPtr"/> <smart-pointer class-name="QtPatternist::AutoPtr"/>
<smart-pointer class-name="QGuard"/> <smart-pointer class-name="QGuard"/>

View File

@ -8266,9 +8266,13 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
<type templateParameter="0"/> <type templateParameter="0"/>
</container> </container>
<container id="stdString" startPattern="std :: string|wstring|u16string|u32string" endPattern="" inherits="stdAllString"/> <container id="stdString" startPattern="std :: string|wstring|u16string|u32string" endPattern="" inherits="stdAllString"/>
<smart-pointer class-name="std::auto_ptr"/> <smart-pointer class-name="std::auto_ptr">
<unique/>
</smart-pointer>
<smart-pointer class-name="std::shared_ptr"/> <smart-pointer class-name="std::shared_ptr"/>
<smart-pointer class-name="std::unique_ptr"/> <smart-pointer class-name="std::unique_ptr">
<unique/>
</smart-pointer>
<smart-pointer class-name="std::weak_ptr"/> <smart-pointer class-name="std::weak_ptr"/>
<type-checks> <type-checks>
<unusedvar> <unusedvar>

View File

@ -5234,8 +5234,12 @@
<podtype name="wxLogLevel"/> <podtype name="wxLogLevel"/>
<!-- https://docs.wxwidgets.org/trunk/group__group__class__smartpointers.html --> <!-- https://docs.wxwidgets.org/trunk/group__group__class__smartpointers.html -->
<smart-pointer class-name="wxObjectDataPtr"/> <smart-pointer class-name="wxObjectDataPtr"/>
<smart-pointer class-name="wxScopedArray"/> <smart-pointer class-name="wxScopedArray">
<smart-pointer class-name="wxScopedPtr"/> <unique/>
</smart-pointer>
<smart-pointer class-name="wxScopedPtr">
<unique/>
</smart-pointer>
<smart-pointer class-name="wxSharedPtr"/> <smart-pointer class-name="wxSharedPtr"/>
<smart-pointer class-name="wxWeakRefDynamic"/> <smart-pointer class-name="wxWeakRefDynamic"/>
<smart-pointer class-name="wxWeakRef"/> <smart-pointer class-name="wxWeakRef"/>

View File

@ -498,8 +498,15 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
else if (nodename == "smart-pointer") { else if (nodename == "smart-pointer") {
const char *className = node->Attribute("class-name"); const char *className = node->Attribute("class-name");
if (className) if (!className)
smartPointers.insert(className); return Error(ErrorCode::MISSING_ATTRIBUTE, "class-name");
SmartPointer& smartPointer = smartPointers[className];
for (const tinyxml2::XMLElement* smartPointerNode = node->FirstChildElement(); smartPointerNode;
smartPointerNode = smartPointerNode->NextSiblingElement()) {
const std::string smartPointerNodeName = smartPointerNode->Name();
if (smartPointerNodeName == "unique")
smartPointer.unique = true;
}
} }
else if (nodename == "type-checks") { else if (nodename == "type-checks") {
@ -1501,14 +1508,19 @@ bool Library::isimporter(const std::string& file, const std::string &importer) c
return (it != mImporters.end() && it->second.count(importer) > 0); return (it != mImporters.end() && it->second.count(importer) > 0);
} }
bool Library::isSmartPointer(const Token *tok) const bool Library::isSmartPointer(const Token* tok) const { return detectSmartPointer(tok); }
const Library::SmartPointer* Library::detectSmartPointer(const Token* tok) const
{ {
std::string typestr; std::string typestr;
while (Token::Match(tok, "%name%|::")) { while (Token::Match(tok, "%name%|::")) {
typestr += tok->str(); typestr += tok->str();
tok = tok->next(); tok = tok->next();
} }
return smartPointers.find(typestr) != smartPointers.end(); auto it = smartPointers.find(typestr);
if (it == smartPointers.end())
return nullptr;
return &it->second;
} }
CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok) CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok)

View File

@ -432,8 +432,13 @@ public:
std::vector<std::string> defines; // to provide some library defines std::vector<std::string> defines; // to provide some library defines
std::unordered_set<std::string> smartPointers; struct SmartPointer {
bool unique = false;
};
std::map<std::string, SmartPointer> smartPointers;
bool isSmartPointer(const Token *tok) const; bool isSmartPointer(const Token *tok) const;
const SmartPointer* detectSmartPointer(const Token* tok) const;
struct PodType { struct PodType {
unsigned int size; unsigned int size;

View File

@ -6170,10 +6170,11 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V
// we are past the end of the type // we are past the end of the type
type = type->previous(); type = type->previous();
continue; continue;
} else if (settings->library.isSmartPointer(type)) { } else if (const Library::SmartPointer* smartPointer = settings->library.detectSmartPointer(type)) {
const Token* argTok = Token::findsimplematch(type, "<"); const Token* argTok = Token::findsimplematch(type, "<");
if (!argTok) if (!argTok)
continue; continue;
valuetype->smartPointer = smartPointer;
valuetype->smartPointerTypeToken = argTok->next(); valuetype->smartPointerTypeToken = argTok->next();
valuetype->smartPointerType = argTok->next()->type(); valuetype->smartPointerType = argTok->next()->type();
valuetype->type = ValueType::Type::NONSTD; valuetype->type = ValueType::Type::NONSTD;

View File

@ -1204,13 +1204,18 @@ public:
nonneg int bits; ///< bitfield bitcount nonneg int bits; ///< bitfield bitcount
nonneg int pointer; ///< 0=>not pointer, 1=>*, 2=>**, 3=>***, etc nonneg int pointer; ///< 0=>not pointer, 1=>*, 2=>**, 3=>***, etc
nonneg int constness; ///< bit 0=data, bit 1=*, bit 2=** nonneg int constness; ///< bit 0=data, bit 1=*, bit 2=**
Reference reference = Reference::None;///< Is the outermost indirection of this type a reference or rvalue reference or not? pointer=2, Reference=LValue would be a T**& Reference reference = Reference::None; ///< Is the outermost indirection of this type a reference or rvalue
const Scope *typeScope; ///< if the type definition is seen this point out the type scope ///< reference or not? pointer=2, Reference=LValue would be a T**&
const ::Type *smartPointerType; ///< Smart pointer type const Scope* typeScope; ///< if the type definition is seen this point out the type scope
const ::Type* smartPointerType; ///< Smart pointer type
const Token* smartPointerTypeToken; ///< Smart pointer type token const Token* smartPointerTypeToken; ///< Smart pointer type token
const Library::Container *container; ///< If the type is a container defined in a cfg file, this is the used container const Library::SmartPointer* smartPointer; ///< Smart pointer
const Token *containerTypeToken; ///< The container type token. the template argument token that defines the container element type. const Library::Container* container; ///< If the type is a container defined in a cfg file, this is the used
std::string originalTypeName; ///< original type name as written in the source code. eg. this might be "uint8_t" when type is CHAR. ///< container
const Token* containerTypeToken; ///< The container type token. the template argument token that defines the
///< container element type.
std::string originalTypeName; ///< original type name as written in the source code. eg. this might be "uint8_t"
///< when type is CHAR.
ValueType() ValueType()
: sign(UNKNOWN_SIGN), : sign(UNKNOWN_SIGN),
@ -1221,6 +1226,7 @@ public:
typeScope(nullptr), typeScope(nullptr),
smartPointerType(nullptr), smartPointerType(nullptr),
smartPointerTypeToken(nullptr), smartPointerTypeToken(nullptr),
smartPointer(nullptr),
container(nullptr), container(nullptr),
containerTypeToken(nullptr) containerTypeToken(nullptr)
{} {}
@ -1233,6 +1239,7 @@ public:
typeScope(nullptr), typeScope(nullptr),
smartPointerType(nullptr), smartPointerType(nullptr),
smartPointerTypeToken(nullptr), smartPointerTypeToken(nullptr),
smartPointer(nullptr),
container(nullptr), container(nullptr),
containerTypeToken(nullptr) containerTypeToken(nullptr)
{} {}
@ -1245,6 +1252,7 @@ public:
typeScope(nullptr), typeScope(nullptr),
smartPointerType(nullptr), smartPointerType(nullptr),
smartPointerTypeToken(nullptr), smartPointerTypeToken(nullptr),
smartPointer(nullptr),
container(nullptr), container(nullptr),
containerTypeToken(nullptr) containerTypeToken(nullptr)
{} {}
@ -1257,24 +1265,11 @@ public:
typeScope(nullptr), typeScope(nullptr),
smartPointerType(nullptr), smartPointerType(nullptr),
smartPointerTypeToken(nullptr), smartPointerTypeToken(nullptr),
smartPointer(nullptr),
container(nullptr), container(nullptr),
containerTypeToken(nullptr), containerTypeToken(nullptr),
originalTypeName(otn) originalTypeName(otn)
{} {}
ValueType(const ValueType &vt)
: sign(vt.sign)
, type(vt.type)
, bits(vt.bits)
, pointer(vt.pointer)
, constness(vt.constness)
, reference(vt.reference)
, typeScope(vt.typeScope)
, smartPointerType(vt.smartPointerType)
, smartPointerTypeToken(vt.smartPointerTypeToken)
, container(vt.container)
, containerTypeToken(vt.containerTypeToken)
, originalTypeName(vt.originalTypeName)
{}
static ValueType parseDecl(const Token *type, const Settings *settings); static ValueType parseDecl(const Token *type, const Settings *settings);

View File

@ -2615,6 +2615,15 @@ static void valueFlowReverse(TokenList* tokenlist,
valueFlowReverse(tok, nullptr, varToken, values, tokenlist, settings); valueFlowReverse(tok, nullptr, varToken, values, tokenlist, settings);
} }
static bool isUniqueSmartPointer(const Token* tok)
{
if (!astIsSmartPointer(tok))
return false;
if (!tok->valueType()->smartPointer)
return false;
return tok->valueType()->smartPointer->unique;
}
std::string lifetimeType(const Token *tok, const ValueFlow::Value *val) std::string lifetimeType(const Token *tok, const ValueFlow::Value *val)
{ {
std::string result; std::string result;
@ -2786,8 +2795,11 @@ static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
false); false);
} }
} }
} else if (Token::Match(tok, ".|::|[")) { } else if (Token::Match(tok, ".|::|[") || tok->isUnaryOp("*")) {
const Token *vartok = tok; const Token *vartok = tok;
if (tok->isUnaryOp("*"))
vartok = tok->astOperand1();
while (vartok) { while (vartok) {
if (vartok->str() == "[" || vartok->originalName() == "->") if (vartok->str() == "[" || vartok->originalName() == "->")
vartok = vartok->astOperand1(); vartok = vartok->astOperand1();
@ -2800,7 +2812,8 @@ static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
if (!vartok) if (!vartok)
return {{tok, std::move(errorPath)}}; return {{tok, std::move(errorPath)}};
const Variable *tokvar = vartok->variable(); const Variable *tokvar = vartok->variable();
if (!astIsContainer(vartok) && !(tokvar && tokvar->isArray() && !tokvar->isArgument()) && if (!isUniqueSmartPointer(vartok) && !astIsContainer(vartok) &&
!(tokvar && tokvar->isArray() && !tokvar->isArgument()) &&
(Token::Match(vartok->astParent(), "[|*") || vartok->astParent()->originalName() == "->")) { (Token::Match(vartok->astParent(), "[|*") || vartok->astParent()->originalName() == "->")) {
for (const ValueFlow::Value &v : vartok->values()) { for (const ValueFlow::Value &v : vartok->values()) {
if (!v.isLocalLifetimeValue()) if (!v.isLocalLifetimeValue())

View File

@ -117,6 +117,7 @@ private:
TEST_CASE(returnReference19); // #9597 TEST_CASE(returnReference19); // #9597
TEST_CASE(returnReference20); // #9536 TEST_CASE(returnReference20); // #9536
TEST_CASE(returnReference21); // #9530 TEST_CASE(returnReference21); // #9530
TEST_CASE(returnReference22);
TEST_CASE(returnReferenceFunction); TEST_CASE(returnReferenceFunction);
TEST_CASE(returnReferenceContainer); TEST_CASE(returnReferenceContainer);
TEST_CASE(returnReferenceLiteral); TEST_CASE(returnReferenceLiteral);
@ -1407,6 +1408,50 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void returnReference22()
{
check("int& f() {\n"
" std::unique_ptr<int> p = std::make_unique<int>(1);\n"
" return *p;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Reference to local variable returned.\n", errout.str());
check("void g(const std::unique_ptr<int>&);\n"
"int& f() {\n"
" std::unique_ptr<int> p = std::make_unique<int>(1);\n"
" g(p);\n"
" return *p;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Reference to local variable returned.\n", errout.str());
check("void g(std::shared_ptr<int>);\n"
"int& f() {\n"
" std::shared_ptr<int> p = std::make_shared<int>(1);\n"
" g(p);\n"
" return *p;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("std::shared_ptr<int> g();\n"
"int& f() {\n"
" return *g();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("std::unique_ptr<int> g();\n"
"int& f() {\n"
" return *g();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Reference to temporary returned.\n", errout.str());
check("struct A { int x; };\n"
"int& f() {\n"
" std::unique_ptr<A> p = std::make_unique<A>();\n"
" return p->x;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Reference to local variable returned.\n", errout.str());
}
void returnReferenceFunction() { void returnReferenceFunction() {
check("int& f(int& a) {\n" check("int& f(int& a) {\n"
" return a;\n" " return a;\n"

View File

@ -221,7 +221,7 @@ private:
std::string getRange(const char code[], const std::string &str, int linenr = 0) { std::string getRange(const char code[], const std::string &str, int linenr = 0) {
Settings settings; Settings settings;
settings.platform(cppcheck::Platform::Unix64); settings.platform(cppcheck::Platform::Unix64);
settings.library.smartPointers.insert("std::shared_ptr"); settings.library.smartPointers["std::shared_ptr"];
Tokenizer tokenizer(&settings, this); Tokenizer tokenizer(&settings, this);
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp"); tokenizer.tokenize(istr, "test.cpp");
@ -248,7 +248,7 @@ private:
settings->bugHunting = true; settings->bugHunting = true;
settings->debugBugHunting = true; settings->debugBugHunting = true;
settings->platform(cppcheck::Platform::Unix64); settings->platform(cppcheck::Platform::Unix64);
settings->library.smartPointers.insert("std::shared_ptr"); settings->library.smartPointers["std::shared_ptr"];
Tokenizer tokenizer(settings, this); Tokenizer tokenizer(settings, this);
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp"); tokenizer.tokenize(istr, "test.cpp");

View File

@ -48,8 +48,9 @@ private:
settings.library.setalloc("fopen", id, -1); settings.library.setalloc("fopen", id, -1);
settings.library.setrealloc("freopen", id, -1, 3); settings.library.setrealloc("freopen", id, -1, 3);
settings.library.setdealloc("fclose", id, 1); settings.library.setdealloc("fclose", id, 1);
settings.library.smartPointers.insert("std::shared_ptr"); settings.library.smartPointers["std::shared_ptr"];
settings.library.smartPointers.insert("std::unique_ptr"); settings.library.smartPointers["std::unique_ptr"];
settings.library.smartPointers["std::unique_ptr"].unique = true;
const char xmldata[] = "<?xml version=\"1.0\"?>\n" const char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def>\n" "<def>\n"