This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix ICE on printf optimization with very large format string (PR middle-end/46534)
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';
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