bfd: Fix GCC warning when CFLAGS="-Og" is used

Message ID 20240904092417.6534-1-list@vahedi.org
State New
Headers
Series bfd: Fix GCC warning when CFLAGS="-Og" is used |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Shahab Vahedi Sept. 4, 2024, 9:24 a.m. UTC
  From: Shahab Vahedi <shahab.vahedi@amd.com>

This patch initializes the "op" variable in skip_cfa_op() function
of bfd/elf-eh-frame.c to "0" at its declaration point to avoid the
"maybe-uninitialized" warning.

Building binutils on a system with GCC version 13.2.0 and a configure
command that sets the optimization level to "-Og" leads to a build
failure because of a warning being treated as an error:
---------------------------------------------------------------------
$ ./configure CFLAGS="-Og"
$ make
  ...
  CC       elf-eh-frame.lo
  /src/gdb/bfd/elf-eh-frame.c: In function 'skip_cfa_op':
  /src/gdb/bfd/elf-eh-frame.c:354:33: error: 'op' may be used
    uninitialized [-Werror=maybe-uninitialized]
  354 |   switch (op & 0xc0 ? op & 0xc0 : op)
      |           ~~~~~~~~~~~~~~~~~~~~~~^~~~
  /src/gdb/bfd/elf-eh-frame.c:348:12: note: 'op' was declared here
  348 |   bfd_byte op;
      |            ^~
  cc1: all warnings being treated as errors
  ...
---------------------------------------------------------------------

The relevant code snippet related to this warning looks like:
---------------------------------------------------------------------
  static inline bool
  read_byte (bfd_byte **iter, bfd_byte *end, unsigned char *result)
  {
    if (*iter >= end)
      return false;
    *result = *((*iter)++);
    return true;
  }

  static bool
  skip_cfa_op (bfd_byte **iter, bfd_byte *end,...)
  {
    bfd_byte op;

    if (!read_byte (iter, end, &op))
      return false;

    switch (op & 0xc0 ? op & 0xc0 : op)
    ...
  }
---------------------------------------------------------------------

This warning probably happens because "-Og" results in GCC not
inlining the "read_byte()" function. Therefore, GCC treats its
invocation inside "skip_cfa_op()" like a black box and that ends
in the aforementioned warning.

Acknowledgement:
  Lancelot Six -- for coming with the idea behind this fix.

bfd/ChangeLog:
	* elf-eh-frame.c (skip_cfa_op): Initialize the "op" variable.
---
 bfd/elf-eh-frame.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Jan Beulich Sept. 4, 2024, 9:33 a.m. UTC | #1
On 04.09.2024 11:24, Shahab Vahedi wrote:
> From: Shahab Vahedi <shahab.vahedi@amd.com>
> 
> This patch initializes the "op" variable in skip_cfa_op() function
> of bfd/elf-eh-frame.c to "0" at its declaration point to avoid the
> "maybe-uninitialized" warning.
> 
> Building binutils on a system with GCC version 13.2.0 and a configure
> command that sets the optimization level to "-Og" leads to a build
> failure because of a warning being treated as an error:
> ---------------------------------------------------------------------
> $ ./configure CFLAGS="-Og"
> $ make
>   ...
>   CC       elf-eh-frame.lo
>   /src/gdb/bfd/elf-eh-frame.c: In function 'skip_cfa_op':
>   /src/gdb/bfd/elf-eh-frame.c:354:33: error: 'op' may be used
>     uninitialized [-Werror=maybe-uninitialized]
>   354 |   switch (op & 0xc0 ? op & 0xc0 : op)
>       |           ~~~~~~~~~~~~~~~~~~~~~~^~~~
>   /src/gdb/bfd/elf-eh-frame.c:348:12: note: 'op' was declared here
>   348 |   bfd_byte op;
>       |            ^~
>   cc1: all warnings being treated as errors
>   ...
> ---------------------------------------------------------------------
> 
> The relevant code snippet related to this warning looks like:
> ---------------------------------------------------------------------
>   static inline bool
>   read_byte (bfd_byte **iter, bfd_byte *end, unsigned char *result)
>   {
>     if (*iter >= end)
>       return false;
>     *result = *((*iter)++);
>     return true;
>   }
> 
>   static bool
>   skip_cfa_op (bfd_byte **iter, bfd_byte *end,...)
>   {
>     bfd_byte op;
> 
>     if (!read_byte (iter, end, &op))
>       return false;
> 
>     switch (op & 0xc0 ? op & 0xc0 : op)
>     ...
>   }
> ---------------------------------------------------------------------
> 
> This warning probably happens because "-Og" results in GCC not
> inlining the "read_byte()" function. Therefore, GCC treats its
> invocation inside "skip_cfa_op()" like a black box and that ends
> in the aforementioned warning.
> 
> Acknowledgement:
>   Lancelot Six -- for coming with the idea behind this fix.
> 
> bfd/ChangeLog:
> 	* elf-eh-frame.c (skip_cfa_op): Initialize the "op" variable.

Okay, yet preferably with ...

> --- 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 = 0x00;

... simply "0" used here.

Jan
  
Shahab Vahedi Sept. 4, 2024, 10:19 a.m. UTC | #2
September 4, 2024 at 11:33 AM, "Jan Beulich" <jbeulich@suse.com> wrote:

> On 04.09.2024 11:24, Shahab Vahedi wrote:
> >  - bfd_byte op;
> >  + bfd_byte op = 0x00;
> 
> ... simply "0" used here.

Re-submitted [1].

Cheers,
Shahab

[1]
https://inbox.sourceware.org/binutils/20240904101514.7177-1-list@vahedi.org/
  

Patch

diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
index 902d7c16334..e0e5bf163e7 100644
--- 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 = 0x00;
   bfd_vma length;
 
   if (!read_byte (iter, end, &op))