Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc: warnings on MPI_STATUS_IGNORE etc with gcc-11.2.1 #5687

Closed
balay opened this issue Nov 19, 2021 · 14 comments · Fixed by #5749
Closed

misc: warnings on MPI_STATUS_IGNORE etc with gcc-11.2.1 #5687

balay opened this issue Nov 19, 2021 · 14 comments · Fixed by #5749

Comments

@balay
Copy link

balay commented Nov 19, 2021

I'm not sure if this is a compiler issue - or code isse [occurs with mpich - but not openmpi]

This is a 32bit build with gcc (GCC) 11.2.1 20210728 (Red Hat 11.2.1-1) on Fedora 35

[glci@pj03 petsc]$ make V=1 PETSC_ARCH=arch-ci-linux-gcc-complex-opt-32bit arch-ci-linux-gcc-complex-opt-32bit/obj/sys/utils/mpits.o
/home/glci/petsc-hash-pkgs/494b06380/bin/mpicc -c -m32 -fPIC -Wall -Wwrite-strings -Wno-strict-aliasing -Wno-unknown-pragmas -g -O    -I/home/glci/petsc/include -I/home/glci/petsc/arch-ci-linux-gcc-complex-opt-32bit/include -I/home/glci/petsc-hash-pkgs/494b06380/include    -MMD -MP /home/glci/petsc/src/sys/utils/mpits.c -o arch-ci-linux-gcc-complex-opt-32bit/obj/sys/utils/mpits.o
/home/glci/petsc/src/sys/utils/mpits.c: In function ‘PetscCommBuildTwoSidedFReq_Ibarrier’:
/home/glci/petsc/src/sys/utils/mpits.c:425:14: warning: ‘MPI_Testall’ accessing 20 bytes in a region of size 0 [-Wstringop-overflow=]
  425 |       ierr = MPI_Testall(nsends,sendreqs,&sent,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:425:14: note: referencing argument 4 of type ‘MPI_Status *’
In file included from /home/glci/petsc/include/petscsys.h:162,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc-hash-pkgs/494b06380/include/mpi.h:947:5: note: in a call to function ‘MPI_Testall’
  947 | int MPI_Testall(int count, MPI_Request array_of_requests[], int *flag,
      |     ^~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c: In function ‘PetscCommBuildTwoSided_Ibarrier’:
/home/glci/petsc/src/sys/utils/mpits.c:125:14: warning: ‘MPI_Testall’ accessing 20 bytes in a region of size 0 [-Wstringop-overflow=]
  125 |       ierr = MPI_Testall(nsends,sendreqs,&sent,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:125:14: note: referencing argument 4 of type ‘MPI_Status *’
In file included from /home/glci/petsc/include/petscsys.h:162,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc-hash-pkgs/494b06380/include/mpi.h:947:5: note: in a call to function ‘MPI_Testall’
  947 | int MPI_Testall(int count, MPI_Request array_of_requests[], int *flag,
      |     ^~~~~~~~~~~
In file included from /home/glci/petsc/include/petscsys.h:1674,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc/src/sys/utils/mpits.c: In function ‘PetscCommBuildTwoSidedF’:
/home/glci/petsc/include/petsclog.h:551:81: warning: ‘MPI_Waitall’ accessing 20 bytes in a region of size 0 [-Wstringop-overflow=]
  551 |   ((petsc_wait_all_ct++,petsc_sum_of_waits_ct += (PetscLogDouble) (count),0) || MPI_Waitall((count),(array_of_requests),(array_of_statuses)))
      |                                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:494:10: note: in expansion of macro ‘MPI_Waitall’
  494 |   ierr = MPI_Waitall(nto*ntags,toreqs,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |          ^~~~~~~~~~~
/home/glci/petsc/include/petsclog.h:551:81: note: referencing argument 3 of type ‘MPI_Status *’
  551 |   ((petsc_wait_all_ct++,petsc_sum_of_waits_ct += (PetscLogDouble) (count),0) || MPI_Waitall((count),(array_of_requests),(array_of_statuses)))
      |                                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:494:10: note: in expansion of macro ‘MPI_Waitall’
  494 |   ierr = MPI_Waitall(nto*ntags,toreqs,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |          ^~~~~~~~~~~
In file included from /home/glci/petsc/include/petscsys.h:162,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc-hash-pkgs/494b06380/include/mpi.h:946:5: note: in a call to function ‘MPI_Waitall’
  946 | int MPI_Waitall(int count, MPI_Request array_of_requests[], MPI_Status array_of_statuses[]) MPICH_API_PUBLIC;
      |     ^~~~~~~~~~~
In file included from /home/glci/petsc/include/petscsys.h:1674,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc/include/petsclog.h:551:81: warning: ‘MPI_Waitall’ accessing 20 bytes in a region of size 0 [-Wstringop-overflow=]
  551 |   ((petsc_wait_all_ct++,petsc_sum_of_waits_ct += (PetscLogDouble) (count),0) || MPI_Waitall((count),(array_of_requests),(array_of_statuses)))
      |                                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:495:10: note: in expansion of macro ‘MPI_Waitall’
  495 |   ierr = MPI_Waitall(*nfrom*ntags,fromreqs,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |          ^~~~~~~~~~~
/home/glci/petsc/include/petsclog.h:551:81: note: referencing argument 3 of type ‘MPI_Status *’
  551 |   ((petsc_wait_all_ct++,petsc_sum_of_waits_ct += (PetscLogDouble) (count),0) || MPI_Waitall((count),(array_of_requests),(array_of_statuses)))
      |                                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/glci/petsc/src/sys/utils/mpits.c:495:10: note: in expansion of macro ‘MPI_Waitall’
  495 |   ierr = MPI_Waitall(*nfrom*ntags,fromreqs,MPI_STATUSES_IGNORE);CHKERRMPI(ierr);
      |          ^~~~~~~~~~~
In file included from /home/glci/petsc/include/petscsys.h:162,
                 from /home/glci/petsc/src/sys/utils/mpits.c:1:
/home/glci/petsc-hash-pkgs/494b06380/include/mpi.h:946:5: note: in a call to function ‘MPI_Waitall’
  946 | int MPI_Waitall(int count, MPI_Request array_of_requests[], MPI_Status array_of_statuses[]) MPICH_API_PUBLIC;
      |     ^~~~~~~~~~~

I see mpi.h has:

#define MPI_STATUSES_IGNORE (MPI_Status *)1

Changing this '0' gets rid of the warning (openmpi uses this) [same issue with 'MPI_STATUS_IGNORE']

#define MPI_STATUSES_IGNORE (MPI_Status *)0
[glci@pj03 petsc]$ make V=1 PETSC_ARCH=arch-ci-linux-gcc-complex-opt-32bit arch-ci-linux-gcc-complex-opt-32bit/obj/sys/utils/mpits.o
/home/glci/petsc-hash-pkgs/494b06380/bin/mpicc -c -m32 -fPIC -Wall -Wwrite-strings -Wno-strict-aliasing -Wno-unknown-pragmas -g -O    -I/home/glci/petsc/include -I/home/glci/petsc/arch-ci-linux-gcc-complex-opt-32bit/include -I/home/glci/petsc-hash-pkgs/494b06380/include    -MMD -MP /home/glci/petsc/src/sys/utils/mpits.c -o arch-ci-linux-gcc-complex-opt-32bit/obj/sys/utils/mpits.o
[glci@pj03 petsc]$ /home/glci/petsc-hash-pkgs/494b06380/bin/mpichversion 
MPICH Version:          3.4.2
MPICH Release date:     Wed May 26 15:51:40 CDT 2021
MPICH Device:           ch3:nemesis
MPICH configure:        --prefix=/home/glci/petsc-hash-pkgs/494b06380 MAKE=/usr/bin/gmake --libdir=/home/glci/petsc-hash-pkgs/494b06380/lib CC=gcc CFLAGS=-m32 -fPIC -g -O AR=/usr/bin/ar ARFLAGS=cr CXX=g++ CXXFLAGS=-m32 -fPIC -g -O -fPIC -std=gnu++17 FFLAGS=-m32 -fPIC -ffree-line-length-0 -g -O -fallow-argument-mismatch FC=gfortran F77=gfortran FCFLAGS=-m32 -fPIC -ffree-line-length-0 -g -O -fallow-argument-mismatch --enable-shared --with-pm=hydra --disable-java --with-device=ch3:nemesis --enable-g=meminit
MPICH CC:       gcc -m32 -fPIC -g -O   -O2
MPICH CXX:      g++ -m32 -fPIC -g -O -fPIC -std=gnu++17  -O2
MPICH F77:      gfortran -m32 -fPIC -ffree-line-length-0 -g -O -fallow-argument-mismatch  -O2
MPICH FC:       gfortran -m32 -fPIC -ffree-line-length-0 -g -O -fallow-argument-mismatch  -O2
MPICH Custom Information: 
[glci@pj03 petsc]$ 
@balay balay changed the title Compile warnings with MPI_STATUSES_IGNORE [32bit] Compile warnings with MPI_STATUSES_IGNORE, MPI_STATUS_IGNORE Nov 19, 2021
@balay balay changed the title [32bit] Compile warnings with MPI_STATUSES_IGNORE, MPI_STATUS_IGNORE Compile warnings with MPI_STATUSES_IGNORE, MPI_STATUS_IGNORE with gcc-11.2.1 on Fedora-35 Nov 20, 2021
@balay
Copy link
Author

balay commented Nov 20, 2021

Ah - I see this with regular 64bit builds aswell..

@hzhou
Copy link
Contributor

hzhou commented Nov 22, 2021

While many compiler warnings are helpful, I hate to be forced to add the complexity of workarounds just to appease the dumb part of the compiler. 😞

I see a few solutions:

  1. Silence the warning with -Wno-stringop-overflow. Not really a solution.
  2. Define the constants to 0. It's only a fix because compiler made this an exception. It cornered us to not able to add additional constants.
  3. Add dummy STATUS objects and points to it. We already do this with MPI_UNWEIGHTED and MPI_WEIGHTS_EMPTY. It is the most semantic correct solution, but the dummy objects add unnecessary (someone refers to it as accidental) complexity.

The solution 3 need #5682 and potentially other patches to work, but it is an un-compromising solution that is most likely future-proof.

@balay
Copy link
Author

balay commented Nov 22, 2021

Right now - I'm using -Wno-stringop-overflow [as I'd like to use '-Werror' to catch issues in CI]

@hzhou
Copy link
Contributor

hzhou commented Nov 22, 2021

@raffenet What do you think regarding #5687 (comment)?

@raffenet
Copy link
Contributor

@raffenet What do you think regarding #5687 (comment)?

Unfortunately we're hamstrung by ABI with these values for now. We should definitely add them to the ABI wishlist.

If we do decide to change ABI, I think using 0 or NULL (solution 2) is OK. I guess an advantage to using a dummy struct (solution 3) is that we could throw and error on NULL usage, since that is not allowed by the specification.

@raffenet
Copy link
Contributor

Added a bullet onto the ABI wiki: https://github.com/pmodels/mpich/wiki/ABI-Change-Wishlist

@hzhou
Copy link
Contributor

hzhou commented Nov 22, 2021

Added a bullet onto the ABI wiki: https://github.com/pmodels/mpich/wiki/ABI-Change-Wishlist

Good point that this is an ABI change, and at least for now, keeping it backward compatible is probably more significant than fixing a warning. That leaves option 1. I guess we can add -Wno-stringop-overflow to configure and add a note in README to advise application to do the same if they desire to use -Werror.

In fact, the more I look at this, the more I'm convinced this is a gcc regression.

@hzhou hzhou changed the title Compile warnings with MPI_STATUSES_IGNORE, MPI_STATUS_IGNORE with gcc-11.2.1 on Fedora-35 misc: warnings on MPI_STATUS_IGNORE etc with gcc-11.2.1 Nov 29, 2021
@hzhou
Copy link
Contributor

hzhou commented Nov 29, 2021

@raffenet I am tempted to label this issue as won't fix. It is just warning annoyances. I am also convinced this is really a compiler regression issue. Your opinion?

@raffenet
Copy link
Contributor

I agree. My only question is do we submit feedback to the GCC folks?

@hzhou
Copy link
Contributor

hzhou commented Nov 29, 2021

I agree. My only question is do we submit feedback to the GCC folks?

It will be nice to do that. @travisk1154 Are you interested in submitting the ticket to GCC?

@tkoehring
Copy link
Contributor

Sure thing!

@hzhou
Copy link
Contributor

hzhou commented Jan 6, 2022

Apparently gcc now is making a difference between

int MPI_Waitall(int count, MPI_Request array_of_requests[], MPI_Status array_of_statuses[]);

and

int MPI_Waitall(int count, MPI_Request array_of_requests[], MPI_Status *array_of_statuses);

@hzhou
Copy link
Contributor

hzhou commented Jan 7, 2022

@balay Could you verify the patch in #5749 resolves the issue?

@balay
Copy link
Author

balay commented Jan 7, 2022

@hzhou Thanks! I've tried manual edit to mpi.h [with this change] - and that works!

holke added a commit to DLR-AMR/t8code that referenced this issue Jun 3, 2022
holke added a commit to DLR-AMR/t8code that referenced this issue Jun 3, 2022
holke added a commit to DLR-AMR/t8code that referenced this issue Jun 3, 2022
derobins added a commit to derobins/hdf5 that referenced this issue Oct 15, 2023
MPICH defines MPI_STATUSES_IGNORE (a pointer) to 1, which raises warnings
w/ gcc. This is a known issue that the MPICH devs are not going to fix.

See here:
    pmodels/mpich#5687

This fix suppresses those issues w/ gcc
lrknox pushed a commit to HDFGroup/hdf5 that referenced this issue Oct 16, 2023
MPICH defines MPI_STATUSES_IGNORE (a pointer) to 1, which raises warnings
w/ gcc. This is a known issue that the MPICH devs are not going to fix.

See here:
    pmodels/mpich#5687

This fix suppresses those issues w/ gcc
brtnfld pushed a commit to brtnfld/hdf5 that referenced this issue Oct 16, 2023
MPICH defines MPI_STATUSES_IGNORE (a pointer) to 1, which raises warnings
w/ gcc. This is a known issue that the MPICH devs are not going to fix.

See here:
    pmodels/mpich#5687

This fix suppresses those issues w/ gcc
jhendersonHDF pushed a commit to jhendersonHDF/hdf5 that referenced this issue Oct 18, 2023
MPICH defines MPI_STATUSES_IGNORE (a pointer) to 1, which raises warnings
w/ gcc. This is a known issue that the MPICH devs are not going to fix.

See here:
    pmodels/mpich#5687

This fix suppresses those issues w/ gcc
derobins added a commit to HDFGroup/hdf5 that referenced this issue Oct 18, 2023
* Address nagfor exceptions stoppage. (#3658)

* added cmake ieee flag for nagfor

* generalized determining the nag compiler

* fixing some misc. NAG warnings

* Simplify. (#3659)


* Address @jhendersonHDF review

* Add expedited testing support to t_filters_parallel (#3665)

* Remove clang warnings (#3656)

* Fixes test failure for gfortran -O2 and -O3, -fdefault-real-16 (#3662)

* added cmake ieee flag for nagfor

* fixes gfortran -O2 and -O3, -fdefault-real-16

* fixed sync

* updated release notes

* Fix link error on clang17/gfortran13/macOS-13 (#3666) (#3671)

* Correct fortran CMake generator expressions (#3670)

* Add AOCC GitHub Action (#3504) (#3657)

* Fix uninitialized subfiling test variable (#3675)

Picked up by gcc 10 on skybridge. Probably spurious, but no harm in
initializing it to a "bad" value.

* Add support for AOCC & Flang w/ the Autotools (#3674)

* Adds a config/clang-fflags options file to support Flang
* Corrects missing "-Wl," from linker options in the libtool wrappers
  when using Flang, the MPI Fortran compiler wrappers, and building
  the shared library. This would often result in unrecognized options
  like -soname.
* Enable -nomp w/ Flang to avoid linking to the OpenMPI library.

CMake can build the parallel, shared library w/ Fortran using AOCC
and Flang, so no changes were needed for that build system.

Fixes GitHub issues #3439, #1588, #366, #280

* Fix a strncpy call to use dest size not src (#3677)

A strncpy call in a path construction call used the size of the src
buffer instead of the dest buffer as the limit n.

This was switched to use the dest size and properly terminate the
string if truncation occurs.

* Remove CANBE_UNUSED() from subfiling VFD (#3678)

This macro was an attempt to quiet warnings about release mode unused
variables that only appear in asserts. It resolves to a void cast, which
doesn't quiet warnings when an assignment has already taken place.

* Suppress MPI_Waitall warnings w/ MPICH (#3680)

MPICH defines MPI_STATUSES_IGNORE (a pointer) to 1, which raises warnings
w/ gcc. This is a known issue that the MPICH devs are not going to fix.

See here:
    pmodels/mpich#5687

This fix suppresses those issues w/ gcc

* Fix a possible NULL pointer dereference in tests (#3676)


The dtypes test could dereference a NULL pointer if a strdup call
failed.

* Fix printf warnings in t_mpi (#3679)

* Fix printf warnings in t_mpi

The type of MPI_Offset varies with implementation. In MPICH, it's long,
which raises warnings when we attempt to use long long format
specifiers. Casting to long long fixes the warnings.

* Fix invalid memory access in S3 comms (#3681)

In the ros3 VFD, passing an empty string parameter to an internal
API call could result in accessing the -1th element of a string.
This would cause failures on big-endian systems like s390x.

This parameter is now checked before writing to the string.

Fixes GitHub #1168

* Add Doxygen for H5Pset_fapl_sec2() (#3685)

*

* switch to using time function instead of date function (#3690)

* Initialize API context MPI types to MPI_BYTE (#3688)

* Add test info output to t_filters_parallel (#3696)

* Suppress format string warnings in subfiling test (#3699)

* Fix unused variable in tselect.c (#3701)

* Fix unused variable warning in H5F_sfile_assert_num (#3700)

* Restore floating-point suffixes in tests (#3698)

A prior commit removed too many F suffixes. This restores the suffixes
for float variables.

* Sync with changes from develop

---------

Co-authored-by: Scot Breitenfeld <brtnfld@hdfgroup.org>
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Co-authored-by: Allen Byrne <50328838+byrnHDF@users.noreply.github.com>
Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants