llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.76k stars 10.97k forks source link

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

Open llvmbot opened 5 years ago

llvmbot commented 5 years ago
Bugzilla Link 41141
Version unspecified
OS Linux
Reporter LLVM Bugzilla Contributor
CC @mathstuf,@devincoughlin,@EugeneZelenko,@haoNoQ,@pavelkryukov

Extended Description

Given the following testcase "split.cpp":

"""

include

include

include

include <boost/algorithm/string.hpp>

void test() { std::string input("A\tB"); std::vector 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, std::allocator<std::__cxx11::basic_string > >, std::cxx11::basic_string, boost::algorithm::detail::is_any_ofF>' 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>' ::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>' return detail::token_finderF( 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' 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' 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>' return detail::token_finderF( 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, std::allocator<std::__cxx11::basic_string > >, std::cxx11::basic_string, boost::algorithm::detail::is_any_ofF>' 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>' ::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' return detail::token_finderF( 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' return detail::token_finderF( 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 } ^

pavelkryukov commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#44247

pavelkryukov commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#44065

haoNoQ 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.

pavelkryukov 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: llvm/llvm-bugzilla-archive#44065

mathstuf commented 4 years ago

Reproducer case OK, even a minimized version was 2.5 MB when preprocessed. Luckily, it compresses well. This is a close-to-minimal test case. I can probably remove some artifacts from the project it lives in with more time if needed.

mathstuf 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.

mathstuf 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).

haoNoQ 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 ...)?

pavelkryukov commented 4 years ago

Bug llvm/llvm-bugzilla-archive#44247 has been marked as a duplicate of this bug.

llvmbot commented 5 years ago

assigned to @haoNoQ