false positive - cpp/unused-static-function

[ryao]

Description of the false positive

Every instance of this on the OpenZFS codebase that I have checked appears to be a false positive.

For example, it reports rrw_enter_read_impl() is unused, when it is used in rrw_enter_read(), which is the next function definition:

https://github.com/ryao/zfs/security/code-scanning/184

The same happens with abd_fletcher_4_impl():

https://github.com/ryao/zfs/security/code-scanning/203

And again with zfs_rangelock_enter_writer(), which is used in zfs_rangelock_enter_impl():

https://github.com/ryao/zfs/security/code-scanning/202

Then there is zfs_rangelock_enter_impl(), which is used in zfs_rangelock_enter():

https://github.com/ryao/zfs/security/code-scanning/200

zfs_ace_byteswap(), used in zfs_acl_byteswap():

https://github.com/ryao/zfs/security/code-scanning/199

chksum_run(), used in chksum_benchit():

https://github.com/ryao/zfs/security/code-scanning/198

zap_cursor_init_impl(), used in zap_cursor_init_serialized(), zap_cursor_init() and zap_cursor_init_noprefetch():

https://github.com/ryao/zfs/security/code-scanning/197

vdev_indirect_mapping_entry_for_offset_impl(), used in multiple functions:

https://github.com/ryao/zfs/security/code-scanning/196

vdev_open_children_impl(), used in vdev_open_children():

https://github.com/ryao/zfs/security/code-scanning/195

space_reftree_add_node(), used in space_reftree_add_seg():

https://github.com/ryao/zfs/security/code-scanning/194

I suspect that all 43 reports are false positives, although so far, I have only explicitly checked about half of them. I have glanced at the ones that I did not explicitly check and I would be very surprised if any of those functions are unused since they are almost entirely important functions in the codebase.

Unused static functions tend to be pruned from the codebase fairly quickly, so I suspect that every report against the OpenZFS codebase is a false positive.

This is probably a duplicate of #10442. However, I figured I would file an issue anyway since the OpenZFS codebase should serve as a decent corpus on which to develop a revised query.

[aibaars]

Thanks for reporting this. The OpenZFS code base sounds like a great test case for this query. I think you're right that this is a duplicate of #10442.

[MathiasVP]

Hi @ryao,

Thanks for the issue. I've created an internal issue for the C/C++ team to investigate this.

A small amount of background information: Historically, FPs like this one arise because of issues in creating the database for a project. Thus, it may be that we don't actually have the call to the static function in the database, and so we consider the function "unused".

We'll see if we can make the query more resilient to malformed databases and thus avoid FPs like this.

[ryao]

A small amount of background information: Historically, FPs like this one arise because of issues in creating the database for a project. Thus, it may be that we don't actually have the call to the static function in the database, and so we consider the function "unused".

We'll see if we can make the query more resilient to malformed databases and thus avoid FPs like this.

If issues creating the database are the cause, then that would indicate that other issues are being missed from not having an accurate database. I do not have any control over how the database is generated, so issues in database generation in itself should be a bug that needs to be addressed.

[MathiasVP]

If issues creating the database are the cause, then that would indicate that other issues are being missed from not having an accurate database. I do not have any control over how the database is generated, so issues in database generation in itself should be a bug that needs to be addressed.

Indeed, you're very right. Issues related to database creation can result in both false positives and missed results. In general, database creation is a very difficult process (since it involves understanding all the weird and wonderful dialects of C/C++ from various compilers), so it's very rare that the CodeQL database will be perfect. For this reason, some queries need extra hardening to prevent nonsensical results from inconsistent databases. This query, in particular, is very sensitive to such database inconsistencies.

I can assure you that we're continuously improving the database creation process (just like we're improving the analysis).

[ryao]

Should this receive the acknowledged label?

[MathiasVP]

Yeah, it should. I've added it now.