[mpich-discuss] Incorrect error checking?

Thu Jun 20 14:05:22 CDT 2019

This is the exact code used in configure:

    struct foo { int a; void *b; };
    int main() {
int buf[10];
struct foo *p1;
p1=(struct foo*)&buf[0];
p1->b = (void *)0;
p1=(struct foo*)&buf[1];
p1->b = (void *)0;
return 0;
    }

If the macro `NEEDS_POINTER_ALIGNMENT_ADJUST` gets set, the above program is supposed to crash (I believe). Nick, could you confirm that on your build machine? If the test code didn’t crash (and exits 0), then I wonder how the macro gets activated.

Anyway, for the potential of user machine differ from build machine, it might be a good idea to remove this check.

—
Hui Zhou
T: 630-252-3430

On Jun 20, 2019, at 1:42 PM, Nick Radcliffe <nradclif at cray.com<mailto:nradclif at cray.com>> wrote:

Hui,

The library was built on one of our internal build servers, and run on a different machine of course. But I don't think it's a configure issue. I did a quick test and it's definitely possible for the address of a pointer to not be 8-byte aligned:

[nradclif at heron ~]$ cat test_align.c
#include <stdlib.h>
#include <stdio.h>

int main()
{
    int *c;

    struct
    {
        int a;
        int *b;
    } __attribute__((packed)) s;

    c = (int *)&s.b;

    fprintf(stdout, "c = %p\n", c);

    return 0;
}
[nradclif at heron ~]$ ./test_align.x
c = 0x7ffe3d4e2c44

I'll have to talk to the user to get more details about their code.

Nick Radcliffe  Software Engineer | Cray Inc.
2131 Lindau Ln #1000 | Bloomington, MN 55425
+1-651-605-8864  nradclif at cray.com<mailto:email at cray.com>  www.cray.com<http://www.cray.com/>

________________________________
From: Zhou, Hui <zhouh at anl.gov<mailto:zhouh at anl.gov>>
Sent: Thursday, June 20, 2019 1:13:43 PM
To: discuss at mpich.org<mailto:discuss at mpich.org>
Cc: Nick Radcliffe
Subject: Re: [mpich-discuss] Incorrect error checking?

That’s interesting. It means the `configure` was finding pointers has to 8-bytes aligned but the user code is having pointers that are 4-byte aligned. I suspect the user did not build the library on the same machine it runs? I am also curious that which architecture actually are not allowing the 4-byte aligned pointers. Nick, is it possible to trace where the questioned mpi library was configured?

—
Hui Zhou
T: 630-252-3430

On Jun 20, 2019, at 1:04 PM, Nick Radcliffe via discuss <discuss at mpich.org<mailto:discuss at mpich.org>> wrote:

Ken,

The reason I brought this up is that a user's (valid) code was failing due to this parameter check. It's possible to use an address that's not 8 byte aligned to store a pointer to an int, i.e.,

    int *x;
    int *y;

    x = malloc(16);
    y = &x[1];

I would generally expect y to be 4 byte aligned in the above case. So sometimes this parameter check will cause a correct program to fail, right?

Nick Radcliffe  Software Engineer | Cray Inc.
2131 Lindau Ln #1000 | Bloomington, MN 55425
+1-651-605-8864  nradclif at cray.com<mailto:email at cray.com>  www.cray.com<http://www.cray.com/>

________________________________
From: Raffenetti, Kenneth J. via discuss <discuss at mpich.org<mailto:discuss at mpich.org>>
Sent: Wednesday, June 19, 2019 2:44:40 PM
To: discuss at mpich.org<mailto:discuss at mpich.org>
Cc: Raffenetti, Kenneth J.
Subject: Re: [mpich-discuss] Incorrect error checking?

Upon further thought, it's just trying to catch a common error, and uses
alignment to detect it since the type information is lost thru the
interface. In testing on x86_64, it doesn't appear that it will catch
the intended error.

As Thomas pointed out in another reply, there are also some issues with
the implementation, though it is correct that we require intptr_t to
build MPICH. At the very least, we could patch to use sizeof(void *) vs.
sizeof(intptr_t). Or we can still remove the check since the use-case is
so narrow. The preprocessor guard also depends on a AC_TRY_RUN that I'd
prefer to delete, to be honest.

Ken

On 6/19/19 9:27 AM, Raffenetti, Kenneth J. via discuss wrote:
> Nick,
>
> I agree this check looks like nonsense. IMO, we can just delete it and
> the others like it. I will create a PR.
>
> Ken
>
> On 6/18/19 5:25 PM, 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?
>>
>>
>>
>> Nick Radcliffe  Software Engineer| Cray Inc.
>>
>> 2131 Lindau Ln #1000 | Bloomington, MN55425
>>
>> +1-651-605-8864  nradclif at cray.com<http://cray.com/> <mailto:email at cray.com>  www.cray.com
>> <http://www.cray.com<http://www.cray.com/>>
>>
>>
>> _______________________________________________
>> discuss mailing list     discuss at mpich.org<mailto:discuss at mpich.org>
>> To manage subscription options or unsubscribe:
>> https://lists.mpich.org/mailman/listinfo/discuss
>>
> _______________________________________________
> discuss mailing list     discuss at mpich.org<mailto:discuss at mpich.org>
> To manage subscription options or unsubscribe:
> https://lists.mpich.org/mailman/listinfo/discuss
>
_______________________________________________
discuss mailing list     discuss at mpich.org<mailto:discuss at mpich.org>
To manage subscription options or unsubscribe:
https://lists.mpich.org/mailman/listinfo/discuss
_______________________________________________
discuss mailing list     discuss at mpich.org<mailto:discuss at mpich.org>
To manage subscription options or unsubscribe:
https://lists.mpich.org/mailman/listinfo/discuss

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mpich.org/pipermail/discuss/attachments/20190620/b43fb3f4/attachment-0001.html>