boost-ext / te

C++17 Run-time Polymorphism (Type Erasure) library
451 stars 38 forks source link

Non default constructible object is created and returned, somehow #42

Open fabiorossetto opened 5 months ago

fabiorossetto commented 5 months ago

Expected Behavior

See the attached minimal reproducible example. SimpleCall returns an object of type NoDefaultCtor. Somehow, the code compiles. This doesn't make sense because how can NoDefaultCtor be constructed. value, which should be 7, is 0 instead.

Actual Behavior

Steps to Reproduce the Problem

#include <cstdio>
#include <https://raw.githubusercontent.com/boost-experimental/te/master/include/boost/te.hpp>
#include <cassert>

namespace te = boost::te;

struct NoDefaultCtor {
    NoDefaultCtor() = delete;

    int value{7};
};

struct SimpleCall {
  constexpr inline auto run() {
    return te::call<NoDefaultCtor>([](auto &self) {}, *this);
  }
};

struct Test {};

int main() {
  // Option 1 or 2 is required on some compilers
  te::poly<SimpleCall> f{Test{}};
  NoDefaultCtor s = f.run();
  return s.value;
}

Specifications

Tried on clang 16.0.0 and GCC 10

fabiorossetto commented 5 months ago

I think I found the problem. In call_impl, the function stored in the vtable is reinterpret casted like this: reinterpret_cast<R (*)(void *, Ts...)>(self.vptr[N - 1]). So I can actually return whatever:

struct SimpleCall {
  constexpr inline int run() {
    return te::call<int>([](auto &self) { return std::string{"not an int"}; }, *this);
  }
};

That seems dangerous to me...

I also don't understand why call takes the return type as argument. Can't the return type be inferred by TExpr using std::invoke_result?

krzysztof-jusiak commented 5 months ago

Thanks for your investigation @fabiorossetto and for sharing your findings. I'm pretty sure there should be static assert there to ensure that the cast is valid, likely should be a bit_cast in c++20 too. I will add it as indeed its dangerous as it is. Regarding the result type in call, that's required as the lambda takes template arguments which are known later on, otherwise the deduction could fail if poked with wrong types similary to std::invoke.

fabiorossetto commented 4 months ago

@krzysztof-jusiak I'd be happy to contribute to the project. I can understand almost all the code, but there are certain parts I cannot follow. In order for me to push a fix I'd need some support with understanding certain parts of the code.

Let me know if you'd be interested in receiving external support with the project!

krzysztof-jusiak commented 4 months ago

Hi @fabiorossetto . Would absolutely love it and will do my best to help. Thank you!

fabiorossetto commented 4 months ago

@krzysztof-jusiak Thank you Krys. Let me know if there is any way I can write you in private with my questions

krzysztof-jusiak commented 4 months ago

Sure, feel free to drop me a line at kris@jusiak.net. Thanks.