* [PATCH] gdb: Fix -Wuninitialied issue
@ 2023-09-12 8:22 Enze Li
2023-09-12 9:09 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Enze Li @ 2023-09-12 8:22 UTC (permalink / raw)
To: gdb-patches; +Cc: enze.li, tdevries
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}
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
--
2.41.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: Fix -Wuninitialied issue
2023-09-12 8:22 [PATCH] gdb: Fix -Wuninitialied issue Enze Li
@ 2023-09-12 9:09 ` Tom de Vries
2023-09-12 9:11 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2023-09-12 9:09 UTC (permalink / raw)
To: Enze Li, gdb-patches; +Cc: enze.li
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>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: Fix -Wuninitialied issue
2023-09-12 9:09 ` Tom de Vries
@ 2023-09-12 9:11 ` Tom de Vries
2023-09-12 13:42 ` Enze Li
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2023-09-12 9:11 UTC (permalink / raw)
To: Enze Li, gdb-patches; +Cc: enze.li
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: Fix -Wuninitialied issue
2023-09-12 9:11 ` Tom de Vries
@ 2023-09-12 13:42 ` Enze Li
0 siblings, 0 replies; 4+ messages in thread
From: Enze Li @ 2023-09-12 13:42 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches, enze.li
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
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-12 13:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 8:22 [PATCH] gdb: Fix -Wuninitialied issue Enze Li
2023-09-12 9:09 ` Tom de Vries
2023-09-12 9:11 ` Tom de Vries
2023-09-12 13:42 ` Enze Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).