Warnings from persona.hpp on progress_required() with GCC 13.1.0

[Paul Hargrove]

Using the current develop branch (at a31b63d right now), the recently added test/regression/spec-issue204.cpp yields a "may be used uninitialized" warning in persona.hpp when compiled with -O -Wall and GCC 13.1 is the backend C++ compiler. The following reduced test case is also sufficient:

#include <upcxx/upcxx.hpp>
bool check() {
  {
    upcxx::persona dummy;
    upcxx::persona_scope ps(dummy);
  }
  return upcxx::progress_required();
}
int main() { return 0; }

Using that reproducer:

$ upcxx -O -Wall bug.cpp
In file included from [redacted]/include/upcxx/backend.hpp:6,
                 from [redacted]/include/upcxx/allocate.hpp:8,
                 from [redacted]/include/upcxx/upcxx.hpp:5,
                 from bug.cpp:1:
In member function 'upcxx::persona* upcxx::detail::persona_scope_raw::get_persona(upcxx::detail::persona_tls&) const',
    inlined from 'bool upcxx::detail::persona_tls::progress_required(upcxx::persona_scope&)' at [redacted]/include/upcxx/persona.hpp:750:35,
    inlined from 'bool upcxx::detail::persona_tls::progress_required()' at [redacted]/include/upcxx/persona.hpp:739:35,
    inlined from 'bool upcxx::progress_required()' at [redacted]/include/upcxx/backend.hpp:48:53,
    inlined from 'bool foo()' at bug.cpp:7:34:
[redacted]/include/upcxx/persona.hpp:452:7: warning: 'ps.upcxx::persona_scope::<unnamed>.upcxx::detail::persona_scope_raw::persona_xor_default_' may be used uninitialized [-Wmaybe-uninitialized]
  452 |       persona_xor_default_ ^
      |       ^~~~~~~~~~~~~~~~~~~~
bug.cpp: In function 'bool foo()':
bug.cpp:5:26: note: 'ps' declared here
    5 |     upcxx::persona_scope ps(dummy);
      |                          ^~

Triage notes:

• Not reproducible with the 2023.3.0 release of UPC++
• Removing -Wall eliminates the warning
• Use of -g instead of -O eliminates the warning, presumably due to weaker analysis
• Removing the call to progress_required() eliminates the warning, as does retaining in a way that the compiler can remove as unused
• So far this has not been see with older GCC releases or with other compiler families

[Dan Bonachea]

The provided reproducer shows the warning on develop but not the last 2023.3.0 release, because the default argument value for progress_required() recently changed in PR 488.

However the following modified (more explicit) reproducer that does not rely upon the changed default shows essentially the same warning with the 2023.3.0 release, and probably older versions as well:

#include <upcxx/upcxx.hpp>
bool check() {
  {
    upcxx::persona my_persona;
    upcxx::persona_scope my_scope(my_persona);
  }
  return upcxx::progress_required(upcxx::default_persona_scope());
}
int main() { return 0; }

output:

$ upcxx -O -Wall issue613b.cpp
In file included from /usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/backend.hpp:6,
                 from /usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/allocate.hpp:8,
                 from /usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/upcxx.hpp:5,
                 from issue613b.cpp:1:
In member function 'upcxx::persona* upcxx::detail::persona_scope_raw::get_persona(upcxx::detail::persona_tls&) const',
    inlined from 'bool upcxx::detail::persona_tls::progress_required(upcxx::persona_scope&)' at /usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/persona.hpp:765:35,
    inlined from 'bool upcxx::progress_required(persona_scope&)' at /usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/backend.hpp:52:53,
    inlined from 'bool check()' at issue613b.cpp:7:34:
/usr/local/pkg/upcxx-dirac/gcc-13.1.0/stable-2023.3.0/include/upcxx/persona.hpp:464:7: warning: 'my_scope.upcxx::persona_scope::<unnamed>.upcxx::detail::persona_scope_raw::persona_xor_default_' may be used uninitialized [-Wmaybe-uninitialized]
  464 |       persona_xor_default_ ^
      |       ^~~~~~~~~~~~~~~~~~~~
issue613b.cpp: In function 'bool check()':
issue613b.cpp:5:26: note: 'my_scope' declared here
    5 |     upcxx::persona_scope my_scope(my_persona);
      |  

I believe this nuisance warning is due to a harmless analysis failure in GCC 13, marking this as a minor in the hopes we can find a surgical way to silence it.

[Dan Bonachea]

I've confirmed through code inspection that this warning appears to be spurious/harmless.

PR 494 factors some nearby related code in persona.hpp, which appears to have a side-effect of silencing this bogus warning.

[Dan Bonachea]

Factor persona_scope constructors

This factors the code used for persona_scope activation in the two non-default persona_scope constructors, removing unnecessary (and error-prone) code duplication. It also removes an unnecessary level of inline dispatching constructors.

There are no logic changes in this commit, just factorization; generated code should remain identical after inlining and optimization.

This simplification appears to have the side-effect that it resolves issue #613 (Warnings from persona.hpp on progress_required() with GCC 13.1.0).

→ <<cset 10636eac9eae>>