Skip to content

Conversation

@hjelmn
Copy link
Member

@hjelmn hjelmn commented Sep 7, 2016

This commit adds some glue code to support the C++ bindings and
updates the bindings to use the new glue code. This protects our
internal headers (which are C99) from C++. This is done as a quick
workaround to compilation errors when the legacy C++ bindings are
requested.

Fixes #2055

Signed-off-by: Nathan Hjelm [email protected]

@hjelmn
Copy link
Member Author

hjelmn commented Sep 7, 2016

@jsquyres This should be adequate to allow the C++ bindings to be compiled without having to mess with internal headers.

@hjelmn
Copy link
Member Author

hjelmn commented Oct 3, 2016

@jsquyres Any comments on this? It needs to go into 2.0.2 and 2.1.0 so I plan to merge it later today if there are no objections.

@hjelmn hjelmn added this to the v2.1.0 milestone Oct 3, 2016
@jsquyres
Copy link
Member

jsquyres commented Oct 3, 2016

I get compile fails:

  CXX      intercepts.lo
intercepts.cc:29:21: fatal error: cc_glue.h: No such file or directory

@jsquyres
Copy link
Member

jsquyres commented Oct 3, 2016

After fixing intercepts.cc to include cxx_glue.h, I still get moar compile failz:

  CXX      comm.lo
comm.cc: In static member function ‘static int MPI::Comm::do_create_keyval(int (*)(MPI_Comm, int, void*, void*, void*, int*), int (*)(MPI_Comm, int, void*, void*), int (*)(const MPI::Comm&, int, void*, void*, void*, bool&), int (*)(MPI::Comm&, int, void*, void*), void*, int&)’:
comm.cc:91:47: error: ‘malloc’ was not declared in this scope
         malloc(sizeof(keyval_intercept_data_t));
                                               ^
comm.cc:116:29: error: ‘free’ was not declared in this scope
         free(cxx_extra_state);
                             ^
make: *** [comm.lo] Error 1
  CXX      datatype.lo
datatype.cc: In static member function ‘static int MPI::Datatype::do_create_keyval(int (*)(MPI_Datatype, int, void*, void*, void*, int*), int (*)(MPI_Datatype, int, void*, void*), int (*)(const MPI::Datatype&, int, void*, const void*, void*, bool&), int (*)(MPI::Datatype&, int, void*, void*), void*, int&)’:
datatype.cc:55:47: error: ‘malloc’ was not declared in this scope
         malloc(sizeof(keyval_intercept_data_t));
                                               ^
datatype.cc:80:29: error: ‘free’ was not declared in this scope
         free(cxx_extra_state);
                             ^
make: *** [datatype.lo] Error 1
  CXX      win.lo
win.cc: In static member function ‘static int MPI::Win::do_create_keyval(int (*)(MPI_Win, int, void*, void*, void*, int*), int (*)(MPI_Win, int, void*, void*), int (*)(const MPI::Win&, int, void*, void*, void*, bool&), int (*)(MPI::Win&, int, void*, void*), void*, int&)’:
win.cc:64:47: error: ‘malloc’ was not declared in this scope
         malloc(sizeof(keyval_intercept_data_t));
                                               ^
win.cc:89:29: error: ‘free’ was not declared in this scope
         free(cxx_extra_state);
                             ^
make: *** [win.lo] Error 1
make: Target `all' not remade because of errors.

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compile errors now fixed, but when I run the cxx test suite in ompi-tests, it segvs in a type test. Here's a bt from the corefile:

(gdb) bt
#0  0x00007fffffffc318 in ?? ()
#1  0x00002aaaaaac4c8c in ompi_mpi_cxx_type_copy_attr_intercept (
    oldtype=0x64f4a0 <ompi_mpi_byte>, keyval=13, extra_state=0x7fffffffc2f0, 
    attribute_val_in=0x7fffffffc318, attribute_val_out=0x7fffffffbf90, 
    flag=0x7fffffffc00c) at intercepts.cc:404
#2  0x00002aaaaacf64a9 in ompi_attr_copy_all (type=TYPE_ATTR, 
    old_object=0x64f4a0 <ompi_mpi_byte>, new_object=0x828d90, oldattr_hash=0x829410, 
    newattr_hash=0x8292f0) at attribute/attribute.c:877
#3  0x00002aaaaad8b94e in PMPI_Type_dup (type=0x64f4a0 <ompi_mpi_byte>, 
    newtype=0x7fffffffc0a8) at ptype_dup.c:74
#4  0x000000000040db2f in MPI::Datatype::Dup (this=0x7fffffffc280)
    at /home/jsquyres/bogus/include/openmpi/ompi/mpi/cxx/datatype_inln.h:266
#5  0x000000000042bcc0 in typekeyval () at typekeyval.cc:154
#6  0x000000000040c2fe in main (argc=1, argv=0x7fffffffc548) at mpi2c++_test.cc:221

It failed on this line:

403       ret = kid->cxx_copy_fn(cxx_datatype, keyval, kid->extra_state,
404 =>                           attribute_val_in, attribute_val_out, bflag);

It looks like the kid extra state has a bogus value for several of the callbacks:

(gdb) p kid
$1 = (MPI::Datatype::keyval_intercept_data_t *) 0x7fffffffc2f0
(gdb) p *kid
$2 = {c_copy_fn = 0x0, c_delete_fn = 0x1600000016, cxx_copy_fn = 0x7fffffffc318, cxx_delete_fn = 0x200000001, extra_state = 0xfffffff000000002}

This problem does not occur on master HEAD; only on this PR. So something must have changed w.r.t. the C++ bindings besides just isolation...?

This commit adds some glue code to support the C++ bindings and
updates the bindings to use the new glue code. This protects our
internal headers (which are C99) from C++. This is done as a quick
workaround to compilation errors when the legacy C++ bindings are
requested.

Fixes open-mpi#2055

Signed-off-by: Nathan Hjelm <[email protected]>
@jsquyres
Copy link
Member

jsquyres commented Oct 4, 2016

@hppritcha Good to go when CI finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants