[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170407142629.GR17319@brightrain.aerifal.cx>
Date: Fri, 7 Apr 2017 10:26:29 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Undefined behavior in sprintf(dest, "%lld", -1LL)
On Fri, Apr 07, 2017 at 02:22:46PM +0000, Pascal Cuoq wrote:
> I am running musl's implementation inside tis-interpreter, a C interpreter that detects a very wide palette of undefined behaviors.
>
> When musl's printf implementation is invoked through the following test driver:
>
> char dest[100];
> int x = sprintf(dest, "%lld\n", -1LL);
>
> tis-interpreter emits this warning:
>
> src/stdio/vfprintf.c:141: warning: although the type provided to the va_arg macro (unsigned long long) and the actual type of the next variadic argument (long long) are corresponding unsigned - signed variants of the same integer type, the actual value of the argument (signed) is negative thus it cannot be represented in the provided (unsigned) type
>
> The warning of course refers to C11 7.16.1.1:2 http://port70.net/~nsz/c/c11/n1570.html#7.16.1.1p2
>
> To get a clearer view, I instrumented the code thus using the debug function tis_printf (we are inside the implementation of printf, it is a bad idea to call printf here):
>
> --- ../musl-1.1.16/src/stdio/vfprintf.c 2017-01-01 04:27:17.000000000 +0100
> +++ src/stdio/vfprintf.c 2017-04-07 16:13:19.291592002 +0200
> @@ -547,7 +547,7 @@
> if (argpos>=0) goto inval;
> } else {
> if (argpos>=0) nl_type[argpos]=st, arg=nl_arg[argpos];
> - else if (f) pop_arg(&arg, st, ap);
> + else if (f) { tis_printf("st:%u LLONG:%u ULLONG:%u\n", st, LLONG, ULLONG); pop_arg(&arg, st, ap); }
> else return 0;
> }
>
> This shows the debug message below:
>
> st:12 LLONG:12 ULLONG:12
>
> On the basis of this result, I would tentatively offer that the undefined behavior that tis-interpreter warns about is real and is caused by line 55 in vfprintf.c:
>
> #define LLONG ULLONG
>
> Because of this approach, when LLONG is passed to the function pop_arg(), the switch case at line 141 is taken:
>
> break; case ULLONG: arg->i = va_arg(*ap, unsigned long long);
>
> This causes UB by consuming a (negative) long long argument from a va_list with va_arg(..., unsigned long long).
>
> Pascal
>
Does defining the ODD_TYPES macro fix the problem? My leaning is to
just remove that #ifdef logic and always use the correct type with
va_arg. All that was doing was saving a few bytes of code; the change
should not affect performance.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
