Divide by zero in serialization when writing a sequence of objects that have empty UPCXX_SERIALIZED_{FIELDS,VALUES}

Issue #473 resolved
Amir Kamil created an issue

Reproducer:

#include <upcxx/upcxx.hpp>

struct A {
  int x = 3;
  UPCXX_SERIALIZED_FIELDS()
};

struct B {
  int y = 4;
  UPCXX_SERIALIZED_VALUES()
};

struct C {
  A as[7];
  struct upcxx_serialization {
    template<typename Writer>
    static void serialize(Writer &w, const C &c) {
      w.write_sequence(c.as, c.as + 7, 7);
    }
    template<typename Reader>
    static C* deserialize(Reader &r, void *storage) {
      C *c = new(storage) C;
      r.template read_sequence_into<A>(c->as, 7);
      return c;
    }
  };
};

int main() {
  upcxx::init();

  A a;
  upcxx::rpc(0, [](const A &x) { return x; }, a).wait(); // succeeds

  B b;
  upcxx::rpc(0, [](const B &x) { return x; }, b).wait(); // succeeds

  std::vector<A> as{5};
  upcxx::rpc(0, [](const std::vector<A> &v) { return v; }, as).wait(); // fails on reply

  std::array<B, 5> bs;
  upcxx::rpc(0, [](const std::array<B, 5> &v) { return v; }, bs).wait(); // fails on reply

  C c;
  c = upcxx::serialization_traits<C>::deserialized_value(c); // fails

  upcxx::finalize();
}

Backtrace from the failure on line 45:

*** Caught a fatal signal (proc 0): SIGFPE(8)
[0] Invoking LLDB for backtrace...
[0] /usr/bin/lldb -p 30505 -o 'bt all' -o quit
[0] (lldb) process attach --pid 30505
[0] Process 30505 stopped
[0] * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
[0]     frame #0: 0x00007fff205d1f86 libsystem_kernel.dylib`swtch_pri + 10
[0] libsystem_kernel.dylib`swtch_pri:
[0] ->  0x7fff205d1f86 <+10>: retq   
[0]     0x7fff205d1f87 <+11>: nop    
[0] 
[0] libsystem_kernel.dylib`swtch:
[0]     0x7fff205d1f88 <+0>:  movq   %rcx, %r10
[0]     0x7fff205d1f8b <+3>:  movl   $0x100003c, %eax          ; imm = 0x100003C 
[0] Target 0: (a.out) stopped.
[0] 
[0] Executable module set to "/Users/akamil/work/upcxx2/a.out".
[0] Architecture set to: x86_64h-apple-macosx-.
[0] (lldb) bt all
[0] * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
[0]   * frame #0: 0x00007fff205d1f86 libsystem_kernel.dylib`swtch_pri + 10
[0]     frame #1: 0x00007fff2060407c libsystem_pthread.dylib`cthread_yield + 11
[0]     frame #2: 0x0000000109fd8b93 a.out`gasneti_system_redirected_coprocess(cmd="/usr/bin/lldb -p 30505 -o 'bt all' -o quit", stdout_fd=3) at gasnet_tools.c:1353:9
[0]     frame #3: 0x0000000109fd881a a.out`gasneti_bt_lldb(fd=3) at gasnet_tools.c:1463:12
[0]     frame #4: 0x0000000109fd317e a.out`gasneti_print_backtrace(fd=2) at gasnet_tools.c:1813:22
[0]     frame #5: 0x0000000109fd3be0 a.out`_gasneti_print_backtrace_ifenabled(fd=2) at gasnet_tools.c:1946:12
[0]     frame #6: 0x000000010a0e3876 a.out`gasneti_defaultSignalHandler(sig=8) at gasnet_internal.c:938:7
[0]     frame #7: 0x00007fff2064ad7d libsystem_platform.dylib`_sigtramp + 29
[0]     frame #8: 0x0000000109f71ad6 a.out`unsigned long upcxx::detail::serialization_writer<false>::write_sequence_<A, A const*, true, upcxx::detail::storage_size<0ul, 1ul> >(this=0x00007ffee5ca7368, beg=0x00007ffee5ca7918, end=0x00007ffee5ca7934, (null)=integral_constant<bool, true> @ 0x00007ffee5ca7258, n=7, elt_ub=storage_size<0, 1> @ 0x00007ffee5ca7250) at serialization.hpp:549:43
[0]     frame #9: 0x0000000109f71948 a.out`unsigned long upcxx::detail::serialization_writer<false>::write_sequence<A const*>(this=0x00007ffee5ca7368, beg=0x00007ffee5ca7918, end=0x00007ffee5ca7934, n=7) at serialization.hpp:619:31
[0]     frame #10: 0x0000000109f714cf a.out`void C::upcxx_serialization::serialize<upcxx::detail::serialization_writer<false> >(w=0x00007ffee5ca7368, c=0x00007ffee5ca7918) at issue473.cpp:18:9
[0]     frame #11: 0x0000000109f5c708 a.out`upcxx::detail::serialization_traits_deserialized_value<C, void>::deserialized_value(x=0x00007ffee5ca7918) at serialization.hpp:1374:9
[0]     frame #12: 0x0000000109f5bd68 a.out`main at issue473.cpp:45:7
[0]     frame #13: 0x00007fff20621621 libdyld.dylib`start + 1
[0]     frame #14: 0x00007fff20621621 libdyld.dylib`start + 1
[0] (lldb) quit
Floating point exception: 8

The offending code in serialization.hpp (line 549):

          std::size_t n0 = (edge_ - size0)/elt_size;

We do not explicitly prohibit empty UPCXX_SERIALIZED_{FIELDS,VALUES}(). Technically, passing no arguments to a variadic macro parameter isn’t allowed in C++17 earlier but is allowed in C++20. In practice, since C99 allows this, compilers generally allow it.

I don’t see any reason to prohibit this. Rather, I think serialization should check for this degenerate case and handle it properly.

Comments (3)

  1. Colin MacLean

    I actually just experienced this issue when trying to compare a copy vs move with the wrapper proposal (Issue #472) for use with futures. I created a type that initialized a new randomly generated variable on default construction or copy to test the effects of a non-trivial copy constructor, and so didn’t actually need to communicate anything. This sort of empty serialization could potentially have legitimate uses when wanting to inject a locally generated object into a future. I agree there is no reason to prohibit this and that it should be handled properly.

  2. Log in to comment