Divide by zero in serialization when writing a sequence of objects that have empty UPCXX_SERIALIZED_{FIELDS,VALUES}
Issue #473
resolved
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)
-
-
- changed status to resolved
Fix serialization of objects with empty UPCXX_SERIALIZED_{FIELDS,VALUES}. Fixes
#473.→ <<cset e4cb320e58e2>>
-
Merge pull request #352 into develop
- issue473:
Add ChangeLog entry for issue 473.
Fix serialization of objects with empty UPCXX_SERIALIZED_{FIELDS,VALUES}. Fixes
#473.
→ <<cset d0e5c99504d2>>
- issue473:
Add ChangeLog entry for issue 473.
Fix serialization of objects with empty UPCXX_SERIALIZED_{FIELDS,VALUES}. Fixes
- Log in to comment
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.