Re: [PATCH] Fix up target_to_host fn in gimple-ssa-sprintf.c (PR c++/90010)
On April 9, 2019 11:47:28 PM GMT+02:00, Jakub Jelinek wrote:
>Hi!
>
>As the following testcase shows, target_to_host handled
>targstr with strlen shorter than hostsz - 4 right (though with wasteful
>clearing of the rest of the buffer when only one '\0' termination is
>enough)
>and similarly also strlen hostsz and more right (making last 4
>characters
>in the buffer "..."), but anything in between resulted in random
>byte(s)
>at the end of the string (strncpy with hostsz - 4 length didn't zero
>terminate and because strlen (targstr) >= hostsz was false, nothing was
>appended). When some target to host charset adjustment was needed, it
>appended that "..." even to strings that actually didn't need it.
>
>The following patch doesn't use "..." if targstr fits (including the
>zero
>termination), otherwise makes sure last 4 bytes in the buffer are
>"...".
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>2019-04-09 Jakub Jelinek
>
> PR c++/90010
> * gimple-ssa-sprintf.c (target_to_host): Fix handling of targstr
> with strlen in between hostsz-3 and hostsz-1 inclusive when no
> translation is needed, and when translation is needed, only append
> ... if the string length is hostsz or more bytes long. Avoid using
> strncpy or strcat.
>
> * gcc.dg/pr90010.c: New test.
>
>--- gcc/gimple-ssa-sprintf.c.jj2019-04-09 12:25:41.0 +0200
>+++ gcc/gimple-ssa-sprintf.c 2019-04-09 14:01:04.875445413 +0200
>@@ -383,9 +383,14 @@ target_to_host (char *hostr, size_t host
> overlong strings just like the translated strings are. */
> if (target_to_host_charmap['\0'] == 1)
> {
>- strncpy (hostr, targstr, hostsz - 4);
>- if (strlen (targstr) >= hostsz)
>- strcpy (hostr + hostsz - 4, "...");
>+ size_t len = strlen (targstr);
>+ if (len >= hostsz)
>+ {
>+memcpy (hostr, targstr, hostsz - 4);
>+strcpy (hostr + hostsz - 4, "...");
>+ }
>+ else
>+ memcpy (hostr, targstr, len + 1);
> return hostr;
> }
>
>@@ -399,10 +404,9 @@ target_to_host (char *hostr, size_t host
> if (!*targstr)
> break;
>
>- if (size_t (ph - hostr) == hostsz - 4)
>+ if (size_t (ph - hostr) == hostsz)
> {
>-*ph = '\0';
>-strcat (ph, "...");
>+strcpy (ph - 4, "...");
> break;
> }
> }
>--- gcc/testsuite/gcc.dg/pr90010.c.jj 2019-04-09 14:22:14.795791124
>+0200
>+++ gcc/testsuite/gcc.dg/pr90010.c 2019-04-09 14:26:10.274961523 +0200
>@@ -0,0 +1,27 @@
>+/* PR c++/90010 */
>+/* { dg-do compile } */
>+/* { dg-options "-Wall" } */
>+
>+char b[4096] = "abc";
>+void bar (char *);
>+
>+void
>+foo ()
>+{
>+ char d[4096];
>+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzthudfred",
>b);/* { dg-warning "'foobarbazquxquuxquuzthudfred' directive output
>may be truncated writing 28 bytes into a region of size between 1 and
>4096" } */
>+ /* { dg-message "'__builtin_snprintf' output between 29 and 4124
>bytes into a destination of size 4096" "" { target *-*-* } .-1 } */
>+ bar (d);
>+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzcorgefred",
>b);/* { dg-warning "'foobarbazquxquuxquuzcorgefred' directive output
>may be truncated writing 29 bytes into a region of size between 1 and
>4096" } */
>+ /* { dg-message "'__builtin_snprintf' output between 30 and 4125
>bytes into a destination of size 4096" "" { target *-*-* } .-1 } */
>+ bar (d);
>+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzcorgewaldo",
>b);/* { dg-warning "'foobarbazquxquuxquuzcorgewaldo' directive output
>may be truncated writing 30 bytes into a region of size between 1 and
>4096" } */
>+ /* { dg-message "'__builtin_snprintf' output between 31 and 4126
>bytes into a destination of size 4096" "" { target *-*-* } .-1 } */
>+ bar (d);
>+ __builtin_snprintf (d, sizeof d,
>"%sfoobarbazquxquuxquuzcorgegarply", b); /* { dg-warning
>"'foobarbazquxquuxquuzcorgegarply' directive output may be truncated
>writing 31 bytes into a region of size between 1 and 4096" } */
>+ /* { dg-message "'__builtin_snprintf' output between 32 and 4127
>bytes into a destination of size 4096" "" { target *-*-* } .-1 } */
>+ bar (d);
>+ __builtin_snprintf (d, sizeof d,
>"%sfoobarfredquxquuxquuzcorgegarply", b); /* { dg-warning
>"'foobarfredquxquuxquuzcorgega\.\.\.' directive output may be truncated
>writing 32 bytes into a region of size between 1 and 4096" } */
>+ /* { dg-message "'__builtin_snprintf' output between 33 and 4128
>bytes into a destination of size 4096" "" { target *-*-* } .-1 } */
>+ bar (d);
>+}
>
> Jakub
[PATCH] Fix up target_to_host fn in gimple-ssa-sprintf.c (PR c++/90010)
Hi!
As the following testcase shows, target_to_host handled
targstr with strlen shorter than hostsz - 4 right (though with wasteful
clearing of the rest of the buffer when only one '\0' termination is enough)
and similarly also strlen hostsz and more right (making last 4 characters
in the buffer "..."), but anything in between resulted in random byte(s)
at the end of the string (strncpy with hostsz - 4 length didn't zero
terminate and because strlen (targstr) >= hostsz was false, nothing was
appended). When some target to host charset adjustment was needed, it
appended that "..." even to strings that actually didn't need it.
The following patch doesn't use "..." if targstr fits (including the zero
termination), otherwise makes sure last 4 bytes in the buffer are "...".
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-04-09 Jakub Jelinek
PR c++/90010
* gimple-ssa-sprintf.c (target_to_host): Fix handling of targstr
with strlen in between hostsz-3 and hostsz-1 inclusive when no
translation is needed, and when translation is needed, only append
... if the string length is hostsz or more bytes long. Avoid using
strncpy or strcat.
* gcc.dg/pr90010.c: New test.
--- gcc/gimple-ssa-sprintf.c.jj 2019-04-09 12:25:41.0 +0200
+++ gcc/gimple-ssa-sprintf.c2019-04-09 14:01:04.875445413 +0200
@@ -383,9 +383,14 @@ target_to_host (char *hostr, size_t host
overlong strings just like the translated strings are. */
if (target_to_host_charmap['\0'] == 1)
{
- strncpy (hostr, targstr, hostsz - 4);
- if (strlen (targstr) >= hostsz)
- strcpy (hostr + hostsz - 4, "...");
+ size_t len = strlen (targstr);
+ if (len >= hostsz)
+ {
+ memcpy (hostr, targstr, hostsz - 4);
+ strcpy (hostr + hostsz - 4, "...");
+ }
+ else
+ memcpy (hostr, targstr, len + 1);
return hostr;
}
@@ -399,10 +404,9 @@ target_to_host (char *hostr, size_t host
if (!*targstr)
break;
- if (size_t (ph - hostr) == hostsz - 4)
+ if (size_t (ph - hostr) == hostsz)
{
- *ph = '\0';
- strcat (ph, "...");
+ strcpy (ph - 4, "...");
break;
}
}
--- gcc/testsuite/gcc.dg/pr90010.c.jj 2019-04-09 14:22:14.795791124 +0200
+++ gcc/testsuite/gcc.dg/pr90010.c 2019-04-09 14:26:10.274961523 +0200
@@ -0,0 +1,27 @@
+/* PR c++/90010 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+char b[4096] = "abc";
+void bar (char *);
+
+void
+foo ()
+{
+ char d[4096];
+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzthudfred", b);
/* { dg-warning "'foobarbazquxquuxquuzthudfred' directive output may be
truncated writing 28 bytes into a region of size between 1 and 4096" } */
+ /* { dg-message "'__builtin_snprintf' output between 29 and 4124 bytes into
a destination of size 4096" "" { target *-*-* } .-1 } */
+ bar (d);
+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzcorgefred", b);
/* { dg-warning "'foobarbazquxquuxquuzcorgefred' directive output may be
truncated writing 29 bytes into a region of size between 1 and 4096" } */
+ /* { dg-message "'__builtin_snprintf' output between 30 and 4125 bytes into
a destination of size 4096" "" { target *-*-* } .-1 } */
+ bar (d);
+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzcorgewaldo", b);
/* { dg-warning "'foobarbazquxquuxquuzcorgewaldo' directive output may be
truncated writing 30 bytes into a region of size between 1 and 4096" } */
+ /* { dg-message "'__builtin_snprintf' output between 31 and 4126 bytes into
a destination of size 4096" "" { target *-*-* } .-1 } */
+ bar (d);
+ __builtin_snprintf (d, sizeof d, "%sfoobarbazquxquuxquuzcorgegarply", b);
/* { dg-warning "'foobarbazquxquuxquuzcorgegarply' directive output may be
truncated writing 31 bytes into a region of size between 1 and 4096" } */
+ /* { dg-message "'__builtin_snprintf' output between 32 and 4127 bytes into
a destination of size 4096" "" { target *-*-* } .-1 } */
+ bar (d);
+ __builtin_snprintf (d, sizeof d, "%sfoobarfredquxquuxquuzcorgegarply", b);
/* { dg-warning "'foobarfredquxquuxquuzcorgega\.\.\.' directive output may be
truncated writing 32 bytes into a region of size between 1 and 4096" } */
+ /* { dg-message "'__builtin_snprintf' output between 33 and 4128 bytes into
a destination of size 4096" "" { target *-*-* } .-1 } */
+ bar (d);
+}
Jakub
