Fix #14247 FN returnDanglingLifetime with designated initializer

[chrchr-github]

No description provided.

[danmar]

I am not sure exactly what this code does with tokvalue..
should it be checked that the dot is a unary operand? we could have something like ab.a=...

[chrchr-github]

Or maybe this needs to be fixed in ValueFlow, i.e. the tokvalue should already point at the assigned variable?

[danmar]

hmm I am actually not very sure about how LifetimeTokens work

[chrchr-github]

The = is produced here and then goes unchanged through LifetimeStore:

cppcheck/lib/valueflow.cpp

Line 2861 in 1f35303

std::vector<const Token*> args = getArguments(tok);

So I guess it's OK to handle the designated initializer later.

[pfultz2]

Yea ValueFlow needs to be updated. It treats the .x = x as an expression to capture the lifetime because it only handles aggregate constructors without the name. It needs to be updated to associate the correct lifetime to the correct variable(it currently assumes its in the order of member variables which is not the case when it skips variables with named aggregates).

[chrchr-github]

I see, but that seems like a different problem.
At least there is a FN for this:

struct A { int i; int& r; };
A f() {
    int x = 0;
    A a{.r = x};
    return a;
}

[pfultz2]

Its the same problem. The valueflow is wrong, it should be lifetime[SubObject]=(x) and not lifetime[SubObject]=(.r=x). The checker is trying to compensate for incorrect valueflow. The valueflow should be corrected. These extra checks here can cause problems later on, and are harder to debug.

At least there is a FN for this:

Its the same problem. It tries to assign the lifetime to i which is just an integer so it doesnt capture a lifetime.

Since this doesnt fix the valueflow it introduces a new FP for cases like this:

struct A { int& r; int i; };
A f() {
    int x = 0;
    A a{.i= x};
    return a;
}

You are looking in the right place in valueflow where this needs to be corrected. The args vector needs to be adjusted for the desginated initailizers so it puts each element in the same order as the struct by matching the names of the variables. We could set the token to null for when its missing an initializer but we might need to update the LifetimeStore::forEach to handle null token.

[danmar]

How about;

Suggested change

	if (tokvalue->str() == "=" && Token::simpleMatch(tokvalue->astOperand1(), ".") && !tokvalue->astOperand1()->astOperand2())
	if (tokvalue->str() == "=" && tokvalue->astOperand1() && astOperand1()->astOperand2()->isUnaryOp("."))

[danmar]

it feels a bit like we should create a "isDesignatedInitializer" utility function. but well it doesn't seem to be needed much. in premiumaddon we have a utility function..
I don't have a strong opinion; create a "isDesignatedInitializer" utility function if you want.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud