Disable GCC11 warning on call to PMIx_Get()

rgmiller commented 2 years ago

This PR adds #pragma statements to suppress a specific warning generated by gcc 11 when the PMIx_Get() function is called.

Description

The version of PMIx included with Spectrum MPI defines the PMIx_Get() function to take a parameter that ends up being a 'const char[512]'. However, the caller is expected to pass in a static const string that is less than 512 bytes long. GCC11 issues a warning about this size mismatch, and since we build with -Werror, that breaks the build.

This commit adds some #pragma lines to disable that particular check for the call to PMIx_Get() when building with GCC 11.

Motivation and Context

See issue #715

How Has This Been Tested?

Built on Summit with GCC 10.2.0 & 11.2.0

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Performance enhancement (non-breaking change which improves efficiency)
[ ] Code cleanup (non-breaking change which makes code smaller or more readable)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Testing (addition of new tests or update to current tests)
[ ] Documentation (a change to man pages or other documentation)

Note: the bug is actually in IBM's implementation of PMIx, not UnifyFS.

Checklist:

[x] My code follows the UnifyFS code style requirements.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.
[x] All commit messages are properly formatted.

MichaelBrim commented 2 years ago

@rgmiller Your current solution is too hacky. Why not do this instead?

char key[512];
strlcpy(key, PMIX_JOB_SIZE);
PMIx_Get(...,key,...);

rgmiller commented 2 years ago

@MichaelBrim

Yeah, it's hacky. In my defense, I do not want that code to be there forever. I want to rip it all out once IBM fixes their PMIx code. That said, we could do an explicit copy, though it would have to be something more like:

char key[PMIX_MAX_KEYLEN+1];
strcpy(key, PMIX_JOB_SIZE);
PMIx_Get(&proc, key, NULL, 0, &valp);

The PMIx spec only says that PMIX_MAX_KEYLEN should be at least 63 bytes, so you can't count on it being 512.

Another, cleaner option would be:

pmix_key_t key = PMIX_JOB_SIZE;
rc = PMIx_Get(&proc, key, NULL, 0, &valp);

That would work for Summit, but you'd run into problems on systems where PMIx didn't define a pmix_key_t. This includes the OpenPMIx reference implementation (which is what Spack builds).

I think the proper fix is to change all the defines in pmix_common.h from lines like:

#define PMIX_JOB_SIZE                       "pmix.job.size"         // (uint32_t) #procs in this job

to lines like:

const pmix_key_t PMIX_JOB_SIZE = "pmix.job.size";         // (uint32_t) #procs in this job

That's something that IBM developers have to do, though. (I need to figure out how to file a bug report with them, though.)

LLNL / UnifyFS