disas/cris.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
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:
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)
+
/* Print out an insn with its operands, and update the info->insn_type
fields. The prefix_opcodep and the rest hold a prefix insn that is
supposed to be output as an address mode. */
@@ -2105,7 +2111,7 @@ print_with_operands (const struct cris_opcode *opcodep,
if ((*cs == 'z' && (insn & 0x20))
|| (opcodep->match == BDAP_QUICK_OPCODE
&& (nbytes <= 2 || buffer[1 + nbytes] == 0)))
- tp = format_dec (number, tp, signedp);
+ tp = FORMAT_DEC (number, tp, signedp);
else
{
unsigned int highbyte = (number >> 24) & 0xff;
@@ -2241,7 +2247,7 @@ print_with_operands (const struct cris_opcode *opcodep,
with_reg_prefix);
if (number >= 0)
*tp++ = '+';
- tp = format_dec (number, tp, 1);
+ tp = FORMAT_DEC (number, tp, 1);
info->flags |= CRIS_DIS_FLAG_MEM_TARGET_IS_REG;
info->target = (prefix_insn >> 12) & 15;
@@ -2340,7 +2346,7 @@ print_with_operands (const struct cris_opcode *opcodep,
{
if (number >= 0)
*tp++ = '+';
- tp = format_dec (number, tp, 1);
+ tp = FORMAT_DEC (number, tp, 1);
}
}
else
@@ -2397,7 +2403,7 @@ print_with_operands (const struct cris_opcode *opcodep,
break;
case 'I':
- tp = format_dec (insn & 63, tp, 0);
+ tp = FORMAT_DEC (insn & 63, tp, 0);
break;
case 'b':
@@ -2426,11 +2432,11 @@ print_with_operands (const struct cris_opcode *opcodep,
break;
case 'c':
- tp = format_dec (insn & 31, tp, 0);
+ tp = FORMAT_DEC (insn & 31, tp, 0);
break;
case 'C':
- tp = format_dec (insn & 15, tp, 0);
+ tp = FORMAT_DEC (insn & 15, tp, 0);
break;
case 'o':
@@ -2463,7 +2469,7 @@ print_with_operands (const struct cris_opcode *opcodep,
if (number > 127)
number = number - 256;
- tp = format_dec (number, tp, 1);
+ tp = FORMAT_DEC (number, tp, 1);
*tp++ = ',';
tp = format_reg (disdata, (insn >> 12) & 15, tp, with_reg_prefix);
}
@@ -2474,7 +2480,7 @@ print_with_operands (const struct cris_opcode *opcodep,
break;
case 'i':
- tp = format_dec ((insn & 32) ? (insn & 31) | ~31L : insn & 31, tp, 1);
+ tp = FORMAT_DEC ((insn & 32) ? (insn & 31) | ~31L : insn & 31, tp, 1);
break;
case 'P':
--
2.41.0
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>
On 2023/11/21 6: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:
>
> 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, ({ \
Nit: the slash at the end is misaligned.
Otherwise,
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> + assert(tp >= temp && tp <= temp + sizeof(temp)); \
> + temp + sizeof(temp) - tp; \
> + }), signedp)
> +
> /* Print out an insn with its operands, and update the info->insn_type
> fields. The prefix_opcodep and the rest hold a prefix insn that is
> supposed to be output as an address mode. */
> @@ -2105,7 +2111,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> if ((*cs == 'z' && (insn & 0x20))
> || (opcodep->match == BDAP_QUICK_OPCODE
> && (nbytes <= 2 || buffer[1 + nbytes] == 0)))
> - tp = format_dec (number, tp, signedp);
> + tp = FORMAT_DEC (number, tp, signedp);
> else
> {
> unsigned int highbyte = (number >> 24) & 0xff;
> @@ -2241,7 +2247,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> with_reg_prefix);
> if (number >= 0)
> *tp++ = '+';
> - tp = format_dec (number, tp, 1);
> + tp = FORMAT_DEC (number, tp, 1);
>
> info->flags |= CRIS_DIS_FLAG_MEM_TARGET_IS_REG;
> info->target = (prefix_insn >> 12) & 15;
> @@ -2340,7 +2346,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> {
> if (number >= 0)
> *tp++ = '+';
> - tp = format_dec (number, tp, 1);
> + tp = FORMAT_DEC (number, tp, 1);
> }
> }
> else
> @@ -2397,7 +2403,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> break;
>
> case 'I':
> - tp = format_dec (insn & 63, tp, 0);
> + tp = FORMAT_DEC (insn & 63, tp, 0);
> break;
>
> case 'b':
> @@ -2426,11 +2432,11 @@ print_with_operands (const struct cris_opcode *opcodep,
> break;
>
> case 'c':
> - tp = format_dec (insn & 31, tp, 0);
> + tp = FORMAT_DEC (insn & 31, tp, 0);
> break;
>
> case 'C':
> - tp = format_dec (insn & 15, tp, 0);
> + tp = FORMAT_DEC (insn & 15, tp, 0);
> break;
>
> case 'o':
> @@ -2463,7 +2469,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> if (number > 127)
> number = number - 256;
>
> - tp = format_dec (number, tp, 1);
> + tp = FORMAT_DEC (number, tp, 1);
> *tp++ = ',';
> tp = format_reg (disdata, (insn >> 12) & 15, tp, with_reg_prefix);
> }
> @@ -2474,7 +2480,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> break;
>
> case 'i':
> - tp = format_dec ((insn & 32) ? (insn & 31) | ~31L : insn & 31, tp, 1);
> + tp = FORMAT_DEC ((insn & 32) ? (insn & 31) | ~31L : insn & 31, tp, 1);
> break;
>
> case 'P':
© 2016 - 2024 Red Hat, Inc.