[mpich-discuss] Incorrect error checking?

[Thomas Jahns]

Hello,

On 6/19/19 12:25 AM, Nick Radcliffe via discuss wrote:
> I recently noticed that a parameter check in MPII_Win_get_attr looks a 
> bit off:
> 
> 
>              /* A common user error is to pass the address of a 4-byte
>               * int when the address of a pointer (or an address-sized int)
>               * should have been used.  We can test for this specific
>               * case.  Note that this code assumes sizeof(intptr_t) is
>               * a power of 2. */
>              if ((intptr_t) attribute_val & (sizeof(intptr_t) - 1)) {
>                  MPIR_ERR_SETANDSTMT(mpi_errno, MPI_ERR_ARG, goto 
> fn_fail, "**attrnotptr");
>              }
> 
> 
> The comment indicates that the check is testing "attribute_val" to see 
> if a pointer to int was passed in, rather than a double pointer or 
> pointer to an integer that can store an address. But the check seems to 
> only be testing for 8 byte alignment (and failing if not aligned). Am I 
> missing something here?

There is more to be discussed here:

* you are correct to observe that the test is only for alignment to
   sizeof(intptr_t) (which on LP64 machines would typically be 8)

but also:

* the code assumes sizeof(intptr_t) == sizeof(any *) which might not be
   true on systems implementing the additional correctness checks of llvm
   SoftBound + CETS

* intptr_t might not even exist (although I assume mpich would not build
   on such platforms for other reasons anyway).

* int might indeed be address-sized, i.e. the check adds nothing of
   value on 32-bit platforms.

 From my point-of-view this is exactly the kind of quick hack that 
riddles "modern" software.

Thomas