* Fix modification of string literal by swprintf @ 2017-06-05 8:59 Thomas Preudhomme 2017-06-07 10:02 ` Corinna Vinschen 0 siblings, 1 reply; 9+ messages in thread From: Thomas Preudhomme @ 2017-06-05 8:59 UTC (permalink / raw) To: newlib [-- Attachment #1: Type: text/plain, Size: 622 bytes --] Don't over-read memory returned by _DTOA_R, and never write to it since the result might be a string literal. For example, when doing: swprintf(tt, 20, L"%.*f", 6, 0.0); we will get back "0". Instead, write the result returned by _DTOA_R to the output buffer. After this, write the 0 chars directly to the the output buffer (if there are any). This also has the (marginal) advantage that we read/write less memory overall. The patch, contributed by Silviu Baranga was tested against libcxx testsuite and showed no regression. Please find the patch in git format-patch format in attachment. Best regards, Thomas [-- Attachment #2: 0001-Don-t-overread-or-write-memory-returned-by-_DTOA_R.patch --] [-- Type: text/x-patch, Size: 2767 bytes --] From 7a31cfb01a0b089daf2bed93b742b6edbf4cba0c Mon Sep 17 00:00:00 2001 From: Silviu Baranga <silviu.baranga@arm.cm> Date: Mon, 5 Jun 2017 09:54:42 +0100 Subject: [PATCH] Don't overread or write memory returned by _DTOA_R Don't over-read memory returned by _DTOA_R, and never write to it since the result might be a string literal. For example, when doing: swprintf(tt, 20, L"%.*f", 6, 0.0); we will get back "0". Instead, write the result returned by _DTOA_R to the output buffer. After this, write the 0 chars directly to the the output buffer (if there are any). This also has the (marginal) advantage that we read/write less memory overall. --- newlib/libc/locale/setlocale.h | 2 +- newlib/libc/stdio/vfwprintf.c | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h index 85a38d5..1440d0e 100644 --- a/newlib/libc/locale/setlocale.h +++ b/newlib/libc/locale/setlocale.h @@ -227,7 +227,7 @@ __get_locale_r (struct _reent *r) _ELIDABLE_INLINE struct __locale_t * __get_current_locale (void) { - return _REENT->_locale ?: __get_global_locale (); + return _REENT->_locale ?: __get_global_locale (); // version for !_MB_CAPABLE? } /* Only access fixed "C" locale using this function. Fake for !_MB_CAPABLE diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c index f0179a0..1bec9b2 100644 --- a/newlib/libc/stdio/vfwprintf.c +++ b/newlib/libc/stdio/vfwprintf.c @@ -1627,13 +1627,20 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, { char *digits, *bp, *rve; -#ifndef _MB_CAPABLE int i; -#endif digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); +#ifdef _MB_CAPABLE + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, + len, NULL); +#else + for (i = 0; i < rve - digits && i < len; ++i) + buf[i] = (wchar_t) digits[i]; +#endif + if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing zeros */ + char *padding = rve; bp = digits + ndigits; if (ch == L'f' || ch == L'F') { if (*digits == L'0' && value) @@ -1642,18 +1649,11 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, } if (value == 0) /* kludge for __dtoa irregularity */ rve = bp; - while (rve < bp) - *rve++ = '0'; - } + for (i = padding - digits; i < rve - digits && i < len; ++i) + buf[i] = L'0'; + } *length = rve - digits; /* full length of the string */ -#ifdef _MB_CAPABLE - _mbsnrtowcs_r (data, buf, (const char **) &digits, *length, - len, NULL); -#else - for (i = 0; i < *length && i < len; ++i) - buf[i] = (wchar_t) digits[i]; -#endif return buf; } } -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-05 8:59 Fix modification of string literal by swprintf Thomas Preudhomme @ 2017-06-07 10:02 ` Corinna Vinschen 2017-06-07 10:41 ` Thomas Preudhomme 0 siblings, 1 reply; 9+ messages in thread From: Corinna Vinschen @ 2017-06-07 10:02 UTC (permalink / raw) To: newlib [-- Attachment #1: Type: text/plain, Size: 4037 bytes --] On Jun 5 09:59, Thomas Preudhomme wrote: > Don't over-read memory returned by _DTOA_R, and never write to it > since the result might be a string literal. > > For example, when doing: > swprintf(tt, 20, L"%.*f", 6, 0.0); > > we will get back "0". > > Instead, write the result returned by _DTOA_R to the output buffer. > After this, write the 0 chars directly to the the output buffer > (if there are any). This also has the (marginal) advantage that > we read/write less memory overall. > > The patch, contributed by Silviu Baranga was tested against libcxx testsuite > and showed no regression. Please find the patch in git format-patch format > in attachment. > > Best regards, > > Thomas > >From 7a31cfb01a0b089daf2bed93b742b6edbf4cba0c Mon Sep 17 00:00:00 2001 > From: Silviu Baranga <silviu.baranga@arm.cm> > Date: Mon, 5 Jun 2017 09:54:42 +0100 > Subject: [PATCH] Don't overread or write memory returned by _DTOA_R > > Don't over-read memory returned by _DTOA_R, and never write to it > since the result might be a string literal. > > For example, when doing: > swprintf(tt, 20, L"%.*f", 6, 0.0); > > we will get back "0". > > Instead, write the result returned by _DTOA_R to the output buffer. > After this, write the 0 chars directly to the the output buffer > (if there are any). This also has the (marginal) advantage that > we read/write less memory overall. > --- > newlib/libc/locale/setlocale.h | 2 +- > newlib/libc/stdio/vfwprintf.c | 24 ++++++++++++------------ > 2 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h > index 85a38d5..1440d0e 100644 > --- a/newlib/libc/locale/setlocale.h > +++ b/newlib/libc/locale/setlocale.h > @@ -227,7 +227,7 @@ __get_locale_r (struct _reent *r) > _ELIDABLE_INLINE struct __locale_t * > __get_current_locale (void) > { > - return _REENT->_locale ?: __get_global_locale (); > + return _REENT->_locale ?: __get_global_locale (); // version for !_MB_CAPABLE? > } This doesn't belong here. Also, the code is already fine for !_MB_CAPABLE as well. > /* Only access fixed "C" locale using this function. Fake for !_MB_CAPABLE > diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c > index f0179a0..1bec9b2 100644 > --- a/newlib/libc/stdio/vfwprintf.c > +++ b/newlib/libc/stdio/vfwprintf.c > @@ -1627,13 +1627,20 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, > > { > char *digits, *bp, *rve; > -#ifndef _MB_CAPABLE > int i; > -#endif > > digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); > > +#ifdef _MB_CAPABLE > + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, > + len, NULL); > +#else > + for (i = 0; i < rve - digits && i < len; ++i) > + buf[i] = (wchar_t) digits[i]; > +#endif > + > if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing zeros */ > + char *padding = rve; > bp = digits + ndigits; > if (ch == L'f' || ch == L'F') { > if (*digits == L'0' && value) > @@ -1642,18 +1649,11 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, > } > if (value == 0) /* kludge for __dtoa irregularity */ > rve = bp; > - while (rve < bp) > - *rve++ = '0'; > - } > > + for (i = padding - digits; i < rve - digits && i < len; ++i) > + buf[i] = L'0'; Appending zeros here without incrementing rve... > + } > *length = rve - digits; /* full length of the string */ ...leads to incorrect setting of *length here. Or am I missing something? > -#ifdef _MB_CAPABLE > - _mbsnrtowcs_r (data, buf, (const char **) &digits, *length, > - len, NULL); > -#else > - for (i = 0; i < *length && i < len; ++i) > - buf[i] = (wchar_t) digits[i]; > -#endif > return buf; > } > } > -- > 1.9.1 > Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-07 10:02 ` Corinna Vinschen @ 2017-06-07 10:41 ` Thomas Preudhomme 2017-06-08 12:15 ` Thomas Preudhomme 0 siblings, 1 reply; 9+ messages in thread From: Thomas Preudhomme @ 2017-06-07 10:41 UTC (permalink / raw) To: newlib Hi Corinna, On 07/06/17 11:01, Corinna Vinschen wrote: > On Jun 5 09:59, Thomas Preudhomme wrote: >> Don't over-read memory returned by _DTOA_R, and never write to it >> since the result might be a string literal. >> >> For example, when doing: >> swprintf(tt, 20, L"%.*f", 6, 0.0); >> >> we will get back "0". >> >> Instead, write the result returned by _DTOA_R to the output buffer. >> After this, write the 0 chars directly to the the output buffer >> (if there are any). This also has the (marginal) advantage that >> we read/write less memory overall. >> >> The patch, contributed by Silviu Baranga was tested against libcxx testsuite >> and showed no regression. Please find the patch in git format-patch format >> in attachment. >> >> Best regards, >> >> Thomas > >> >From 7a31cfb01a0b089daf2bed93b742b6edbf4cba0c Mon Sep 17 00:00:00 2001 >> From: Silviu Baranga <silviu.baranga@arm.cm> >> Date: Mon, 5 Jun 2017 09:54:42 +0100 >> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >> >> Don't over-read memory returned by _DTOA_R, and never write to it >> since the result might be a string literal. >> >> For example, when doing: >> swprintf(tt, 20, L"%.*f", 6, 0.0); >> >> we will get back "0". >> >> Instead, write the result returned by _DTOA_R to the output buffer. >> After this, write the 0 chars directly to the the output buffer >> (if there are any). This also has the (marginal) advantage that >> we read/write less memory overall. >> --- >> newlib/libc/locale/setlocale.h | 2 +- >> newlib/libc/stdio/vfwprintf.c | 24 ++++++++++++------------ >> 2 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h >> index 85a38d5..1440d0e 100644 >> --- a/newlib/libc/locale/setlocale.h >> +++ b/newlib/libc/locale/setlocale.h >> @@ -227,7 +227,7 @@ __get_locale_r (struct _reent *r) >> _ELIDABLE_INLINE struct __locale_t * >> __get_current_locale (void) >> { >> - return _REENT->_locale ?: __get_global_locale (); >> + return _REENT->_locale ?: __get_global_locale (); // version for !_MB_CAPABLE? >> } > > This doesn't belong here. Also, the code is already fine for !_MB_CAPABLE > as well. My bad, this was a note I made to myself when reading on MB_CAPABLE some time ago. This does not belong to the patch and is indeed wrong. > >> /* Only access fixed "C" locale using this function. Fake for !_MB_CAPABLE >> diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c >> index f0179a0..1bec9b2 100644 >> --- a/newlib/libc/stdio/vfwprintf.c >> +++ b/newlib/libc/stdio/vfwprintf.c >> @@ -1627,13 +1627,20 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, >> >> { >> char *digits, *bp, *rve; >> -#ifndef _MB_CAPABLE >> int i; >> -#endif >> >> digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); >> >> +#ifdef _MB_CAPABLE >> + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, >> + len, NULL); >> +#else >> + for (i = 0; i < rve - digits && i < len; ++i) >> + buf[i] = (wchar_t) digits[i]; >> +#endif >> + >> if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing zeros */ >> + char *padding = rve; >> bp = digits + ndigits; >> if (ch == L'f' || ch == L'F') { >> if (*digits == L'0' && value) >> @@ -1642,18 +1649,11 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, >> } >> if (value == 0) /* kludge for __dtoa irregularity */ >> rve = bp; >> - while (rve < bp) >> - *rve++ = '0'; >> - } >> >> + for (i = padding - digits; i < rve - digits && i < len; ++i) >> + buf[i] = L'0'; > > Appending zeros here without incrementing rve... > >> + } >> *length = rve - digits; /* full length of the string */ > > ...leads to incorrect setting of *length here. Or am I missing > something? I'll transmit to Silviu for him to answer. Thanks for the comments. Best regards, Thomas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-07 10:41 ` Thomas Preudhomme @ 2017-06-08 12:15 ` Thomas Preudhomme 2017-06-09 13:32 ` Corinna Vinschen 0 siblings, 1 reply; 9+ messages in thread From: Thomas Preudhomme @ 2017-06-08 12:15 UTC (permalink / raw) To: newlib [-- Attachment #1: Type: text/plain, Size: 4310 bytes --] Hi Corinna, On 07/06/17 11:41, Thomas Preudhomme wrote: > Hi Corinna, > > On 07/06/17 11:01, Corinna Vinschen wrote: >> On Jun 5 09:59, Thomas Preudhomme wrote: >>> Don't over-read memory returned by _DTOA_R, and never write to it >>> since the result might be a string literal. >>> >>> For example, when doing: >>> swprintf(tt, 20, L"%.*f", 6, 0.0); >>> >>> we will get back "0". >>> >>> Instead, write the result returned by _DTOA_R to the output buffer. >>> After this, write the 0 chars directly to the the output buffer >>> (if there are any). This also has the (marginal) advantage that >>> we read/write less memory overall. >>> >>> The patch, contributed by Silviu Baranga was tested against libcxx testsuite >>> and showed no regression. Please find the patch in git format-patch format >>> in attachment. >>> >>> Best regards, >>> >>> Thomas >> >>> >From 7a31cfb01a0b089daf2bed93b742b6edbf4cba0c Mon Sep 17 00:00:00 2001 >>> From: Silviu Baranga <silviu.baranga@arm.cm> >>> Date: Mon, 5 Jun 2017 09:54:42 +0100 >>> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >>> >>> Don't over-read memory returned by _DTOA_R, and never write to it >>> since the result might be a string literal. >>> >>> For example, when doing: >>> swprintf(tt, 20, L"%.*f", 6, 0.0); >>> >>> we will get back "0". >>> >>> Instead, write the result returned by _DTOA_R to the output buffer. >>> After this, write the 0 chars directly to the the output buffer >>> (if there are any). This also has the (marginal) advantage that >>> we read/write less memory overall. >>> --- >>> newlib/libc/locale/setlocale.h | 2 +- >>> newlib/libc/stdio/vfwprintf.c | 24 ++++++++++++------------ >>> 2 files changed, 13 insertions(+), 13 deletions(-) >>> >>> diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h >>> index 85a38d5..1440d0e 100644 >>> --- a/newlib/libc/locale/setlocale.h >>> +++ b/newlib/libc/locale/setlocale.h >>> @@ -227,7 +227,7 @@ __get_locale_r (struct _reent *r) >>> _ELIDABLE_INLINE struct __locale_t * >>> __get_current_locale (void) >>> { >>> - return _REENT->_locale ?: __get_global_locale (); >>> + return _REENT->_locale ?: __get_global_locale (); // version for >>> !_MB_CAPABLE? >>> } >> >> This doesn't belong here. Also, the code is already fine for !_MB_CAPABLE >> as well. > > My bad, this was a note I made to myself when reading on MB_CAPABLE some time > ago. This does not belong to the patch and is indeed wrong. > >> >>> /* Only access fixed "C" locale using this function. Fake for !_MB_CAPABLE >>> diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c >>> index f0179a0..1bec9b2 100644 >>> --- a/newlib/libc/stdio/vfwprintf.c >>> +++ b/newlib/libc/stdio/vfwprintf.c >>> @@ -1627,13 +1627,20 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, >>> int ndigits, int flags, >>> >>> { >>> char *digits, *bp, *rve; >>> -#ifndef _MB_CAPABLE >>> int i; >>> -#endif >>> >>> digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); >>> >>> +#ifdef _MB_CAPABLE >>> + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, >>> + len, NULL); >>> +#else >>> + for (i = 0; i < rve - digits && i < len; ++i) >>> + buf[i] = (wchar_t) digits[i]; >>> +#endif >>> + >>> if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing >>> zeros */ >>> + char *padding = rve; >>> bp = digits + ndigits; >>> if (ch == L'f' || ch == L'F') { >>> if (*digits == L'0' && value) >>> @@ -1642,18 +1649,11 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, >>> int ndigits, int flags, >>> } >>> if (value == 0) /* kludge for __dtoa irregularity */ >>> rve = bp; >>> - while (rve < bp) >>> - *rve++ = '0'; >>> - } >>> >>> + for (i = padding - digits; i < rve - digits && i < len; ++i) >>> + buf[i] = L'0'; >> >> Appending zeros here without incrementing rve... >> >>> + } >>> *length = rve - digits; /* full length of the string */ >> >> ...leads to incorrect setting of *length here. Or am I missing >> something? > > I'll transmit to Silviu for him to answer. Please find his updated patch attached. Best regards, Thomas [-- Attachment #2: 0001-Don-t-overread-or-write-memory-returned-by-_DTOA_R.patch --] [-- Type: text/x-patch, Size: 2173 bytes --] From ee96a273969b0bccd308b81bcdf37a6fdefa06f6 Mon Sep 17 00:00:00 2001 From: Silviu Baranga <silviu.baranga@arm.cm> Date: Mon, 5 Jun 2017 09:54:42 +0100 Subject: [PATCH] Don't overread or write memory returned by _DTOA_R Don't over-read memory returned by _DTOA_R, and never write to it since the result might be a string literal. For example, when doing: swprintf(tt, 20, L"%.*f", 6, 0.0); we will get back "0". Instead, write the result returned by _DTOA_R to the output buffer. After this, write the 0 chars directly to the the output buffer (if there are any). This also has the (marginal) advantage that we read/write less memory overall. --- newlib/libc/stdio/vfwprintf.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c index f0179a0..eb5a2dc 100644 --- a/newlib/libc/stdio/vfwprintf.c +++ b/newlib/libc/stdio/vfwprintf.c @@ -1627,12 +1627,18 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, { char *digits, *bp, *rve; -#ifndef _MB_CAPABLE int i; -#endif digits = _DTOA_R (data, value, mode, ndigits, decpt, &dsgn, &rve); +#ifdef _MB_CAPABLE + _mbsnrtowcs_r (data, buf, (const char **) &digits, rve - digits, + len, NULL); +#else + for (i = 0; i < rve - digits && i < len; ++i) + buf[i] = (wchar_t) digits[i]; +#endif + if ((ch != L'g' && ch != L'G') || flags & ALT) { /* Print trailing zeros */ bp = digits + ndigits; if (ch == L'f' || ch == L'F') { @@ -1642,18 +1648,13 @@ wcvt(struct _reent *data, _PRINTF_FLOAT_TYPE value, int ndigits, int flags, } if (value == 0) /* kludge for __dtoa irregularity */ rve = bp; - while (rve < bp) - *rve++ = '0'; - } + for (i = rve - digits; i < bp - digits && i < len; ++i) + buf[i] = L'0'; + + rve = rve > bp ? rve : bp; + } *length = rve - digits; /* full length of the string */ -#ifdef _MB_CAPABLE - _mbsnrtowcs_r (data, buf, (const char **) &digits, *length, - len, NULL); -#else - for (i = 0; i < *length && i < len; ++i) - buf[i] = (wchar_t) digits[i]; -#endif return buf; } } -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-08 12:15 ` Thomas Preudhomme @ 2017-06-09 13:32 ` Corinna Vinschen 2017-06-14 11:05 ` Renlin Li 0 siblings, 1 reply; 9+ messages in thread From: Corinna Vinschen @ 2017-06-09 13:32 UTC (permalink / raw) To: newlib [-- Attachment #1: Type: text/plain, Size: 911 bytes --] On Jun 8 13:15, Thomas Preudhomme wrote: > Please find his updated patch attached. > > Best regards, > > Thomas > >From ee96a273969b0bccd308b81bcdf37a6fdefa06f6 Mon Sep 17 00:00:00 2001 > From: Silviu Baranga <silviu.baranga@arm.cm> > Date: Mon, 5 Jun 2017 09:54:42 +0100 > Subject: [PATCH] Don't overread or write memory returned by _DTOA_R > > Don't over-read memory returned by _DTOA_R, and never write to it > since the result might be a string literal. > > For example, when doing: > swprintf(tt, 20, L"%.*f", 6, 0.0); > > we will get back "0". > > Instead, write the result returned by _DTOA_R to the output buffer. > After this, write the 0 chars directly to the the output buffer > (if there are any). This also has the (marginal) advantage that > we read/write less memory overall. Pushed. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: Fix modification of string literal by swprintf 2017-06-09 13:32 ` Corinna Vinschen @ 2017-06-14 11:05 ` Renlin Li 2017-06-15 8:01 ` Thomas Preudhomme 0 siblings, 1 reply; 9+ messages in thread From: Renlin Li @ 2017-06-14 11:05 UTC (permalink / raw) To: newlib, vinschen, Thomas Preud'homme Hi there, It seems this patch cause few regressions on arm/aarch64 baremetal targets. libstdc++-v3: FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/dr1261.cc execution test FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/stof.cc execution test FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/to_wstring.cc execution test FAIL: 27_io/basic_ostream/inserters_arithmetic/wchar_t/4402.cc execution test newlib: FAIL: newlib.stdio/swprintf.c execution Regards, Renlin On 09/06/17 14:32, Corinna Vinschen wrote: > On Jun 8 13:15, Thomas Preudhomme wrote: >> Please find his updated patch attached. >> >> Best regards, >> >> Thomas > >> >From ee96a273969b0bccd308b81bcdf37a6fdefa06f6 Mon Sep 17 00:00:00 2001 >> From: Silviu Baranga <silviu.baranga@arm.cm> >> Date: Mon, 5 Jun 2017 09:54:42 +0100 >> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >> >> Don't over-read memory returned by _DTOA_R, and never write to it >> since the result might be a string literal. >> >> For example, when doing: >> swprintf(tt, 20, L"%.*f", 6, 0.0); >> >> we will get back "0". >> >> Instead, write the result returned by _DTOA_R to the output buffer. >> After this, write the 0 chars directly to the the output buffer >> (if there are any). This also has the (marginal) advantage that >> we read/write less memory overall. > > Pushed. > > > Thanks, > Corinna > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-14 11:05 ` Renlin Li @ 2017-06-15 8:01 ` Thomas Preudhomme 2017-06-15 9:05 ` Renlin Li 2017-06-19 10:58 ` Corinna Vinschen 0 siblings, 2 replies; 9+ messages in thread From: Thomas Preudhomme @ 2017-06-15 8:01 UTC (permalink / raw) To: newlib Hi Renlin, Here's Silviu's answer: "This happens when --enable-newlib-mb is used (which wasn't in my test configuration). Unfortunately this complicates thing a lot and I have no easy fix for it. The best action at the moment would be a revert." Corinna, could you revert the patch for now? Sorry for the trouble. Best regards, Thomas On 14/06/17 12:05, Renlin Li wrote: > Hi there, > > It seems this patch cause few regressions on arm/aarch64 baremetal targets. > > libstdc++-v3: > FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/dr1261.cc execution test > FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/stof.cc execution test > FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/to_wstring.cc > execution test > FAIL: 27_io/basic_ostream/inserters_arithmetic/wchar_t/4402.cc execution test > > newlib: > FAIL: newlib.stdio/swprintf.c execution > > Regards, > Renlin > > On 09/06/17 14:32, Corinna Vinschen wrote: >> On Jun 8 13:15, Thomas Preudhomme wrote: >>> Please find his updated patch attached. >>> >>> Best regards, >>> >>> Thomas >> >>> >From ee96a273969b0bccd308b81bcdf37a6fdefa06f6 Mon Sep 17 00:00:00 2001 >>> From: Silviu Baranga <silviu.baranga@arm.cm> >>> Date: Mon, 5 Jun 2017 09:54:42 +0100 >>> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >>> >>> Don't over-read memory returned by _DTOA_R, and never write to it >>> since the result might be a string literal. >>> >>> For example, when doing: >>> swprintf(tt, 20, L"%.*f", 6, 0.0); >>> >>> we will get back "0". >>> >>> Instead, write the result returned by _DTOA_R to the output buffer. >>> After this, write the 0 chars directly to the the output buffer >>> (if there are any). This also has the (marginal) advantage that >>> we read/write less memory overall. >> >> Pushed. >> >> >> Thanks, >> Corinna >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-15 8:01 ` Thomas Preudhomme @ 2017-06-15 9:05 ` Renlin Li 2017-06-19 10:58 ` Corinna Vinschen 1 sibling, 0 replies; 9+ messages in thread From: Renlin Li @ 2017-06-15 9:05 UTC (permalink / raw) To: Thomas Preudhomme, newlib Yes, in my configuration, we enable the newlib-mb feature. And the executing path to handle the trailing '0' is not consistent with how the digits is handled. Thanks, Renlin On 15/06/17 09:00, Thomas Preudhomme wrote: > Hi Renlin, > > Here's Silviu's answer: > > "This happens when --enable-newlib-mb is used (which wasn't in my test configuration). > > Unfortunately this complicates thing a lot and I have no easy fix for it. > The best action at the moment would be a revert." > > Corinna, could you revert the patch for now? > > Sorry for the trouble. Best regards, > > Thomas > > On 14/06/17 12:05, Renlin Li wrote: >> Hi there, >> >> It seems this patch cause few regressions on arm/aarch64 baremetal targets. >> >> libstdc++-v3: >> FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/dr1261.cc execution test >> FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/stof.cc execution test >> FAIL: 21_strings/basic_string/numeric_conversions/wchar_t/to_wstring.cc >> execution test >> FAIL: 27_io/basic_ostream/inserters_arithmetic/wchar_t/4402.cc execution test >> >> newlib: >> FAIL: newlib.stdio/swprintf.c execution >> >> Regards, >> Renlin >> >> On 09/06/17 14:32, Corinna Vinschen wrote: >>> On Jun 8 13:15, Thomas Preudhomme wrote: >>>> Please find his updated patch attached. >>>> >>>> Best regards, >>>> >>>> Thomas >>> >>>> >From ee96a273969b0bccd308b81bcdf37a6fdefa06f6 Mon Sep 17 00:00:00 2001 >>>> From: Silviu Baranga <silviu.baranga@arm.cm> >>>> Date: Mon, 5 Jun 2017 09:54:42 +0100 >>>> Subject: [PATCH] Don't overread or write memory returned by _DTOA_R >>>> >>>> Don't over-read memory returned by _DTOA_R, and never write to it >>>> since the result might be a string literal. >>>> >>>> For example, when doing: >>>> swprintf(tt, 20, L"%.*f", 6, 0.0); >>>> >>>> we will get back "0". >>>> >>>> Instead, write the result returned by _DTOA_R to the output buffer. >>>> After this, write the 0 chars directly to the the output buffer >>>> (if there are any). This also has the (marginal) advantage that >>>> we read/write less memory overall. >>> >>> Pushed. >>> >>> >>> Thanks, >>> Corinna >>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Fix modification of string literal by swprintf 2017-06-15 8:01 ` Thomas Preudhomme 2017-06-15 9:05 ` Renlin Li @ 2017-06-19 10:58 ` Corinna Vinschen 1 sibling, 0 replies; 9+ messages in thread From: Corinna Vinschen @ 2017-06-19 10:58 UTC (permalink / raw) To: newlib [-- Attachment #1: Type: text/plain, Size: 445 bytes --] On Jun 15 09:00, Thomas Preudhomme wrote: > Hi Renlin, > > Here's Silviu's answer: > > "This happens when --enable-newlib-mb is used (which wasn't in my test > configuration). > > Unfortunately this complicates thing a lot and I have no easy fix for it. > The best action at the moment would be a revert." > > Corinna, could you revert the patch for now? Done. Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-19 10:58 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-05 8:59 Fix modification of string literal by swprintf Thomas Preudhomme 2017-06-07 10:02 ` Corinna Vinschen 2017-06-07 10:41 ` Thomas Preudhomme 2017-06-08 12:15 ` Thomas Preudhomme 2017-06-09 13:32 ` Corinna Vinschen 2017-06-14 11:05 ` Renlin Li 2017-06-15 8:01 ` Thomas Preudhomme 2017-06-15 9:05 ` Renlin Li 2017-06-19 10:58 ` Corinna Vinschen
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).