public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use
@ 2022-11-01 10:57 Andrew Burgess
  2022-11-01 11:18 ` [PUSHED] opcodes/arm: don't pass non-string literal to printf like function Andrew Burgess
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2022-11-01 10:57 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

After I pushed this series:

  https://sourceware.org/pipermail/binutils/2022-October/123250.html

I managed to break .... pretty much all the buildbot builds :/

I've gone ahead and pushed the patch below as an emergency fix which
should resolve all the build problems.

If anyone disagrees with the fix here, please just let me know and I
can put together something inline with any feedback.

Sorry for the breakage.

Andrew

---

The earlier commit:

  commit 6576bffe6cbbb53c5756b2fccd2593ba69b74cdf
  Date:   Thu Jul 7 13:43:45 2022 +0100

      opcodes/arm: add disassembler styling for arm

was causing a compiler warning about a possible uninitialized variable
usage within opcodes/arm-dis.c.

The problem is in print_mve_unpredictable, and relates to the reason
variable, which is set by a switch table.

Currently the switch table does cover every valid value, though there
is no default case.  The variable switched on is passed in as an
argument to the print_mve_unpredictable function.

Looking at how print_mve_unpredictable is used, there is only one use,
the second argument is the one that is used for the switch table,
looking at how this argument is set, I don't believe it is possible
for this argument to take an invalid value.

So, I think the compiler warning is a false positive.  As such, my
proposed solution is to initialize the reason variable to the string
"??", this will silence the warning, and the "??" string should never
end up being printed.
---
 opcodes/arm-dis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index 6d302ec50ba..101b3f84dfe 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -7234,7 +7234,9 @@ print_mve_unpredictable (struct disassemble_info *info,
 {
   void *stream = info->stream;
   fprintf_styled_ftype func = info->fprintf_styled_func;
-  const char *reason;
+  /* Initialize REASON to avoid compiler warning about uninitialized
+     usage, though such usage should be impossible.  */
+  const char *reason = "??";
 
   switch (unpredict_code)
     {
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PUSHED] opcodes/arm: don't pass non-string literal to printf like function
  2022-11-01 10:57 [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use Andrew Burgess
@ 2022-11-01 11:18 ` Andrew Burgess
  2022-11-04 11:01   ` [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use Andrew Burgess
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2022-11-01 11:18 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

To my great shame.  After fixing the previous issue, yet another
problem showed up on a couple of the buildbots (but not all, I'm
guessing due to compiler version differences).

Again, I've pushed the fix for this issue.  I don't think there will
be any disagreement for this fix, but do let me know if there is and
I'm happy to provide an adjustment.

Once again, sorry for the breakage,

Andrew

---

The earlier commit:

  commit 6576bffe6cbbb53c5756b2fccd2593ba69b74cdf
  Date:   Thu Jul 7 13:43:45 2022 +0100

      opcodes/arm: add disassembler styling for arm

introduced two places where a register name was passed as the format
string to the disassembler's fprintf_styled_func callback.  This will
cause a warning from some compilers, like this:

  ../../binutils-gdb/opcodes/arm-dis.c: In function ‘print_mve_vld_str_addr’:
  ../../binutils-gdb/opcodes/arm-dis.c:6005:3: error: format not a string literal and no format arguments [-Werror=format-security]
   6005 |   func (stream, dis_style_register, arm_regnames[gpr]);
        |   ^~~~

This commit fixes these by using "%s" as the format string.
---
 opcodes/arm-dis.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index 101b3f84dfe..31ed81f5a4e 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -6002,7 +6002,7 @@ print_mve_vld_str_addr (struct disassemble_info *info,
     add_sub = "-";
 
   func (stream, dis_style_text, "[");
-  func (stream, dis_style_register, arm_regnames[gpr]);
+  func (stream, dis_style_register, "%s", arm_regnames[gpr]);
   if (p == 1)
     {
       func (stream, dis_style_text, ", ");
@@ -8588,7 +8588,8 @@ print_insn_coprocessor_1 (const struct sopcode32 *opcodes,
 			      is_unpredictable = true;
 			    u_reg = value;
 			  }
-			func (stream, dis_style_register, arm_regnames[value]);
+			func (stream, dis_style_register, "%s",
+			      arm_regnames[value]);
 			break;
 		      case 'V':
 			if (given & (1 << 6))
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use
  2022-11-01 11:18 ` [PUSHED] opcodes/arm: don't pass non-string literal to printf like function Andrew Burgess
@ 2022-11-04 11:01   ` Andrew Burgess
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Burgess @ 2022-11-04 11:01 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

*sigh*

Off-list, I was pointed at yet another uninitialized variable problem
caused by my disassembler styling patch.  This one is the same as the
first fix I pushed, just in another function.

Hopefully /this/ will be the last fix I end up pushing.

Thanks for your continued patience,

Andrew

---

After this commit:

  commit 6576bffe6cbbb53c5756b2fccd2593ba69b74cdf
  Date:   Thu Jul 7 13:43:45 2022 +0100

      opcodes/arm: add disassembler styling for arm

Some people were seeing their builds failing with complaints about a
possible uninitialized variable usage.  I previously fixed an instance
of this issue in this commit:

  commit 2df82cd4b459fbc32120e0ad1ce19e26349506fe
  Date:   Tue Nov 1 10:36:59 2022 +0000

      opcodes/arm: silence compiler warning about uninitialized variable use

which did fix the build problems that the sourceware buildbot was
hitting, however, an additional instance of the same problem was
brought to my attention, and that is fixed in this commit.

Where commit 2df82cd4b4 fixed the uninitialized variable problem in
print_mve_unpredictable, this commit fixes the same problem in
print_mve_undefined.

As with the previous commit, I don't believe we could really ever get
an uninitialized variable usage, based on the current usage of the
function, so I have just initialized the reason variable to "??".
---
 opcodes/arm-dis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index 31ed81f5a4e..47a0a38adec 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -7148,7 +7148,9 @@ print_mve_undefined (struct disassemble_info *info,
 {
   void *stream = info->stream;
   fprintf_styled_ftype func = info->fprintf_styled_func;
-  const char *reason;
+  /* Initialize REASON to avoid compiler warning about uninitialized
+     usage, though such usage should be impossible.  */
+  const char *reason = "??";
 
   switch (undefined_code)
     {
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-11-04 11:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01 10:57 [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use Andrew Burgess
2022-11-01 11:18 ` [PUSHED] opcodes/arm: don't pass non-string literal to printf like function Andrew Burgess
2022-11-04 11:01   ` [PUSHED] opcodes/arm: silence compiler warning about uninitialized variable use Andrew Burgess

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