Re: [PATCH] gdb: Fix -Wuninitialied issue

[Enze Li via Gdb-patches <gdb-patches@sourceware.org>]

On Tue, Sep 12 2023 at 11:11:05 AM +0200, Tom de Vries wrote:

> 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.
>

Okey, fixed.  I'm checking this in now.

Best Regards,
Enze

> 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
>>