walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Add support for string_view (#3480)

This commit is contained in:
Paul Fultz II 2021-10-05 01:28:19 -05:00 committed by GitHub
parent 71809044bd
commit 8668d445c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 253 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cfg/boost.cfg
View File

@ -81,6 +81,7 @@
<container id="boostVector" startPattern="boost :: vector|small_vector &lt;" inherits="stdVector"/>
<container id="boostStableVector" startPattern="boost :: stable_vector|static_vector &lt;" inherits="stdVectorDeque"/>
<container id="boostDeque" startPattern="boost :: deque &lt;" inherits="stdDeque"/>
<container id="boostStringView" startPattern="boost :: string_view" inherits="stdStringView"/>
<!-- ########## Boost smart pointers ########## -->
<!-- https://www.boost.org/doc/libs/1_70_0/libs/smart_ptr/doc/html/smart_ptr.html -->
<smart-pointer class-name="boost::scoped_ptr">

3
cfg/cppcheck-cfg.rng
View File

@ -416,6 +416,9 @@
<optional>
<attribute name="hasInitializerListConstructor"><ref name="DATA-BOOL"/></attribute>
</optional>
<optional>
<attribute name="view"><ref name="DATA-BOOL"/></attribute>
</optional>
<optional>
<attribute name="itEndPattern"><text/></attribute>
</optional>

12
cfg/std.cfg
View File

@ -8305,6 +8305,18 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
<type templateParameter="0"/>
</container>
<container id="stdString" startPattern="std :: string|wstring|u16string|u32string" endPattern="" inherits="stdAllString"/>
<container id="stdAllStringView" inherits="stdAllString" view="true">
<size>
<function name="remove_prefix" action="change"/>
<function name="remove_suffix" action="change"/>
</size>
</container>
<container id="stdBasicStringView" startPattern="std :: basic_string_view &lt;" inherits="stdAllStringView">
<type templateParameter="0"/>
</container>
<container id="stdStringView" startPattern="std :: string_view|wstring_view|u16string_view|u32string_view" endPattern="" inherits="stdAllStringView"/>
<container id="stdExperimentalStringView" startPattern="std :: experimental :: string_view|wstring_view|u16string_view|u32string_view" endPattern="" inherits="stdAllStringView"/>
<container id="stdExperimentalBasicStringView" startPattern="std :: experimental :: basic_string_view &lt;" inherits="stdBasicStringView" />
<smart-pointer class-name="std::auto_ptr">
<unique/>
</smart-pointer>

13
lib/astutils.cpp
View File

@ -249,9 +249,18 @@ bool astIsIterator(const Token *tok)
return tok && tok->valueType() && tok->valueType()->type == ValueType::Type::ITERATOR;
}
bool astIsContainer(const Token *tok)
bool astIsContainer(const Token* tok) {
return getLibraryContainer(tok) != nullptr && !astIsIterator(tok);
}
bool astIsContainerView(const Token* tok)
{
return getLibraryContainer(tok) != nullptr && tok->valueType()->type != ValueType::Type::ITERATOR;
const Library::Container* container = getLibraryContainer(tok);
return container && !astIsIterator(tok) && container->view;
}
bool astIsContainerOwned(const Token* tok) {
return astIsContainer(tok) && !astIsContainerView(tok);
}
std::string astCanonicalType(const Token *expr)

3
lib/astutils.h
View File

