[clang-tools-extra] [clang-tidy] Add WarnOnSizeOfPointer mode to bugprone-sizeof-expression (PR #94356)

DonĂ¡t Nagy via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 10 10:14:32 PDT 2024


NagyDonat wrote:

I evaluated the effects of the additional change [Generalize the suppression to all sizeof(array[0]) expressions](https://github.com/llvm/llvm-project/pull/94356/commits/ce3a37610228ceb9a9e60575f87886bf8e183493) compared to the earlier version of this PR, and it seems that this tweak eliminates a large amount of false positives:

| Project | New Reports | Resolved Reports |
|---------|-------------|------------------|
| memcached | No reports | No reports 
| tmux | No reports | No reports 
| curl | No reports | [3 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_previous_sizeofexpression_after_most_changes&newcheck=curl_curl-7_66_0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| twin | No reports | No reports 
| vim | No reports | No reports 
| openssl | No reports | [8 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_previous_sizeofexpression_after_most_changes&newcheck=openssl_openssl-3.0.0-alpha7_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| sqlite | No reports | [12 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_previous_sizeofexpression_after_most_changes&newcheck=sqlite_version-3.33.0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| ffmpeg | No reports | [9 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_previous_sizeofexpression_after_most_changes&newcheck=ffmpeg_n4.3.1_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| postgres | No reports | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_previous_sizeofexpression_after_most_changes&newcheck=postgres_REL_13_0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| tinyxml2 | No reports | No reports 
| libwebm | No reports | No reports 
| xerces | No reports | [10 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_previous_sizeofexpression_after_most_changes&newcheck=xerces_v3.2.3_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| bitcoin | No reports | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_previous_sizeofexpression_after_most_changes&newcheck=bitcoin_v0.20.1_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| protobuf | No reports | [4 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_previous_sizeofexpression_after_most_changes&newcheck=protobuf_v3.13.0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| qtbase | No reports | [7 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_previous_sizeofexpression_after_most_changes&newcheck=qtbase_v6.2.0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 
| contour | No reports | No reports 
| openrct2 | No reports | No reports 
| llvm-project | No reports | [6 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=llvm-project_llvmorg-12.0.0_previous_sizeofexpression_after_most_changes&newcheck=llvm-project_llvmorg-12.0.0_new_sizeofexpression_with_full_arrayindexzero_suppression&diff-type=Resolved) 

I randomly checked ~20% of the resolved reports, and they all were false positives (that is, situations where `sizeof(pointer)` is the idiomatic and correct solution), so I'm satisfied with this additional change.

At this point I'm satisfied with the behavior of this check on the real-world code and I'm not planning to add more features within this PR. (There is one known issue which I highlighted in a FIXME in commit [Extend GenericFunctionTest, document a class of FPs with a FIXME](https://github.com/llvm/llvm-project/pull/94356/commits/59bd1b83e4fa89b371f4d1a96c51fc7a1b4ad170) -- but that's significantly less severe than the issues before this PR, so I think it's enough to handle that later.) 

I also answered all the review suggestions, so I'd be grateful for another round of reviews from @PiotrZSL @EugeneZelenko or anybody else who has time for it.

https://github.com/llvm/llvm-project/pull/94356


More information about the cfe-commits mailing list