diff --git a/change_notes/2026-08-03-unused-local-function-false-positives.md b/change_notes/2026-08-03-unused-local-function-false-positives.md new file mode 100644 index 000000000..0ca4f62c5 --- /dev/null +++ b/change_notes/2026-08-03-unused-local-function-false-positives.md @@ -0,0 +1,7 @@ +- `A0-1-3`, `RULE-0-2-4` - `UnusedLocalFunction.ql`: + - Improved call-use detection to consider both dynamic call graph targets and direct static call + targets. + - Excluded pure virtual functions from local-function reporting. + - This reduces false positives for: + - private members used in contexts where only static call targets are available, and + - private pure virtual interface contracts. diff --git a/cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll b/cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll index 39715e484..0ad6d2740 100644 --- a/cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll +++ b/cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll @@ -20,6 +20,15 @@ predicate overloadedFunctionIsCalled(Function unusedFunction) { exists(Function f | f = unusedFunction.getAnOverload() and f = getTarget(_)) } +/** + * Checks if `fn` has either a dynamic or static call target. + */ +predicate functionIsCalled(Function fn) { + fn = getTarget(_) + or + exists(FunctionCall staticCall | staticCall.getTarget() = fn) +} + /** Checks if a Function's address was taken. */ predicate addressBeenTaken(Function unusedFunction) { exists(FunctionAccess fa | fa.getTarget() = unusedFunction) @@ -37,29 +46,32 @@ class LocalFunction extends UnusedFunctions::UsableFunction { string localFunctionType; LocalFunction() { - this.(MemberFunction).isPrivate() and - localFunctionType = "Private member" - or - // A function in an anonymous namespace (which is deduced to have internal linkage) - this instanceof AnonymousNamespaceFunction and - not this instanceof MemberFunction and - localFunctionType = "Anonymous namespace" - or - // Class members in anonymous namespaces also have internal linkage. - this instanceof AnonymousNamespaceFunction and - this instanceof MemberFunction and - localFunctionType = "Anonymous namespace class member" - or - // Static functions with internal linkage - this.isStatic() and - // Member functions never have internal linkage - not this instanceof MemberFunction and - // Functions in anonymous namespaces automatically have the "static" specifier added by the - // extractor. We therefore excluded them from this case, and instead report them in the - // anonymous namespace, as we don't know whether the "static" specifier was explicitly - // provided by the user. - not this instanceof AnonymousNamespaceFunction and - localFunctionType = "Static" + not this instanceof PureVirtualFunction and + ( + this.(MemberFunction).isPrivate() and + localFunctionType = "Private member" + or + // A function in an anonymous namespace (which is deduced to have internal linkage) + this instanceof AnonymousNamespaceFunction and + not this instanceof MemberFunction and + localFunctionType = "Anonymous namespace" + or + // Class members in anonymous namespaces also have internal linkage. + this instanceof AnonymousNamespaceFunction and + this instanceof MemberFunction and + localFunctionType = "Anonymous namespace class member" + or + // Static functions with internal linkage + this.isStatic() and + // Member functions never have internal linkage + not this instanceof MemberFunction and + // Functions in anonymous namespaces automatically have the "static" specifier added by the + // extractor. We therefore excluded them from this case, and instead report them in the + // anonymous namespace, as we don't know whether the "static" specifier was explicitly + // provided by the user. + not this instanceof AnonymousNamespaceFunction and + localFunctionType = "Static" + ) } /** Gets the type of local function. */ @@ -74,7 +86,7 @@ module UnusedLocalFunction { query predicate problems(LocalFunction unusedLocalFunction, string message) { not isExcluded(unusedLocalFunction, Config::getQuery()) and // No static or dynamic call target for this function - not unusedLocalFunction = getTarget(_) and + not functionIsCalled(unusedLocalFunction) and // If this is a TemplateFunction or an instantiation of a template, then only report it as unused // if all other instantiations of the template are unused not exists( @@ -88,7 +100,7 @@ module UnusedLocalFunction { | // There exists an instantiation which is called functionFromInstantiatedTemplate.isConstructedFrom(functionFromUninstantiatedTemplate) and - functionFromInstantiatedTemplate = getTarget(_) + functionIsCalled(functionFromInstantiatedTemplate) ) and // A function is defined as "used" if any one of the following holds true: // - It's an explicitly deleted functions e.g. =delete diff --git a/cpp/common/test/rules/unusedlocalfunction/test.cpp b/cpp/common/test/rules/unusedlocalfunction/test.cpp index 9ce37dcaa..c784990ab 100644 --- a/cpp/common/test/rules/unusedlocalfunction/test.cpp +++ b/cpp/common/test/rules/unusedlocalfunction/test.cpp @@ -157,4 +157,44 @@ class C3 { public: void f() {} // COMPLIANT - public external linkage }; -} // namespace N1 \ No newline at end of file +} // namespace N1 + +class UsedFromFriendContext { + friend class FriendCaller; + +private: + static void create() {} // COMPLIANT - called from a friend. + void open() {} // COMPLIANT - called from a friend. +}; + +class FriendCaller { +public: + void use() { + UsedFromFriendContext::create(); + UsedFromFriendContext ctx; + ctx.open(); + } +}; + +void test_friend_calls() { + FriendCaller caller; + caller.use(); +} + +class PureVirtualBase { +public: + void callImpl() { impl(); } + +private: + virtual void impl() = 0; // COMPLIANT - pure virtual contract. +}; + +class PureVirtualDerived : public PureVirtualBase { +private: + void impl() override {} +}; + +void test_pure_virtual_private_member() { + PureVirtualDerived derived; + derived.callImpl(); +} \ No newline at end of file