Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

etl/delegate: fix accident creation of a delegate to an rvalue delegate when copying/assigning from delegate with mismatching signature #965

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

VladimirP1
Copy link

Original problem, how to reproduce:

#include <etl/delegate.h>

namespace ctx {
struct os {};
struct any {
  any() = default;
  any(os) {}
};
} // namespace ctx

struct Test {
    etl::delegate<void(ctx::os)> GetCb() {
      return etl::delegate<void(ctx::any)>::create<Test,
      &Test::CbAnyCtx>(
          *this);
    }

private:
  void CbAnyCtx(ctx::any ctx) {}
};

int main() {
  Test().GetCb()(ctx::os{});
  etl::delegate<void(void)> f = []() {};
  return 0;
}

This compiles and does not look suspicious at the first glance but results in a segmentation fault at runtime.

What happens here:

  1. A delegate is created etl::delegate<void(ctx::any ctx)>::create<Test, &Test::CbAnyCtx>(*this);
  2. But we need a etl::delegate<void(ctx::os ctx)>, the compiler selects the constructor from const TLambda&.
  3. Note that etl::is_same<etl::delegate<TReturn(TParams...)>, TLambda>::value is false because the signatures of the delegates do not match
  4. The newly created delegate is returned and the first delegate is destroyed. Now the second delegate holds a dangling pointer

This PR attempts to fix this issue by using a reliable method for checking if a functor is a delegate

…te when copying/assigning from delegate with mismatching signature
@jwellbelove
Copy link
Contributor

Thanks, I'll take a look at that when I get back from Spain later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants