[mpich2-dev] [PATCH 1/1] Fix uses of MPI_Aint in mpich2 tests

[William Gropp]

Hmm, this raises a concern about how making MPI_Aint larger than void  
* might impact the user, since users will expect these to be the same.

The use of MPI_Aint was needed to suppress error messages from  
compilers that don't like to see casts that truncate (as an (int)  
would here).

How feasible is it to use an internal type (e.g., MPIR_Size_t)  
instead of MPI_Aint for the datatype/file code, and leave MPI_Aint as  
the sizeof void *?  Some of the datatype routines won't be able to  
return lengths, but that's already true for MPI_Type_size.  Will that  
be adequate for the file operations?

Bill

On Mar 19, 2008, at 3:37 PM, Douglas Miller wrote:

>
> Fix several problems related to having MPI_Aint larger than (void *).
> 1. In copy_fn() the cast-and-dereference of attribute_val_out will
> overwrite the stack in MPIR_Attr_dup_list() as that function only
> provides a void * for attribute_val_out.
>
> 2. In delete_fn() do not promote values to MPI_Aint for comparing,
> especially since neither are MPI_Aint to begin with.
>
> 3. Also in delete_fn(), the error path will segfault because it tries
> to dereference attribute_val which is not a pointer.
>
> 4. Other uses of MPI_Aint are arbitrary and complicate the test.
> Since attribute values in this program start out as ints and are
> stored as void * within attributes, it seems confusing to be casting
> them to/from MPI_Aint in between. This also avoids 64-bit  
> manipulations
> on 32-bit architectures.
>
> -----
>
> diff --git a/lib/mpi/mpich2/test/mpi/attr/attric.c
> b/lib/mpi/mpich2/test/mpi/attr/attric.c
> index 6b87a4e..56738e9 100644
> --- a/lib/mpi/mpich2/test/mpi/attr/attric.c
> +++ b/lib/mpi/mpich2/test/mpi/attr/attric.c
> @@ -41,24 +41,24 @@ int main( int argc, char **argv )
>  int copy_fn( MPI_Comm oldcomm, int keyval, void *extra_state,
>              void *attribute_val_in, void *attribute_val_out, int  
> *flag)
>  {
>      /* Note that if (sizeof(int) < sizeof(void *), just setting  
> the int
>         part of attribute_val_out may leave some dirty bits
>      */
> -    *(MPI_Aint *)attribute_val_out = (MPI_Aint)attribute_val_in;
> +    *(void **)attribute_val_out = attribute_val_in;
>      *flag = 1;
>      return MPI_SUCCESS;
>  }
>
>  int delete_fn( MPI_Comm comm, int keyval, void *attribute_val,
>                void *extra_state)
>  {
>      int world_rank;
>      MPI_Comm_rank( MPI_COMM_WORLD, &world_rank );
> -    if ((MPI_Aint)attribute_val != (MPI_Aint)world_rank) {
> -       printf( "incorrect attribute value %d\n", *(int*) 
> attribute_val );
> +    if (attribute_val != (void *)world_rank) {
> +       printf( "incorrect attribute value %ld\n", (long) 
> attribute_val );
>         MPI_Abort(MPI_COMM_WORLD, 1005 );
>      }
>      return MPI_SUCCESS;
>  }
>
>  int test_communicators( void )
> @@ -68,13 +68,13 @@ int test_communicators( void )
>      int flag, world_rank, world_size, key_1, key_3;
>      int errs = 0;
>      /*      integer n, ,
>             .        key_2
>
>      */
> -    MPI_Aint value;
> +    int      value;
>      int      isLeft;
>
>      MPI_Comm_rank( MPI_COMM_WORLD, &world_rank );
>      MPI_Comm_size( MPI_COMM_WORLD, &world_size );
>  #ifdef DEBUG
>      if (world_rank == 0) {
> @@ -105,25 +105,25 @@ int test_communicators( void )
>         MPI_Keyval_create(MPI_NULL_COPY_FN, MPI_NULL_DELETE_FN,
>                           &key_3, &value );
>
>         /* This may generate a compilation warning; it is, however, an
>            easy way to cache a value instead of a pointer */
>         /* printf( "key1 = %x key3 = %x\n", key_1, key_3 ); */
> -       MPI_Attr_put(comm, key_1, (void *) (MPI_Aint) world_rank );
> +       MPI_Attr_put(comm, key_1, (void *)world_rank );
>         /*         MPI_Attr_put(lo_comm, key_2, world_size ) */
>         MPI_Attr_put(comm, key_3, (void *)0 );
>
>         MPI_Comm_dup(comm, &dup_comm );
>
>         /* Note that if sizeof(int) < sizeof(void *), we can't use
>            (void **)&value to get the value we passed into  
> Attr_put.  To
> avoid
>            problems (e.g., alignment errors), we recover the value  
> into
>            a (void *) and cast to int. Note that this may generate  
> warning
>            messages from the compiler.  */
>         MPI_Attr_get(dup_comm, key_1, (void **)&vvalue, &flag );
> -       value = (MPI_Aint)vvalue;
> +       value = (int)vvalue;
>
>         if (! flag) {
>             errs++;
>             printf( "dup_comm key_1 not found on %d\n", world_rank );
>             fflush( stdout );
>             MPI_Abort(MPI_COMM_WORLD, 3004 );
> @@ -148,13 +148,13 @@ int test_communicators( void )
>               printf( "dup_comm key_2 value incorrect: %d\n", value );
>              fflush( stdout );
>               MPI_Abort(MPI_COMM_WORLD, 3007 );
>            }
>         */
>         MPI_Attr_get(dup_comm, key_3, (void **)&vvalue, &flag );
> -       value = (MPI_Aint)vvalue;
> +       value = (int)vvalue;
>         if (flag) {
>             errs++;
>             printf( "dup_comm key_3 found!\n" );
>             fflush( stdout );
>             MPI_Abort(MPI_COMM_WORLD, 3008 );
>         }
>

William Gropp
Paul and Cynthia Saylor Professor of Computer Science
University of Illinois Urbana-Champaign


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.mcs.anl.gov/mailman/private/mpich2-dev/attachments/20080319/1742a180/attachment.htm>