Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

New apparent clang-tidy false positive from code using boost::is_any_of #40111

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR41141
Status NEW
Importance P normal
Reported by Christopher Saunders (csaunders@illumina.com)
Reported on 2019-03-19 08:26:11 -0700
Last modified on 2020-03-24 10:58:28 -0700
Version unspecified
Hardware PC Linux
CC alexfh@google.com, bugzilla.llvm@me.benboeckel.net, dcoughlin@apple.com, djasper@google.com, eugene.zelenko@gmail.com, klimek@google.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com, pavel.kryukov@phystech.edu
Fixed by commit(s)
Attachments boost-analyze-fp-repro.cxx.gz (289344 bytes, application/gzip)
Blocks
Blocked by
See also
Given the following testcase "split.cpp":

"""
#include <iostream>
#include <string>
#include <vector>

#include <boost/algorithm/string.hpp>

void test() {
  std::string input("A\tB");
  std::vector<std::string> result;
  boost::split(result, input, boost::is_any_of("\t"));
  assert(result.size()==2u);
}
"""

..run in clang-tidy as follows:

"""
clang+llvm-8.0.0-rc5-x86_64-linux-gnu-ubuntu-14.04/bin/clang-tidy -checks=-
*,clang-analyzer-cplusplus.NewDeleteLeaks split.cpp -- -I
/home/csaunders/opt/x86_64-linux/boost-1.69.0/include
"""

clang-tidy version 8.0.0rc5 now issues two warnings which were not issued by
version 7.0.1:

1. /home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:135:25:
warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]
2. /home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:139:17:
warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]

(the full message is included below).

I'm not very familiar with the internals of the corresponding boost code, but
doing my best to trace through it, there does not appear to be a leak. This
issue came up due to a huge number of such warnings being issued on a large
project with the llvm 8 release candidate that were not apparent with llvm 7 --
we're trying to understand whether this is a new clang-tidy issue, and if not,
what possible workarounds exist.

Full clang-tidy message:

2 warnings generated.
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:135:25:
warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]
                    if(!use_fixed_storage(m_Size) && m_Storage.m_dynSet!=0)
                        ^
/home/csaunders/tmp/split/split.cpp:11:3: note: Calling
'split<std::vector<std::__cxx11::basic_string<char>,
std::allocator<std::__cxx11::basic_string<char> > >,
std::__cxx11::basic_string<char>, boost::algorithm::detail::is_any_ofF<char>>'
  boost::split(result, input, boost::is_any_of("\t"));
  ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/split.hpp:149:17: note: Calling
'token_finder<boost::algorithm::detail::is_any_ofF<char>>'
                ::boost::algorithm::token_finder( Pred, eCompress ) );
                ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/finder.hpp:219:20: note: Calling
constructor for 'token_finderF<boost::algorithm::detail::is_any_ofF<char>>'
            return detail::token_finderF<PredicateT>( Pred, eCompress );
                   ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/finder.hpp:554:25: note: Calling
copy constructor for 'is_any_ofF<char>'
                        m_Pred(Pred), m_eCompress(eCompress) {}
                        ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:114:21: note:
Taking false branch
                    if(use_fixed_storage(m_Size))
                    ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:123:44: note:
Memory is allocated
                        m_Storage.m_dynSet=new set_value_type[m_Size];
                                           ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/finder.hpp:554:25: note: Returning
from copy constructor for 'is_any_ofF<char>'
                        m_Pred(Pred), m_eCompress(eCompress) {}
                        ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/finder.hpp:219:20: note: Returning from
constructor for 'token_finderF<boost::algorithm::detail::is_any_ofF<char>>'
            return detail::token_finderF<PredicateT>( Pred, eCompress );
                   ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/split.hpp:149:17: note: Returned
allocated memory
                ::boost::algorithm::token_finder( Pred, eCompress ) );
                ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/split.hpp:149:51: note: Calling
'~is_any_ofF'
                ::boost::algorithm::token_finder( Pred, eCompress ) );
                                                  ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:135:25: note:
Potential memory leak
                    if(!use_fixed_storage(m_Size) && m_Storage.m_dynSet!=0)
                        ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:139:17:
warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]
                }
                ^
/home/csaunders/tmp/split/split.cpp:11:3: note: Calling
'split<std::vector<std::__cxx11::basic_string<char>,
std::allocator<std::__cxx11::basic_string<char> > >,
std::__cxx11::basic_string<char>, boost::algorithm::detail::is_any_ofF<char>>'
  boost::split(result, input, boost::is_any_of("\t"));
  ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/split.hpp:149:17: note: Calling
'token_finder<boost::algorithm::detail::is_any_ofF<char>>'
                ::boost::algorithm::token_finder( Pred, eCompress ) );
                ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/finder.hpp:219:55: note: Calling copy
constructor for 'is_any_ofF<char>'
            return detail::token_finderF<PredicateT>( Pred, eCompress );
                                                      ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:114:21: note:
Taking false branch
                    if(use_fixed_storage(m_Size))
                    ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:123:44: note:
Memory is allocated
                        m_Storage.m_dynSet=new set_value_type[m_Size];
                                           ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/finder.hpp:219:55: note: Returning from
copy constructor for 'is_any_ofF<char>'
            return detail::token_finderF<PredicateT>( Pred, eCompress );
                                                      ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/finder.hpp:219:55: note: Calling
'~is_any_ofF'
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:135:51: note:
Left side of '&&' is false
                    if(!use_fixed_storage(m_Size) && m_Storage.m_dynSet!=0)
                                                  ^
/home/csaunders/opt/x86_64-linux/boost-
1.69.0/include/boost/algorithm/string/detail/classification.hpp:139:17: note:
Potential memory leak
                }
                ^
Quuxplusone commented 4 years ago

_Bug 44247 has been marked as a duplicate of this bug._

Quuxplusone commented 4 years ago

Ok, now that it has been bumped and i didn't see this one before:

I can no longer reproduce this problem. It might have something to do with my configuration (version of boost or the C++ standard library).

Does anybody have a preprocessed file with the bug (i.e., clang -E ...)?

Quuxplusone commented 4 years ago

Does anybody have a preprocessed file with the bug (i.e., clang -E ...)?

It's not minimized at all, but I'm attaching the case I have (9.0.1 as packaged by Fedora).

Quuxplusone commented 4 years ago

Hrm. It's too large to upload. I'll try and trim down my case to see if I can't get it smaller.

Quuxplusone commented 4 years ago

Attached boost-analyze-fp-repro.cxx.gz (289344 bytes, application/gzip): Reproducer case

Quuxplusone commented 4 years ago

By the way, usually Boost headers are included as "system" headers and Clang-Tidy should ignore them. I've opened a bug here some time ago: https://bugs.llvm.org/show_bug.cgi?id=44065

Quuxplusone commented 4 years ago

Reproduced with -target x86_64-unknown-linux, thanks!

Didn't fully debug it yet but looks like we can't load from m_Size in order to call use_fixed_storage(m_Size) from inside ~is_any_ofF() because symbolic-offset bindings (i.e., writes into the nested array m_fixSet[] in that structure by unknown index) screw us over.

I'll see what I can do.