| Message ID | 20101118141839.GX29412@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 18, 2010 at 3:18 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Nov 18, 2010 at 01:09:25PM +0100, Richard Guenther wrote: >> On Thu, Nov 18, 2010 at 12:06 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> > Hi! >> > >> > For the printf ("...\n") -> puts ("...") optimization we use alloca >> > to copy the string and change it before passing it to build_string_literal. >> > This doesn't work very well if the string is so long that we hit >> > RLIMIT_STACK. >> > >> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for >> > trunk? >> >> As stated in the PR we can avoid the copying completely if by >> adjusting tree.c:build_string to do this modification itself. No need >> to copy things twice. > > So something like this? > > Duplicating both build_string and build_string_literal for this special case > would be ugly. > > 2010-11-18 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/46534 > * builtins.c (fold_builtin_printf): Don't copy and modify string > before build_string_literal, instead modify what > build_string_literal returned. > > * gcc.c-torture/compile/pr46534.c: New test. > > --- gcc/builtins.c.jj 2010-11-18 10:00:20.040373470 +0100 > +++ gcc/builtins.c 2010-11-18 14:48:06.826405229 +0100 > @@ -12892,15 +12892,28 @@ fold_builtin_printf (location_t loc, tre > { > /* If the string was "string\n", call puts("string"). */ > size_t len = strlen (str); > - if ((unsigned char)str[len - 1] == target_newline) > + if ((unsigned char)str[len - 1] == target_newline > + && (size_t) (int) len == len > + && (int) len > 0) > { > + char *newstr; > + tree offset_node, string_cst; > + > /* Create a NUL-terminated string that's one char shorter > than the original, stripping off the trailing '\n'. */ > - char *newstr = XALLOCAVEC (char, len); > - memcpy (newstr, str, len - 1); > - newstr[len - 1] = 0; > - > - newarg = build_string_literal (len, newstr); > + newarg = build_string_literal (len, str); > + string_cst = string_constant (newarg, &offset_node); > + gcc_checking_assert (string_cst > + && (TREE_STRING_LENGTH (string_cst) > + == (int) len) > + && integer_zerop (offset_node) > + && (unsigned char) > + TREE_STRING_POINTER (string_cst)[len - 1] > + == target_newline); > + /* build_string_literal creates a new STRING_CST, > + modify it in place to avoid double copying. */ > + newstr = CONST_CAST (char *, TREE_STRING_POINTER (string_cst)); > + newstr[len - 1] = '\0'; Hm, yes. That works for me. Richard. > if (fn_puts) > call = build_call_expr_loc (loc, fn_puts, 1, newarg); > } > --- gcc/testsuite/gcc.c-torture/compile/pr46534.c.jj 2010-11-18 10:07:00.695450656 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr46534.c 2010-11-18 10:07:00.695450656 +0100 > @@ -0,0 +1,17 @@ > +/* PR middle-end/46534 */ > + > +extern int printf (const char *, ...); > + > +#define S1 " " > +#define S2 S1 S1 S1 S1 S1 S1 S1 S1 S1 S1 > +#define S3 S2 S2 S2 S2 S2 S2 S2 S2 S2 S2 > +#define S4 S3 S3 S3 S3 S3 S3 S3 S3 S3 S3 > +#define S5 S4 S4 S4 S4 S4 S4 S4 S4 S4 S4 > +#define S6 S5 S5 S5 S5 S5 S5 S5 S5 S5 S5 > +#define S7 S6 S6 S6 S6 S6 S6 S6 S6 S6 S6 > + > +void > +foo (void) > +{ > + printf (S7 "\n"); > +} > > > Jakub >
--- gcc/builtins.c.jj 2010-11-18 10:00:20.040373470 +0100 +++ gcc/builtins.c 2010-11-18 14:48:06.826405229 +0100 @@ -12892,15 +12892,28 @@ fold_builtin_printf (location_t loc, tre { /* If the string was "string\n", call puts("string"). */ size_t len = strlen (str); - if ((unsigned char)str[len - 1] == target_newline) + if ((unsigned char)str[len - 1] == target_newline + && (size_t) (int) len == len + && (int) len > 0) { + char *newstr; + tree offset_node, string_cst; + /* Create a NUL-terminated string that's one char shorter than the original, stripping off the trailing '\n'. */ - char *newstr = XALLOCAVEC (char, len); - memcpy (newstr, str, len - 1); - newstr[len - 1] = 0; - - newarg = build_string_literal (len, newstr); + newarg = build_string_literal (len, str); + string_cst = string_constant (newarg, &offset_node); + gcc_checking_assert (string_cst + && (TREE_STRING_LENGTH (string_cst) + == (int) len) + && integer_zerop (offset_node) + && (unsigned char) + TREE_STRING_POINTER (string_cst)[len - 1] + == target_newline); + /* build_string_literal creates a new STRING_CST, + modify it in place to avoid double copying. */ + newstr = CONST_CAST (char *, TREE_STRING_POINTER (string_cst)); + newstr[len - 1] = '\0'; if (fn_puts) call = build_call_expr_loc (loc, fn_puts, 1, newarg); } --- gcc/testsuite/gcc.c-torture/compile/pr46534.c.jj 2010-11-18 10:07:00.695450656 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr46534.c 2010-11-18 10:07:00.695450656 +0100 @@ -0,0 +1,17 @@ +/* PR middle-end/46534 */ + +extern int printf (const char *, ...); + +#define S1 " " +#define S2 S1 S1 S1 S1 S1 S1 S1 S1 S1 S1 +#define S3 S2 S2 S2 S2 S2 S2 S2 S2 S2 S2 +#define S4 S3 S3 S3 S3 S3 S3 S3 S3 S3 S3 +#define S5 S4 S4 S4 S4 S4 S4 S4 S4 S4 S4 +#define S6 S5 S5 S5 S5 S5 S5 S5 S5 S5 S5 +#define S7 S6 S6 S6 S6 S6 S6 S6 S6 S6 S6 + +void +foo (void) +{ + printf (S7 "\n"); +}