Skip to content

Fix #11886 performance regression (hang) in 2.12dev #5355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 4 commits into from
Aug 22, 2023
Merged

Fix #11886 performance regression (hang) in 2.12dev #5355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5423262
Fix #11886 performance regression (hang) in 2.12dev
chrchr-github Aug 21, 2023
ff16270
Fix error message
chrchr-github Aug 21, 2023
f55d640
Fix string comparison
chrchr-github Aug 21, 2023
2447855
Address reviewer comments
chrchr-github Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/symboldatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4350,6 +4350,17 @@ const Function *Function::getOverriddenFunction(bool *foundAllBaseClasses) const
return getOverriddenFunctionRecursive(nestedIn->definedType, foundAllBaseClasses);
}

// prevent recursion if base is the same except for different template parameters
static bool isDerivedFromItself(const std::string& thisName, const std::string& baseName)
{
if (thisName.back() != '>')
return false;
const auto pos = thisName.find('<');
Copy link
Copy Markdown
Collaborator

@danmar danmar Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if this a hot path we could add something like this first to get a quick bailout:

if (thisName.back() != '>')
    return false;

Well if it's not hot then it's just extra code so feel free to skip that.

if (pos == std::string::npos)
return false;
return thisName.compare(0, pos + 1, baseName, 0, pos + 1) == 0;
}

const Function * Function::getOverriddenFunctionRecursive(const ::Type* baseType, bool *foundAllBaseClasses) const
{
// check each base class
Expand Down Expand Up @@ -4402,7 +4413,7 @@ const Function * Function::getOverriddenFunctionRecursive(const ::Type* baseType
}
}

if (!derivedFromType->derivedFrom.empty() && !derivedFromType->hasCircularDependencies()) {
if (!derivedFromType->derivedFrom.empty() && !derivedFromType->hasCircularDependencies() && !isDerivedFromItself(baseType->classScope->className, i.name)) {
// avoid endless recursion, see #5289 Crash: Stack overflow in isImplicitlyVirtual_rec when checking SVN and
// #5590 with a loop within the class hierarchy.
const Function *func = getOverriddenFunctionRecursive(derivedFromType, foundAllBaseClasses);
Expand Down
2 changes: 1 addition & 1 deletion lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5548,7 +5548,7 @@ static void valueFlowSymbolicOperators(const SymbolDatabase& symboldatabase, con
ValueFlow::Value v = value;
v.bound = ValueFlow::Value::Bound::Point;
v.valueType = ValueFlow::Value::ValueType::INT;
v.errorPath.emplace_back(strlenTok, "Return index of string to the first element that is 0");
v.errorPath.emplace_back(strlenTok, "Return index of first '\\0' character in string");
setTokenValue(tok, std::move(v), settings);
}
}
Expand Down
15 changes: 15 additions & 0 deletions test/testclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ class TestClass : public TestFixture {
TEST_CASE(const89);
TEST_CASE(const90);
TEST_CASE(const91);
TEST_CASE(const92);

TEST_CASE(const_handleDefaultParameters);
TEST_CASE(const_passThisToMemberOfOtherClass);
Expand Down Expand Up @@ -6654,6 +6655,20 @@ class TestClass : public TestFixture {
ASSERT_EQUALS("", errout.str());
}

void const92() { // #11886
checkConst("void g(int);\n"
"template<int n>\n"
"struct S : public S<n - 1> {\n"
" void f() {\n"
" g(n - 1);\n"
" }\n"
"};\n"
"template<>\n"
"struct S<0> {};\n"
"struct D : S<150> {};\n");
// don't hang
}

void const_handleDefaultParameters() {
checkConst("struct Foo {\n"
" void foo1(int i, int j = 0) {\n"
Expand Down