Author: goodell Date: 2012-11-14 11:20:14 -0600 (Wed, 14 Nov 2012) New Revision: 10610 Modified: mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c mpich2/trunk/test/mpi/datatype/indexed-misc.c Log: fix hindexed pointer-casting bugs Bug report from Dmitry Durnov @ Intel. This patch also includes a fix based on code suggested by Dmitry. No reviewer. Modified: mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c =================================================================== --- mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c 2012-11-12 17:07:38 UTC (rev 10609) +++ mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c 2012-11-14 17:20:14 UTC (rev 10610) @@ -137,9 +137,14 @@ */ if (contig_count == 1) { + const void *disp_arr_tmp; /* no ternary assignment to avoid clang warnings */ + if (dispinbytes) + disp_arr_tmp = &(((const MPI_Aint *)displacement_array)[first]); + else + disp_arr_tmp = &(((const int *)displacement_array)[first]); err = PREPEND_PREFIX(Dataloop_create_blockindexed)(1, (int) old_type_count, - &(((int *)displacement_array)[first]), + disp_arr_tmp, dispinbytes, oldtype, dlp_p, @@ -166,9 +171,14 @@ } if (blksz == blocklength_array[first]) { + const void *disp_arr_tmp; /* no ternary assignment to avoid clang warnings */ + if (dispinbytes) + disp_arr_tmp = &(((const MPI_Aint *)displacement_array)[first]); + else + disp_arr_tmp = &(((const int *)displacement_array)[first]); err = PREPEND_PREFIX(Dataloop_create_blockindexed)(icount-first, blksz, - &(((int *)displacement_array)[first]), + disp_arr_tmp, dispinbytes, oldtype, dlp_p, Modified: mpich2/trunk/test/mpi/datatype/indexed-misc.c =================================================================== --- mpich2/trunk/test/mpi/datatype/indexed-misc.c 2012-11-12 17:07:38 UTC (rev 10609) +++ mpich2/trunk/test/mpi/datatype/indexed-misc.c 2012-11-14 17:20:14 UTC (rev 10610) @@ -10,14 +10,38 @@ #ifdef HAVE_STRING_H #include <string.h> #endif +#include <assert.h> +#include <limits.h> -static int verbose = 0; +static int verbose = 1; +#define check(cond_) \ + do { \ + if (!(cond_)) { \ + if (verbose) { \ + fprintf(stderr, "condition '%s' does not hold, at line %d\n", #cond_, __LINE__); \ + } \ + errs += 1; \ + } \ + } while (0) + +#define check_err(err_, what_failed_) \ + do { \ + if (err_) { \ + if (verbose) { \ + fprintf(stderr, "error: %s, at line %d\n", (what_failed_), __LINE__); \ + } \ + errs += (err_); \ + } \ + } while (0) + /* tests */ int indexed_contig_test(void); int indexed_zeroblock_first_test(void); int indexed_zeroblock_middle_test(void); int indexed_zeroblock_last_test(void); +int indexed_contig_leading_zero_test(void); +int indexed_same_lengths(void); /* helper functions */ int parse_args(int argc, char **argv); @@ -62,6 +86,18 @@ err); errs += err; + err = indexed_contig_leading_zero_test(); + if (err && verbose) fprintf(stderr, + "%d errors in indexed_contig_leading_zero_test.\n", + err); + errs += err; + + err = indexed_same_lengths(); + if (err && verbose) fprintf(stderr, + "%d errors in indexed_contig_leading_zero_test.\n", + err); + errs += err; + /* print message and exit */ if (errs) { fprintf(stderr, "Found %d errors\n", errs); @@ -299,6 +335,280 @@ return errs; } +/* very similar to indexed_zeroblock_first_test, but only has a single contig in + * order to catch a particular optimization path in MPICH's + * Dataloop_create_indexed routine */ +int indexed_contig_leading_zero_test(void) +{ + int err, errs = 0; + + int i; + MPI_Datatype type = MPI_DATATYPE_NULL; + MPI_Datatype struct_type = MPI_DATATYPE_NULL; + MPI_Datatype types[2]; + int len[3] = { 0, 4, 0 }; + int disp[3] = { INT_MAX, 2, INT_MAX}; + MPI_Aint adisp[3]; + MPI_Aint lb, ub; + int *buf = NULL; + + err = MPI_Type_indexed(3, len, disp, MPI_INT, &type); + check_err(err, "creating indexed type in indexed_contig_leading_zero_test()"); + err = MPI_Type_commit(&type); + check_err(err, "committing indexed type in indexed_contig_leading_zero_test()"); + + MPI_Type_lb(type, &lb); + check(lb == 2 * sizeof(int)); + MPI_Type_ub(type, &ub); + check(ub == 6 * sizeof(int)); + + /* make sure packing/unpacking works (hits a simple "is_contig" case in + * MPICH's pack/unpack routines) */ + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_contig_leading_zero_test()"); + for (i = 0; i < 10; ++i) { + int expected; + if (i >= 2 && i < 6) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + /* -------------------------------------------------------------------- */ + /* A more rigorous test of the indexed type. Use a hard-to-optimize struct + * type to force a more complicated datatype processing path + * (MPID_Segment_manipulate in MPICH) */ + len[0] = 1; + len[1] = 1; + adisp[0] = 0; + adisp[1] = 8*sizeof(int); + types[0] = type; + types[1] = MPI_INT; + + /* struct layout: xx0123xx4x ('x' indicates a hole), one char is an + * MPI_INT */ + MPI_Type_create_struct(2, len, adisp, types, &struct_type); + check_err(err, "creating struct type in indexed_contig_leading_zero_test()"); + err = MPI_Type_commit(&struct_type); + check_err(err, "committing struct type in indexed_contig_leading_zero_test()"); + + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + err = pack_and_unpack((char *) buf, 1, struct_type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_contig_test()"); + + for (i = 0; i < 10; ++i) { + int expected; + if ((i >= 2 && i < 6) || i == 8) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + MPI_Type_free(&struct_type); + MPI_Type_free( &type ); + + /* -------------------------------------------------------------------- */ + /* now do the same as above, but with hindexed */ + len[0] = 0; + len[1] = 4; + len[2] = 0; + /* use *_MAX vars to improve our chances of hitting any pointer-casting + * bugs in a big way (segfaults, etc.) */ + if (sizeof(MPI_Aint) == sizeof(long long)) { + adisp[0] = (MPI_Aint)LLONG_MAX; + adisp[1] = 2*sizeof(int); + adisp[2] = (MPI_Aint)LLONG_MAX; + } + else { + adisp[0] = (MPI_Aint)INT_MAX; + adisp[1] = 2*sizeof(int); + adisp[2] = (MPI_Aint)INT_MAX; + } + + err = MPI_Type_hindexed(3, len, adisp, MPI_INT, &type); + check_err(err, "creating hindexed type in indexed_contig_leading_zero_test()"); + + err = MPI_Type_commit(&type); + check_err(err, "committing hindexed type in indexed_contig_leading_zero_test()"); + + MPI_Type_lb(type, &lb); + check(lb == 2 * sizeof(int)); + MPI_Type_ub(type, &ub); + check(ub == 6 * sizeof(int)); + + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_contig_test()"); + + for (i = 0; i < 10; ++i) { + int expected; + if (i >= 2 && i < 6) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + /* -------------------------------------------------------------------- */ + /* A more rigorous test of the hindexed type. Use a hard-to-optimize struct + * type to force a more complicated datatype processing path + * (MPID_Segment_manipulate in MPICH) */ + len[0] = 1; + len[1] = 1; + adisp[0] = 0; + adisp[1] = 8*sizeof(int); + + /* struct layout: xx0123xx4x ('x' indicates a hole), one char is an + * MPI_INT */ + err = MPI_Type_create_struct(2, len, adisp, types, &struct_type); + check_err(err, "committing struct type in indexed_contig_leading_zero_test()"); + err = MPI_Type_commit(&struct_type); + check_err(err, "committing struct type in indexed_contig_leading_zero_test()"); + + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + /* fails in old MPICH (3.0rc1 and earlier), despite correct ub/lb + * determination */ + err = pack_and_unpack((char *) buf, 1, struct_type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_contig_test()"); + + for (i = 0; i < 10; ++i) { + int expected; + if ((i >= 2 && i < 6) || i == 8) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + MPI_Type_free(&struct_type); + MPI_Type_free(&type); + + return errs; +} + +/* Test an indexed (and hindexed) type where the block length is the same for + * all blocks, but with differing displacements so that it cannot directly be + * converted to a vector type. It is also important to add a dummy element at + * the beginning in order to cause int/MPI_Aint misalignment for the + * displacement of the first non-zero-width component. */ +int indexed_same_lengths(void) +{ + int err, errs = 0; + + int i; + MPI_Datatype type = MPI_DATATYPE_NULL; + int len[4]; + int disp[4]; + MPI_Aint adisp[4]; + MPI_Aint lb, ub; + int *buf = NULL; + + len[0] = 0; + len[1] = 1; + len[2] = 1; + len[3] = 1; + + disp[0] = 0; + disp[1] = 1; + disp[2] = 3; + disp[3] = 8; + + err = MPI_Type_indexed(4, len, disp, MPI_INT, &type); + check_err(err, "creating indexed type in indexed_same_lengths()"); + err = MPI_Type_commit(&type); + check_err(err, "committing indexed type in indexed_same_lengths()"); + + MPI_Type_lb(type, &lb); + check(lb == 1 * sizeof(int)); + MPI_Type_ub(type, &ub); + check(ub == 9 * sizeof(int)); + + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_same_lengths()"); + for (i = 0; i < 10; ++i) { + int expected; + if (i == 1 || i == 3 || i == 8) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + MPI_Type_free(&type); + + /* -------------------------------------------------------------------- */ + /* now do the same as above, but with hindexed */ + len[0] = 0; + len[1] = 1; + len[2] = 1; + len[3] = 1; + + adisp[0] = 0 * sizeof(int); + adisp[1] = 1 * sizeof(int); + adisp[2] = 3 * sizeof(int); + adisp[3] = 8 * sizeof(int); + + err = MPI_Type_hindexed(4, len, adisp, MPI_INT, &type); + check_err(err, "creating hindexed type in indexed_same_lengths()"); + err = MPI_Type_commit(&type); + check_err(err, "committing hindexed type in indexed_same_lengths()"); + + MPI_Type_lb(type, &lb); + check(lb == 1 * sizeof(int)); + MPI_Type_ub(type, &ub); + check(ub == 9 * sizeof(int)); + + buf = malloc(10*sizeof(int)); + assert(buf != NULL); + for (i = 0; i < 10; ++i) { + buf[i] = i + 1; + } + err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int)); + check_err(err, "packing/unpacking in indexed_same_lengths()"); + for (i = 0; i < 10; ++i) { + int expected; + if (i == 1 || i == 3 || i == 8) + expected = i + 1; + else + expected = 0; + check(buf[i] == expected); + } + free(buf); + + MPI_Type_free(&type); + + return errs; +} + /* pack_and_unpack() * * Perform packing and unpacking of a buffer for the purposes of checking