Re: [PATCH v2] disas/cris: Pass buffer size to format_dec() to avoid overflow warning

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 20/11/23 22:45, Paolo Bonzini wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Propagate the buffer size to format_dec() and use snprintf().
> 
> This should silence this UBSan -Wformat-overflow warning:

"produced when using GCC 12.1.0:"

> 
>    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.odaki@daynix.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-ID: <20231120132222.82138-1-philmd@linaro.org>
> [Rewritten to fix logic and avoid repeated expression. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   disas/cris.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/disas/cris.c b/disas/cris.c
> index 0b0a3fb9165..409a224c5d1 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);
>   }
> @@ -1876,6 +1876,12 @@ print_flags (struct cris_disasm_data *disdata, unsigned int insn, char *cp)
>     return cp;
>   }
>   
> +#define FORMAT_DEC(number, tp, signedp)                      \
> +    format_dec (number, tp, ({                                \
> +            assert(tp >= temp && tp <= temp + sizeof(temp)); \
> +            temp + sizeof(temp) - tp;                        \
> +        }), signedp)

Thank you Paolo for this respin!

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>