Naios / function2

Improved and configurable drop-in replacement to std::function that supports move only types, multiple overloads and more
http://naios.github.io/function2
Boost Software License 1.0
539 stars 47 forks source link

Potential null pointer dereference in Function2 #52

Open TheBitshifter opened 2 years ago

TheBitshifter commented 2 years ago

@Naios


Commit Hash

Function2 tags 4.2.0 and 4.2.1, hashes: 02ca99831de59c7c3a4b834789260253cace0ced f569a63cfe369df867a1a4d17aaa12269156536c

Expected Behavior

Test case pasted below compiles without errors with g++-11 -O1 -Wnull-dereference -c -o null-deref.o null-deref.cpp

Actual Behavior

Test case given below produces a warning for a null pointer dereference:

g++-11 -O1 -Wnull-dereference   -c -o null-deref.o null-deref.cpp
null-deref.cpp: In static member function ‘static Ret fu2::abi_400::detail::type_erasure::invocation_table::function_trait<Ret(Args ...)>::internal_invoker<T, IsInplace>::invoke(fu2::abi_400::detail::type_erasure::data_accessor*, std::size_t, Args ...) [with T = fu2::abi_400::detail::type_erasure::box<false, Foo::bar()::<lambda()>, std::allocator<Foo::bar()::<lambda()> > >; bool IsInplace = true; Ret = int; Args = {}]’:
null-deref.cpp:14:44: warning: potential null pointer dereference [-Wnull-dereference]
   14 |     CallbackT c = std::move([=]() { return x; });
      |                                            ^
null-deref.cpp:14:44: warning: potential null pointer dereference [-Wnull-dereference]

Steps to Reproduce

Test case file:

//#include "function2.hpp"
#include "function2-upstream-master.hpp"
#include <functional>
#include <iostream>
//#include "function2.hpp-fixed"

using namespace std;

class Foo {
public:
  using CallbackT = fu2::unique_function<int(void)>;
  void bar(void) {
    int x = 10;
    CallbackT c = std::move([=]() { return x; });
  }
};

int main(void) {
  Foo a;
  a.bar();
  return 0;
}

Compile with the command:

g++-11 -O1 -Wnull-dereference   -c -o null-deref.o null-deref.cpp

This issue can be solved by the following patch to function2.hpp

Patch to tag 4.2.0
--- function2.hpp   2022-06-16 19:56:34.645027778 +0000
+++ function2.hpp-fixed 2022-06-16 15:54:34.989443571 +0000
@@ -573,6 +573,7 @@
         auto obj = retrieve<T>(std::integral_constant<bool, IsInplace>{},      \
                                data, capacity);                                \
         auto box = static_cast<T CONST VOLATILE*>(obj);                        \
+       if(box == nullptr) throw_or_abort##NOEXCEPT(std::integral_constant<bool, false>{});      \
         return invocation::invoke(                                             \
             static_cast<std::decay_t<decltype(box->value_)> CONST VOLATILE     \
                             REF>(box->value_),                                 \

Your Environment

Naios commented 2 years ago

Thank you for your report. I'm quite certain that this is a false positive. I will take a closer look into it.