Fix #15063 FP knownArgument when there is function pointer call

[chrchr-github]

No description provided.

[dmcppcheck]

AI review

The fix works for the reported case, but the same knownArgument false positive remains when the function pointer is an element of a member array, e.g. (p->arr[1])(x, 10) or p->arr[1](x, 10). There the astOperand1() of the call's ( is [, which the extended pattern %name%|(|*|. still does not match.

I found no regressions: TestOther and TestValueFlow pass, and none of my test inputs produced a warning with the PR that base did not produce.

Testing

Built cppcheck and testrunner in pr/ and cppcheck in base/.

./testrunner TestOther TestValueFlow in pr/: 343 tests, 0 failed.

I compared base and PR with --enable=style on hand-written C and C++ files containing function pointer calls of different shapes.

Fixed by the PR (warning in base, none with the PR):

• (p->f)(x, 10)
• (s.f)(x, 10)
• (p[1].f)(x, 10)
• (p->next->f)(x, 10)
• (p->*pm)(x, 10)
• (s.*pm)(x, 10)

No warning in base or PR:

• (*a)(x, 10), (*a[1])(x, 10), (*(a + 1))(x, 10)
• a[0](x, 10), (a[0])(x, 10)
• (x ? a : b)(x, 10), ((fp_t)a)(x, 10), (a)(x, 10)
• (N::f)(x, 10), (::N::f)(x, 10), (&N::f)(x, 10)
• calls through a lambda, (l)(x, 10) and (+[](int a, int b) { ... })(x, 10)

Still a false positive with the PR — /work/scratch/t3.c:

struct S {
    int (*f)(int a, int n);
    int (*arr[2])(int a, int n);
    struct S* next;
};
void use(int);
int k1(struct S* p, int x) { return (p->arr[1])(x, 10); }
int k2(struct S* p, int x) { return p->arr[1](x, 10); }
int k3(struct S* p, int x) { return (p[1].f)(x, 10); }
int k4(struct S* p, int x) { return (p->next->f)(x, 10); }
int k5(struct S s, int x) { return (s.arr[0])(x, 10); }
int k6(struct S* p, int i, int x) { return (p->arr[i])(x, 10); }
/* lines 13-17: genuine comma-operator cases, no knownArgument in base or PR */

Command: ../pr/cppcheck -q --enable=style --template='{file}:{line}:{column}: {id}: {message}' t3.c

t3.c:7:50: knownArgument: Argument 'x,10' to function arr is always 10. It does not matter what value 'x' has.
t3.c:8:48: knownArgument: Argument 'x,10' to function arr is always 10. It does not matter what value 'x' has.
t3.c:11:48: knownArgument: Argument 'x,10' to function arr is always 10. It does not matter what value 'x' has.
t3.c:12:57: knownArgument: Argument 'x,10' to function arr is always 10. It does not matter what value 'x' has.

Base gives the same four warnings, plus the ones for lines 9 and 10 that the PR fixes.

Genuine comma operators: use((x, 10)), use((p->f(x, 1), 10)), (p->f)((x, 10), 2) and use((p->next, 10)) give no knownArgument in either base or PR, so they show no regression but also do not confirm that true positives are kept.

---

Automated review of ee06c45 by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

This still misses calls where the callee expression is an array subscript of a member, because astOperand1() of the call's ( is then [:

struct S { int (*arr[2])(int a, int n); };
int k1(struct S* p, int x) { return (p->arr[1])(x, 10); }
int k2(struct S* p, int x) { return p->arr[1](x, 10); }

Both lines still give knownArgument: Argument 'x,10' to function arr is always 10. It does not matter what value 'x' has. with this PR, as do (s.arr[0])(x, 10) and (p->arr[i])(x, 10).

Adding [ to the pattern would cover these. More generally, a ( that has both astOperand1() and astOperand2() and is not a cast is a call whatever the callee expression looks like, so the list of operators may not be needed at all.

[dmcppcheck]

AI review

I found no new problems. The [ that the earlier review asked for is now in the pattern. With it, (p->f)(x, 10), (p->a[0])(x, 10), (s.g)(x, 10) and (h[0])(x, 10) no longer give knownArgument. The full testrunner passes.

There is one side effect, which I would not block on. A real comma operator inside a single argument, such as (p->f)((x, 10)) or p->a[1]((x, 10)), gave knownArgument on base and gives nothing with the PR. The same expression in a call through a plain name, h((x, 10)), is already unreported on base, so the PR only makes these calls behave like named calls.

Testing

I built cppcheck and testrunner in pr/ and cppcheck in base/.

Test suite: I ran ./testrunner -q. All 5351 tests passed (0 failed, 362 todos).

Comparison with base: I used cppcheck -q --enable=style a.c on this file:

struct S { int (*f)(int n); int (*a[2])(int n); int (*g)(int,int); };
int g1(struct S* p, int x) { return (p->f)((x, 10)); }
int g2(struct S* p, int x) { return p->a[1]((x, 10)); }
int g3(int (*h)(int), int x) { return h((x, 10)); }
int g4(struct S* p, int x) { return (p->a[0])(x, 10); }
int g5(struct S s, int x) { return (s.g)(x, 10); }
int g6(int x) { int (*h[2])(int,int); return (h[0])(x, 10); }

• Base: knownArgument on lines 2, 3, 5, 6 and 7.
• PR: no knownArgument at all.
  • Lines 5–7: these were false positives, and they are fixed.
  • Lines 2–3: these were correct warnings about a real comma operator. The PR loses them, which matches how base already treats line 4 (a call through a plain name gets no warning there).
• Both: the unrelated legacyUninitvar on line 7 appears in base and in the PR.

---

Automated review of 5ea92e6 by Claude. The pull request was built and tested locally.