bfd: Fix -Wmaybe-uninitialized in elf-eh-frame.c
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Testing passed
|
Commit Message
Fix the following build error:
elf-eh-frame.c: In function ‘skip_cfa_op’:
elf-eh-frame.c:354:33: error: ‘op’ may be used uninitialized [-Werror=maybe-uninitialized]
354 | switch (op & 0xc0 ? op & 0xc0 : op)
| ~~~~~~~~~~~~~~~~~~~~~~^~~~
elf-eh-frame.c:348:12: note: ‘op’ was declared here
348 | bfd_byte op;
| ^~
Build error seen with GCC 13.2.1 20231011 (Red Hat 13.2.1-4)
* elf-eh-frame.c (skip_cfa_op): Define variable op with value 0.
---
bfd/elf-eh-frame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 15.03.2024 16:57, Aaron Merey wrote:
> Fix the following build error:
>
> elf-eh-frame.c: In function ‘skip_cfa_op’:
> elf-eh-frame.c:354:33: error: ‘op’ may be used uninitialized [-Werror=maybe-uninitialized]
> 354 | switch (op & 0xc0 ? op & 0xc0 : op)
> | ~~~~~~~~~~~~~~~~~~~~~~^~~~
> elf-eh-frame.c:348:12: note: ‘op’ was declared here
> 348 | bfd_byte op;
> | ^~
>
> Build error seen with GCC 13.2.1 20231011 (Red Hat 13.2.1-4)
>
> * elf-eh-frame.c (skip_cfa_op): Define variable op with value 0.
While I can see that a workaround is needed if indeed even a recent
compiler doesn't get things right here, this pretty clearly being a
bogus diagnostic in this case - have you also filed a bug report
against the compiler? Would be nice to cross-reference that here.
Furthermore, with no-one else having noticed this so far I wonder
whether there aren't further criteria here for the issue to surface,
like special compiler options passed in your particular build. Those
would imo also be relevant to mention.
> --- a/bfd/elf-eh-frame.c
> +++ b/bfd/elf-eh-frame.c
> @@ -345,7 +345,7 @@ next_cie_fde_offset (const struct eh_cie_fde *ent,
> static bool
> skip_cfa_op (bfd_byte **iter, bfd_byte *end, unsigned int encoded_ptr_width)
> {
> - bfd_byte op;
> + bfd_byte op = 0;
> bfd_vma length;
>
> if (!read_byte (iter, end, &op))
As to "pretty clearly bogus": If the compiler decides to not inline
read_byte(), it shouldn't make assumptions on op's state. Whereas if
the function is inlined, its return value is clearly tied the state
of the variable.
I further wonder whether, if there already is one such case, it
wouldn't be better to work around the issue in read_byte() itself ...
I can't really see why e.g. the use in skip_leb128() shouldn't get
the exact same warning for "byte".
Jan
@@ -345,7 +345,7 @@ next_cie_fde_offset (const struct eh_cie_fde *ent,
static bool
skip_cfa_op (bfd_byte **iter, bfd_byte *end, unsigned int encoded_ptr_width)
{
- bfd_byte op;
+ bfd_byte op = 0;
bfd_vma length;
if (!read_byte (iter, end, &op))