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

virtual serialization: "Error, could not find corresponding entry in derived for base" when serializing non constructible classes #143

Closed
cz4rs opened this issue Oct 19, 2020 · 12 comments · Fixed by #144
Assignees

Comments

@cz4rs
Copy link
Contributor

cz4rs commented Oct 19, 2020

Scenario based on serialization of vt::objgroup::holder::HolderBase (required for vt::objgroup::ObjGroupManager component).

Given following class hierarchy:

struct HolderBase {
  checkpoint_virtual_serialize_root()

  virtual ~HolderBase() = default;

  template <typename Serializer>
  void serialize(Serializer& s) {}
};

template <typename ObjT>
struct HolderObjBase : HolderBase {
  checkpoint_virtual_serialize_derived_from(HolderBase)

  virtual ObjT* get() = 0;

  template <typename Serializer>
  void serialize(Serializer& s) {}
};

template <typename ObjT>
struct HolderBasic final : HolderObjBase<ObjT> {
  checkpoint_virtual_serialize_derived_from(HolderObjBase<ObjT>)

  ObjT* get() override { return obj_; }
  ObjT* obj_ = nullptr;

  template <typename Serializer>
  void serialize(Serializer& s) {}
};

When virtual serialization is attempted:

  std::unique_ptr<HolderBase> ptr = std::make_unique<HolderBasic<int>>();
  checkpoint::getMemoryFootprint(ptr);

Test crashes with following error:

-----------------------------------------------------------------------
Assertion failed in Checkpoint library:
-----------------------------------------------------------------------
   Reason: Error, could not find corresponding entry in derived for base
Condition: false
     File: ../src/checkpoint/dispatch/vrt/serializer_registry.h
     Line: 156
     Func: getBaseIdx
-----------------------------------------------------------------------

Backtrace after crash:

(...)
#4  in checkpoint::debug::assertOut (cond=0x5555555f6de6 "false", str=0x5555555f6da8 "Error, could not find corresponding entry in derived for base", 
    file=0x5555555f6a90 "../src/checkpoint/dispatch/vrt/serializer_registry.h", line=156, func=0x5555555f6d97 "getBaseIdx") at ../src/checkpoint/common.h:103
#5  in checkpoint::dispatch::vrt::serializer_registry::getBaseIdx<checkpoint::tests::unit::HolderObjBase<int> > (base_idx=0)
    at ../src/checkpoint/dispatch/vrt/serializer_registry.h:156
#6  in checkpoint::tests::unit::HolderObjBase<int>::_checkpointDynamicSerialize (this=0x5555556634a0, s=0x7fffffffd7b0, base_ser_idx=0, expected_idx=1)
    at ../tests/unit/test_footprinter.cc:543
#7  in checkpoint::tests::unit::HolderBasic<int>::_checkpointDynamicSerialize (this=0x5555556634a0, s=0x7fffffffd7b0, base_ser_idx=0, expected_idx=-1)
    at ../tests/unit/test_footprinter.cc:553
#8  in checkpoint::dispatch::vrt::virtualSerialize<checkpoint::tests::unit::HolderBase, checkpoint::Footprinter> (base=@0x7fffffffd4c8: 0x5555556634a0, s=...)
    at ../src/checkpoint/dispatch/vrt/virtual_serialize.h:66
(...)
@cz4rs
Copy link
Contributor Author

cz4rs commented Oct 19, 2020

@lifflander @PhilMiller can you please have a look at this?
I have pushed a commit that reproduces this in #144 (the test case can probably be further simplified and moved into test_virtual_serialize.cc), but I have not been able to fix this so far

@PhilMiller
Copy link
Member

I'm seeing the exact same issue in some code I'm working on

@PhilMiller
Copy link
Member

PhilMiller commented Oct 19, 2020

I just checked that this is not the result of the changes in #133 - checking out its predecessor and running the test on that (modified to use the older-style macros with more arguments) still fails

@PhilMiller
Copy link
Member

It actually still fails for me even when commenting out the templating on the classes in question!

@PhilMiller
Copy link
Member

It seems that the intermediate class in the inheritance hierarchy plays some role in the problem - when I skip that, the test passes.

@cz4rs
Copy link
Contributor Author

cz4rs commented Oct 19, 2020

It seems that the intermediate class in the inheritance hierarchy plays some role in the problem - when I skip that, the test passes.

also, when the get() method in the middle class in not abstract, it passes ok

-  virtual ObjT* get() = 0;
+  virtual ObjT* get() { return nullptr; };

@cz4rs
Copy link
Contributor Author

cz4rs commented Oct 19, 2020

ok, so clearly it's not a template issue, seems like an abstract class in the middle is not ok
I will try to play around with different hierarchies and produce a truly minimal example

@PhilMiller
Copy link
Member

I think that potentially is the hint that we need to unravel this - there's maybe an instantiation failure around the constructor functor, which the compiler doesn't complain about because it finds something less specialized to instantiate instead (SFINAE)

@PhilMiller
Copy link
Member

Perhaps dispatch::Reconstructor<ObjT>::constructAllowFail(buf) in object_registry.h isn't behaving as intended

@PhilMiller
Copy link
Member

Nope, looks like that's designed to always instantiate, and fail an assertion if it's ever called

@lifflander
Copy link
Contributor

I have a fix in place now on #144

lifflander added a commit that referenced this issue Oct 19, 2020
@cz4rs cz4rs changed the title virtual serialization: "Error, could not find corresponding entry in derived for base" when serializing templated classes virtual serialization: "Error, could not find corresponding entry in derived for base" when serializing non constructible classes Oct 19, 2020
@cz4rs
Copy link
Contributor Author

cz4rs commented Oct 19, 2020

renaming after the fix, since templates were not the source of the issue here

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 a pull request may close this issue.

3 participants