Re: [PATCH] gdb/python/python-internal.h: avoid uninitialized constexpr

[Tom de Vries <tdevries@suse.de>]

On 6/18/24 11:09, Lancelot SIX wrote:
> The following recent change introduced a regression when building using
> clang++:
> 
>      commit 764af878259768bb70c65bdf3f3285c2d6409bbd
>      Date:   Wed Jun 12 18:58:49 2024 +0200
> 
>          [gdb/python] Add typesafe wrapper around PyObject_CallMethod
> 
> The error message is:
> 
>      ../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
>      constexpr char gdbpy_method_format;
>                     ^
>                                         = '\0'
>        CXX    python/py-block.o
>      1 error generated.
>      make[2]: *** [Makefile:1959: python/py-arch.o] Error 1
>      make[2]: *** Waiting for unfinished jobs....
>      In file included from ../../gdb/python/py-auto-load.c:25:
>      ../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
>      constexpr char gdbpy_method_format;
>                     ^
>                                         = '\0'
>      1 error generated.
>      make[2]: *** [Makefile:1959: python/py-auto-load.o] Error 1
>      In file included from ../../gdb/python/py-block.c:23:
>      ../../gdb/python/python-internal.h:151:16: error: default initialization of an object of const type 'const char'
>      constexpr char gdbpy_method_format;
>                     ^
>                                         = '\0'
>      1 error generated.
> 
> This patch fixes this by changing gdbpy_method_format to be a templated
> struct, and only have its specializations define the static constexpr
> member "format".  This way, we avoid having an uninitialized constexpr
> expression, regardless of it being instantiated or not.
> 

Hi Lancelot,

thanks for looking into this.

This seems to have been my doing, since I dropped the " = '\0'" bit.

I can reproduce the problem.

I also tried adding back the dropped bit, and saw that it fixes the 
compilation error.

However, that also brings back the problem I was trying to solve: doing 
this:
...
-      gdbpy_ref<> result = gdbpy_call_method (m_window, "close");
+      gdbpy_ref<> result = gdbpy_call_method (m_window, "close", 1.0);
...
compiles, using '\0' as format specificier for double, and we only run 
into trouble a runtime.

The patch you propose both:
- fixes the build with clang, and
- gives a compile time error for the unsupported format specifier case.

So, LGTM.

Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> Change-Id: I5bec241144f13500ef78daea30f00d01e373692d
> ---
>   gdb/python/python-internal.h | 24 ++++++++++++++++++------
>   1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index f4c35babe46..fec0010a444 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -148,19 +148,31 @@ typedef long Py_hash_t;
>   /* A template variable holding the format character (as for
>      Py_BuildValue) for a given type.  */
>   template<typename T>
> -constexpr char gdbpy_method_format;
> +struct gdbpy_method_format {};
>   
>   template<>
> -constexpr char gdbpy_method_format<gdb_py_longest> = GDB_PY_LL_ARG[0];
> +struct gdbpy_method_format<gdb_py_longest>
> +{
> +  static constexpr char format = GDB_PY_LL_ARG[0];
> +};
>   
>   template<>
> -constexpr char gdbpy_method_format<gdb_py_ulongest> = GDB_PY_LLU_ARG[0];
> +struct gdbpy_method_format<gdb_py_ulongest>
> +{
> +  static constexpr char format = GDB_PY_LLU_ARG[0];
> +};
>   
>   template<>
> -constexpr char gdbpy_method_format<int> = 'i';
> +struct gdbpy_method_format<int>
> +{
> +  static constexpr char format = 'i';
> +};
>   
>   template<>
> -constexpr char gdbpy_method_format<unsigned> = 'I';
> +struct gdbpy_method_format<unsigned>
> +{
> +  static constexpr char format = 'I';
> +};
>   
>   /* A helper function to compute the PyObject_CallMethod /
>      Py_BuildValue format given the argument types.  */
> @@ -169,7 +181,7 @@ template<typename... Args>
>   constexpr std::array<char, sizeof... (Args) + 1>
>   gdbpy_make_fmt ()
>   {
> -  return { gdbpy_method_format<Args>..., '\0' };
> +  return { gdbpy_method_format<Args>::format..., '\0' };
>   }
>   
>   /* Typesafe wrapper around PyObject_CallMethod.
> 
> base-commit: 0915235d341841ac7f13bd3136991c19b4a6746b