Re: [PATCH] gdb: Fix -Wuninitialied issue

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

On 9/12/23 11:09, Tom de Vries via Gdb-patches wrote:
> On 9/12/23 10:22, Enze Li wrote:
>> I see the following warning when building GDB on FreeBSD/amd64 with
>> Clang 14,
>>
>> ======================================================================
>>    CXX    mdebugread.o
>> mdebugread.c:1069:3: error: variable 'f' is uninitialized when used 
>> here [-Werror,-Wuninitialized]
>>                  f->set_loc_enumval (tsym.value);
>>                  ^
>> mdebugread.c:836:17: note: initialize the variable 'f' to silence this 
>> warning
>>          struct field *f;
>>                         ^
>>                          = nullptr
>> ======================================================================
>>
>> after digging a little, I realized that we can not simply do what
>> Clang 14 says.
>>
>> The root cause of this issue is that we lost the initialization of
>> the variable 'f' in this commit,
>>
>>    commit 2774f2dad5f05e68771c07df6ab0fb23baa2118e
>>    Date:   Thu Aug 31 09:37:44 2023 +0200
>>
>>        [gdb/symtab] Factor out type::{alloc_fields,copy_fields}
>>
> 
> Hi,
> 
> thanks for fixing this.  I've reviewed my commit once more, and only 
> found the instance that this patch fixes.
> 
> LGTM.
> 
> Approved-By: Tom de Vries <tdevries@suse.de>
> 

Forgot to mention: before pushing, please fix the typo in $subject:

Wuninitialied -> Wuninitialized.

Thanks,
- Tom

>> we have made these modifications,
>>
>> ----------------------------------------------------------------------
>> --- a/gdb/mdebugread.c
>> +++ b/gdb/mdebugread.c
>> @@ -1034,9 +1034,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char 
>> *ext_sh, int bigend,
>>
>>          t->set_code (type_code);
>>          t->set_length (sh->value);
>> -       t->set_num_fields (nfields);
>> -       f = ((struct field *) TYPE_ALLOC (t, nfields * sizeof (struct 
>> field)));
>> -       t->set_fields (f);
>> +       t->alloc_fields (nfields, false);
>> ----------------------------------------------------------------------
>>
>> The problem is that the variable 'f' is used in the second half of
>> parse_symbol, that's why Clang complained.
>>
>> To fix this issue we need to ensure that the varibale 'f' is
>> initialized.  Calling the fields method is an obvious way to fix this
>> issue.
>>
>> Tested on FreeBSD/amd64 by rebuilding.
>> ---
>>   gdb/mdebugread.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
>> index ea3e15be53b2..9cb30ce0acd0 100644
>> --- a/gdb/mdebugread.c
>> +++ b/gdb/mdebugread.c
>> @@ -1035,6 +1035,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char 
>> *ext_sh, int bigend,
>>       t->set_code (type_code);
>>       t->set_length (sh->value);
>>       t->alloc_fields (nfields);
>> +    f = t->fields();
>>       if (type_code == TYPE_CODE_ENUM)
>>         {
>>
>> base-commit: 318d3bda5cad124bd11eebb0349d0f183ba625b1
>