gamesh411 added a comment.
In D83717#2279263 <https://reviews.llvm.org/D83717#2279263>, @aaron.ballman
wrote:
> One of the concerns I have with this not being a flow-sensitive check is that
> most of the bad situations are not going to be caught by the clang-tidy
> version of the check. The CERT rules show contrived code examples, but the
> more frequent issue looks like:
>
> void cleanup(struct whatever *ptr) {
> assert(ptr); // This potentially calls abort()
> free(ptr->buffer);
> free(ptr);
> }
>
> void some_cleanup_func(void) {
> for (size_t idx = 0; idx < GlobalElementCount; ++idx) {
> cleanup(GlobalElement[idx]);
> }
> }
>
> void some_exit_handler(void) {
> ...
> some_cleanup_func();
> ...
> }
>
> The fact that we're not looking through the call sites (even without cross-TU
> support) means the check isn't going to catch the most problematic cases. You
> could modify the called function collector to gather this a bit better, but
> you'd issue false positives in flow-sensitive situations like:
>
> void some_cleanup_func(void) {
> for (size_t idx = 0; idx < GlobalElementCount; ++idx) {
> struct whatever *ptr = GlobalElement[idx];
> if (ptr) {
> // Now we know abort() won't be called
> cleanup(ptr);
> }
> }
> }
>
> Have you run this check over any large code bases to see if it currently
> catches any true positive diagnostics?
I have tried llvm, tmux, curl and tried codesearch.com to look for other
sources containing `atexit`, but no results were found. So it is hard to see
whether this flow-sensitive approach would result in many false positives.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83717/new/
https://reviews.llvm.org/D83717
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
