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

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

On 6/18/24 15:00, Tom de Vries wrote:
> 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>
> 

FWIW, I also did a build-with-clang and test run, and only see the usual 
suspects + clang-build specific PR31237.

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
>