[mpich-discuss] Incorrect error checking?
Raffenetti, Kenneth J.
raffenet at mcs.anl.gov
Thu Jun 20 14:09:19 CDT 2019
Hmm, interesting. I think this example false-positive is pretty good
reason to remove the check from MPICH.
Ken
On 6/20/19 1:42 PM, Nick Radcliffe via discuss 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, MN55425
>
> +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>
> *Sent:* Thursday, June 20, 2019 1:13:43 PM
> *To:* 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,MN55425
>> +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
>
>
> _______________________________________________
> discuss mailing list discuss at mpich.org
> To manage subscription options or unsubscribe:
> https://lists.mpich.org/mailman/listinfo/discuss
>
More information about the discuss
mailing list