On 2023/11/20 22:22, Philippe Mathieu-Daudé wrote:
Propagate the buffer size to format_dec() and use snprintf().

This should silence this UBSan -Wformat-overflow warning:

   In file included from /usr/include/stdio.h:906,
                    from include/qemu/osdep.h:114,
                    from ../disas/cris.c:21:
   In function 'sprintf',
       inlined from 'format_dec' at ../disas/cris.c:1737:3,
       inlined from 'print_with_operands' at ../disas/cris.c:2477:12,
       inlined from 'print_insn_cris_generic.constprop' at 
../disas/cris.c:2690:8:
   /usr/include/bits/stdio2.h:30:10: warning: null destination pointer 
[-Wformat-overflow=]
    30 |   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    31 |                                   __glibc_objsize (__s), __fmt,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    32 |                                   __va_arg_pack ());
       |                                   ~~~~~~~~~~~~~~~~~

Reported-by: Akihiko Odaki <akihiko.od...@daynix.com>
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
TODO: add compiler version

My compiler is GCC 12.1.0, and I confirmed this change suppresses the warnings.


Surgical alternative to this patch from Akihiko:
https://lore.kernel.org/all/20231120112329.4149-1-akihiko.od...@daynix.com/
---
  disas/cris.c | 47 ++++++++++++++++++++++++++++++++---------------
  1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 0b0a3fb916..74a487c733 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1731,10 +1731,10 @@ format_hex (unsigned long number,
     unsigned (== 0).  */
static char *
-format_dec (long number, char *outbuffer, int signedp)
+format_dec(long number, char *outbuffer, size_t outsize, int signedp)
  {
    last_immediate = number;
-  sprintf (outbuffer, signedp ? "%ld" : "%lu", number);
+  snprintf(outbuffer, outsize, signedp ? "%ld" : "%lu", number);
return outbuffer + strlen (outbuffer);
  }
@@ -1898,6 +1898,7 @@ print_with_operands (const struct cris_opcode *opcodep,
       intermediate parts of the insn.  */
    char temp[sizeof (".d [$r13=$r12-2147483648],$r10") * 2];
    char *tp = temp;
+  ptrdiff_t tp_avail;
    static const char mode_char[] = "bwd?";
    const char *s;
    const char *cs;
@@ -2102,12 +2103,13 @@ print_with_operands (const struct cris_opcode *opcodep,
                number = 42;
              }
- if ((*cs == 'z' && (insn & 0x20))
-               || (opcodep->match == BDAP_QUICK_OPCODE
-                   && (nbytes <= 2 || buffer[1 + nbytes] == 0)))
-             tp = format_dec (number, tp, signedp);
-           else
-             {
+        if ((*cs == 'z' && (insn & 0x20))
+            || (opcodep->match == BDAP_QUICK_OPCODE
+            && (nbytes <= 2 || buffer[1 + nbytes] == 0))) {
+            tp_avail = temp - tp;

I think this should be: temp + sizeof(temp) - tp

Reply via email to