@ -85,6 +85,9 @@ bool astIsIterator(const Token *tok);
bool astIsContainer(const Token *tok);
bool astIsContainerView(const Token* tok);
bool astIsContainerOwned(const Token* tok);
/**
* Get canonical type of expression. const/static/etc are not included and neither *&.
* For example:

6
lib/checkautovariables.cpp
View File

@ -558,14 +558,13 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
break;
}
}
}
const bool escape = Token::Match(tok->astParent(), "return|throw");
for (const ValueFlow::Value& val:tok->values()) {
if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue())
continue;
if (!printInconclusive && val.isInconclusive())
continue;
const bool escape = Token::Match(tok->astParent(), "return|throw");
for (const LifetimeToken& lt :
getLifetimeTokens(getParentLifetime(val.tokvalue), escape || isAssignedToNonLocal(tok))) {
const Token * tokvalue = lt.token;
@ -575,7 +574,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
continue;
if (!isLifetimeBorrowed(tok, mSettings))
continue;
if (tokvalue->exprId() == tok->exprId() && !(tok->variable() && tok->variable()->isArray()))
if (tokvalue->exprId() == tok->exprId() && !(tok->variable() && tok->variable()->isArray()) &&
!astIsContainerView(tok->astParent()))
continue;
if ((tokvalue->variable() && !isEscapedReference(tokvalue->variable()) &&
isInScope(tokvalue->variable()->nameToken(), scope)) ||

3
lib/library.cpp
View File

@ -439,6 +439,9 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
const char* const hasInitializerListConstructor = node->Attribute("hasInitializerListConstructor");
if (hasInitializerListConstructor)
container.hasInitializerListConstructor = std::string(hasInitializerListConstructor) == "true";
const char* const view = node->Attribute("view");
if (view)
container.view = std::string(view) == "true";
for (const tinyxml2::XMLElement *containerNode = node->FirstChildElement(); containerNode; containerNode = containerNode->NextSiblingElement()) {
const std::string containerNodeName = containerNode->Name();

30
lib/library.h
View File

@ -204,8 +204,8 @@ public:
class Container {
public:
Container() :
type_templateArgNo(-1),
Container()
: type_templateArgNo(-1),
size_templateArgNo(-1),
arrayLike_indexOp(false),
stdStringLike(false),
@ -213,14 +213,33 @@ public:
opLessAllowed(true),
hasInitializerListConstructor(false),
unstableErase(false),
unstableInsert(false) {}
unstableInsert(false),
view(false)
{}
enum class Action {
RESIZE, CLEAR, PUSH, POP, FIND, INSERT, ERASE, CHANGE_CONTENT, CHANGE, CHANGE_INTERNAL,
RESIZE,
CLEAR,
PUSH,
POP,
FIND,
INSERT,
ERASE,
CHANGE_CONTENT,
CHANGE,
CHANGE_INTERNAL,
NO_ACTION
};
enum class Yield {
AT_INDEX, ITEM, BUFFER, BUFFER_NT, START_ITERATOR, END_ITERATOR, ITERATOR, SIZE, EMPTY,
AT_INDEX,
ITEM,
BUFFER,
BUFFER_NT,
START_ITERATOR,
END_ITERATOR,
ITERATOR,
SIZE,
EMPTY,
NO_YIELD
};
struct Function {
@ -238,6 +257,7 @@ public:
bool hasInitializerListConstructor;
bool unstableErase;
bool unstableInsert;
bool view;
Action getAction(const std::string& function) const {
const std::map<std::string, Function>::const_iterator i = functions.find(function);

94
lib/valueflow.cpp
View File

@ -3031,22 +3031,32 @@ static bool isNotLifetimeValue(const ValueFlow::Value& val)
return !val.isLifetimeValue();
}
static bool isLifetimeOwned(const ValueType* vtParent)
{
if (vtParent->container)
return !vtParent->container->view;
return vtParent->type == ValueType::CONTAINER;
}
static bool isLifetimeOwned(const ValueType *vt, const ValueType *vtParent)
{
if (!vtParent)
return false;
if (!vt) {
if (vtParent->type == ValueType::CONTAINER)
if (isLifetimeOwned(vtParent))
return true;
return false;
}
// If converted from iterator to pointer then the iterator is most likely a pointer
if (vtParent->pointer == 1 && vt->pointer == 0 && vt->type == ValueType::ITERATOR)
return false;
if (vt->type != ValueType::UNKNOWN_TYPE && vtParent->type != ValueType::UNKNOWN_TYPE) {
if (vt->pointer != vtParent->pointer)
return true;
if (vt->type != vtParent->type) {
if (vtParent->type == ValueType::RECORD)
return true;
if (vtParent->type == ValueType::CONTAINER)
if (isLifetimeOwned(vtParent))
return true;
}
}
@ -3062,6 +3072,8 @@ static bool isLifetimeBorrowed(const ValueType *vt, const ValueType *vtParent)
return false;
if (vt->pointer > 0 && vt->pointer == vtParent->pointer)
return true;
if (vtParent->container && vtParent->container->view)
return true;
if (vt->type != ValueType::UNKNOWN_TYPE && vtParent->type != ValueType::UNKNOWN_TYPE && vtParent->container == vt->container) {
if (vtParent->pointer > vt->pointer)
return true;
@ -3146,6 +3158,42 @@ static bool isDifferentType(const Token* src, const Token* dst)
return false;
}
static std::vector<ValueType> getParentValueTypes(const Token* tok, const Settings* settings = nullptr)
{
if (!tok)
return {};
if (!tok->astParent())
return {};
if (Token::Match(tok->astParent(), "(|{|,")) {
int argn = -1;
const Token* ftok = getTokenArgumentFunction(tok, argn);
if (ftok && ftok->function()) {
std::vector<ValueType> result;
std::vector<const Variable*> argsVars = getArgumentVars(ftok, argn);
for (const Variable* var : getArgumentVars(ftok, argn)) {
if (!var)
continue;
if (!var->valueType())
continue;
result.push_back(*var->valueType());
}
return result;
}
}
if (settings && Token::Match(tok->astParent()->tokAt(-2), ". push_back|push_front|insert|push (") &&
astIsContainer(tok->astParent()->tokAt(-2)->astOperand1())) {
const Token* contTok = tok->astParent()->tokAt(-2)->astOperand1();
const ValueType* vtCont = contTok->valueType();
if (!vtCont->containerTypeToken)
return {};
ValueType vtParent = ValueType::parseDecl(vtCont->containerTypeToken, settings);
return {std::move(vtParent)};
}
if (tok->astParent()->valueType())
return {*tok->astParent()->valueType()};
return {};
}
bool isLifetimeBorrowed(const Token *tok, const Settings *settings)
{
if (!tok)
@ -3605,6 +3653,13 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
}
if (update)
valueFlowForwardLifetime(tok->next(), tokenlist, errorLogger, settings);
} else if (tok->valueType()) {
// TODO: Propagate lifetimes with library functions
if (settings->library.getFunction(tok->previous()))
return;
// Assume constructing the valueType
valueFlowLifetimeConstructor(tok, tokenlist, errorLogger, settings);
valueFlowForwardLifetime(tok->next(), tokenlist, errorLogger, settings);
}
}
@ -3673,7 +3728,13 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
Token* parent = tok->astParent();
while (Token::simpleMatch(parent, ","))
parent = parent->astParent();
if (Token::simpleMatch(parent, "{") && hasInitList(parent->astParent())) {
if (Token::Match(tok, "{|(") && astIsContainerView(tok) && !tok->function()) {
std::vector<const Token*> args = getArguments(tok);
if (args.size() == 1 && astIsContainerOwned(args.front())) {
LifetimeStore{args.front(), "Passed to container view.", ValueFlow::Value::LifetimeKind::SubObject}.byRef(
tok, tokenlist, errorLogger, settings);
}
} else if (Token::simpleMatch(parent, "{") && hasInitList(parent->astParent())) {
valueFlowLifetimeConstructor(tok, Token::typeOf(parent->previous()), tokenlist, errorLogger, settings);
} else if (Token::simpleMatch(tok, "{") && hasInitList(parent)) {
std::vector<const Token *> args = getArguments(tok);
@ -3764,6 +3825,16 @@ static bool isDecayedPointer(const Token *tok)
return astIsPointer(tok->astParent());
}
static bool isConvertedToView(const Token* tok, const Settings* settings)
{
std::vector<ValueType> vtParents = getParentValueTypes(tok, settings);
return std::any_of(vtParents.begin(), vtParents.end(), [&](const ValueType& vt) {
if (!vt.container)
return false;
return vt.container->view;
});
}
static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger *errorLogger, const Settings *settings)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@ -3861,6 +3932,13 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
}
// Converting to container view
else if (astIsContainerOwned(tok) && isConvertedToView(tok, settings)) {
LifetimeStore ls =
LifetimeStore{tok, "Converted to container view", ValueFlow::Value::LifetimeKind::SubObject};
ls.byRef(tok, tokenlist, errorLogger, settings);
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
// container lifetimes
else if (astIsContainer(tok)) {
Token * parent = astParentSkipParens(tok);
@ -3902,6 +3980,16 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
continue;
toks.push_back(v.tokvalue);
}
} else if (astIsContainerView(tok)) {
for (const ValueFlow::Value& v : tok->values()) {
if (!v.isLifetimeValue())
continue;
if (!v.tokvalue)
continue;
if (!astIsContainerOwned(v.tokvalue))
continue;
toks.push_back(v.tokvalue);
}
} else {
toks = {tok};
}

2
man/reference-cfg-format.md
View File

@ -580,6 +580,8 @@ A lot of C++ libraries, among those the STL itself, provide containers with very
The `hasInitializerListConstructor` attribute can be set when the container has a constructor taking an initializer list.
The `view` attribute can be set when the container is a view, which means it borrows the lifetime of another container.
Inside the `<container>` tag, functions can be defined inside of the tags `<size>`, `<access>` and `<other>` (on your choice). Each of them can specify an action like "resize" and/or the result it yields, for example "end-iterator".
The following example provides a definition for std::vector, based on the definition of "stdContainer" (not shown):

2
releasenotes.txt
View File

@ -1 +1,3 @@
release notes for cppcheck-2.7
Add support for container views. The `view` attribute has been added to the `<container>` library tag to specify the class is a view. The lifetime analysis has been updated to use this new attribute to find dangling lifetime containers.

97
test/testautovariables.cpp
View File

@ -141,6 +141,7 @@ private:
TEST_CASE(danglingLifetimeLambda);
TEST_CASE(danglingLifetimeContainer);
TEST_CASE(danglingLifetimeContainerView);
TEST_CASE(danglingLifetime);
TEST_CASE(danglingLifetimeFunction);
TEST_CASE(danglingLifetimeAggegrateConstructor);
@ -2444,6 +2445,102 @@ private:
ASSERT_EQUALS("", errout.str());
}
void danglingLifetimeContainerView()
{
check("std::string_view f() {\n"
" std::string s = \"\";\n"
" return s;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n",
errout.str());
check("std::string_view f() {\n"
" std::string s = \"\";\n"
" std::string_view sv = s;\n"
" return sv;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n",
errout.str());
check("std::string_view f() {\n"
" std::string s = \"\";\n"
" return std::string_view{s};\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n",
errout.str());
check("std::string_view f(std::string_view s) {\n"
" return s;\n"
"}\n"
"std::string_view g() {\n"
" std::string s = \"\";\n"
" return f(s);\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:6] -> [test.cpp:6] -> [test.cpp:5] -> [test.cpp:6]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n",
errout.str());
check("const char * f() {\n"
" std::string s;\n"
" std::string_view sv = s;\n"
" return sv.begin();\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 's' that will be invalid when returning.\n",
errout.str());
check("const char * f() {\n"
" std::string s;\n"
" return std::string_view{s}.begin();\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n",
errout.str());
check("const char * f() {\n"
" std::string s;\n"
" return std::string_view(s).begin();\n"
"}\n");
TODO_ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n",
"",
errout.str());
check("const char * f(std::string_view sv) {\n"
" return sv.begin();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("const char * f(std::string s) {\n"
" std::string_view sv = s;\n"
" return sv.begin();\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n",
errout.str());
check("std::string_view f(std::string s) {\n"
" return s;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:1] -> [test.cpp:2]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n",
errout.str());
check("const char * f(const std::string& s) {\n"
" std::string_view sv = s;\n"
" return sv.begin();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("std::string_view f(const std::string_view& sv) {\n"
" return sv;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void danglingLifetime() {
check("auto f() {\n"
" std::vector<int> a;\n"