On Wed, 25 Mar 2015, Jakub Jelinek wrote:
> Hi!
>
> As discussed in the PR, fixing this issue for real (make sure we at least
> until the objsz pass don't lose information on which field's address if any
> has been taken) is probably too dangerous at this point, so this patch
> just adds a simple workaround:
> another objsz pass instance run early before first ccp pass, in which we
> only process __bos (x, 1) and __bos (x, 3), and rather than folding them
> right away we instead just replace say
> _1 = __builtin_object_size (ptr_2, 1);
> with
> _7 = __builtin_object_size (ptr_2, 1);
> _1 = MIN <_7, 17>;
> if 17 is what the __builtin_object_size folds to. The reason for the MIN or
> MAX is that later DCE etc. could still make the value smaller later on
> (as shown in the third snippet of __builtin_object_size).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Looks good to me besides...
> For GCC 6 will need to write some real fix and revert this (except for the
> testcases).
>
> 2015-03-25 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/64715
> * passes.def: Add another instance of pass_object_sizes before
> ccp1.
> * tree-object-size.c (pass_object_sizes::execute): In
> first_pass_instance, only handle __bos (, 1) and __bos (, 3)
> calls, and keep the call in the IL, as {MIN,MAX}_EXPR of the
> __bos result and the computed constant. Remove redundant
> checks, obsoleted by gimple_call_builtin_p test. When propagating
> folded __bos into uses, if the use is {MIN,MAX}_EXPR we can fold
> into constant, propagate even that constant into their uses.
>
> * gcc.dg/builtin-object-size-15.c: New test.
> * gcc.dg/pr64715-1.c: New test.
> * gcc.dg/pr64715-2.c: New test.
>
> --- gcc/passes.def.jj 2015-01-19 14:40:46.000000000 +0100
> +++ gcc/passes.def 2015-03-25 12:18:21.079207954 +0100
> @@ -77,6 +77,7 @@ along with GCC; see the file COPYING3.
> PUSH_INSERT_PASSES_WITHIN (pass_all_early_optimizations)
> NEXT_PASS (pass_remove_cgraph_callee_edges);
> NEXT_PASS (pass_rename_ssa_copies);
> + NEXT_PASS (pass_object_sizes);
> NEXT_PASS (pass_ccp);
> /* After CCP we rewrite no longer addressed locals into SSA
> form if possible. */
> --- gcc/tree-object-size.c.jj 2015-03-20 17:58:31.000000000 +0100
> +++ gcc/tree-object-size.c 2015-03-25 14:40:03.664185560 +0100
> @@ -1268,25 +1268,60 @@ pass_object_sizes::execute (function *fu
> continue;
>
> init_object_sizes ();
> +
> + /* In the first pass instance, only attempt to fold
> + __builtin_object_size (x, 1) and __builtin_object_size (x, 3),
> + and rather than folding the builtin to the constant if any,
> + create a MIN_EXPR or MAX_EXPR of the __builtin_object_size
> + call result and the computed constant. */
> + if (first_pass_instance)
> + {
> + tree ost = gimple_call_arg (call, 1);
> + if (tree_fits_uhwi_p (ost))
> + {
> + unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
> + tree ptr = gimple_call_arg (call, 0);
> + tree lhs = gimple_call_lhs (call);
> + if ((object_size_type == 1 || object_size_type == 3)
> + && (TREE_CODE (ptr) == ADDR_EXPR
> + || TREE_CODE (ptr) == SSA_NAME)
> + && lhs)
> + {
> + tree type = TREE_TYPE (lhs);
> + unsigned HOST_WIDE_INT bytes
> + = compute_builtin_object_size (ptr, object_size_type);
> + if (bytes != (unsigned HOST_WIDE_INT) (object_size_type
> == 1
> + ? -1 : 0)
> + && wi::fits_to_tree_p (bytes, type))
> + {
> + tree tem = make_ssa_name (type);
> + gimple_call_set_lhs (call, tem);
> + enum tree_code code
> + = object_size_type == 1 ? MIN_EXPR : MAX_EXPR;
> + tree cst = build_int_cstu (type, bytes);
> + gimple g = gimple_build_assign (lhs, code, tem, cst);
> + gsi_insert_after (&i, g, GSI_NEW_STMT);
> + update_stmt (call);
> + }
> + }
> + }
> + continue;
> + }
> +
> result = fold_call_stmt (as_a <gcall *> (call), false);
> if (!result)
> {
> - if (gimple_call_num_args (call) == 2
> - && POINTER_TYPE_P (TREE_TYPE (gimple_call_arg (call, 0))))
> - {
> - tree ost = gimple_call_arg (call, 1);
> + tree ost = gimple_call_arg (call, 1);
>
> - if (tree_fits_uhwi_p (ost))
> - {
> - unsigned HOST_WIDE_INT object_size_type
> - = tree_to_uhwi (ost);
> + if (tree_fits_uhwi_p (ost))
> + {
> + unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
>
> - if (object_size_type < 2)
> - result = fold_convert (size_type_node,
> - integer_minus_one_node);
> - else if (object_size_type < 4)
> - result = build_zero_cst (size_type_node);
> - }
> + if (object_size_type < 2)
> + result = fold_convert (size_type_node,
> + integer_minus_one_node);
> + else if (object_size_type < 4)
> + result = build_zero_cst (size_type_node);
> }
>
> if (!result)
> @@ -1317,8 +1352,37 @@ pass_object_sizes::execute (function *fu
> FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> SET_USE (use_p, result);
> gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
> + enum tree_code use_code = ERROR_MARK;
> + if (is_gimple_assign (use_stmt))
> + use_code = gimple_assign_rhs_code (use_stmt);
> fold_stmt (&gsi);
> - update_stmt (gsi_stmt (gsi));
> + use_stmt = gsi_stmt (gsi);
> + if (use_stmt)
> + {
> + update_stmt (use_stmt);
> + /* objsz1 pass might produce MIN or MAX on the result.
> + If we manage to optimize them into INTEGER_CSTs,
> + propagate even those into all uses and fold those
> + stmts. */
> + if ((use_code == MIN_EXPR || use_code == MAX_EXPR)
> + && is_gimple_assign (use_stmt)
> + && gimple_assign_rhs_code (use_stmt) == INTEGER_CST)
> + {
> + imm_use_iterator iter2;
> + tree lhs2 = gimple_assign_lhs (use_stmt);
> + tree rhs = gimple_assign_rhs1 (use_stmt);
> + FOR_EACH_IMM_USE_STMT (use_stmt, iter2, lhs2)
> + {
> + FOR_EACH_IMM_USE_ON_STMT (use_p, iter2)
> + SET_USE (use_p, rhs);
> + gsi = gsi_for_stmt (use_stmt);
> + fold_stmt (&gsi);
> + use_stmt = gsi_stmt (gsi);
> + if (use_stmt)
> + update_stmt (use_stmt);
> + }
> + }
> + }
this hunk which I think is not really necessary given that
the late object-size pass now runs right before FRE which
performs constant propagation. My dev tree has the existing
code simplified to
/* Propagate into all uses. */
replace_uses_by (lhs, result);
but I guess we could now as well replace the __bos stmt via
update_call_from_tree.
Thus, ok without the extra propagation into min/max and with
or without cleanup opportunities I poitned out.
Thanks,
Richard.
> }
> }
> }
> --- gcc/testsuite/gcc.dg/builtin-object-size-15.c.jj 2015-03-25
> 13:01:46.735777306 +0100
> +++ gcc/testsuite/gcc.dg/builtin-object-size-15.c 2015-03-25
> 14:13:24.307094194 +0100
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +extern void abort (void);
> +
> +int
> +main ()
> +{
> + struct A { char buf1[9]; char buf2[1]; } a;
> +
> + if (__builtin_object_size (a.buf1 + (0 + 4), 1) != 5)
> + abort ();
> + char *p = a.buf1;
> + p += 1;
> + p += 3;
> + if (__builtin_object_size (p, 1) != 5)
> + abort ();
> + p = (char *) &a;
> + char *q = p + 1;
> + char *r = q + 3;
> + char *t = r;
> + if (r != (char *) &a + 4)
> + t = (char *) &a + 1;
> + if (__builtin_object_size (t, 1) != 6)
> + abort ();
> + return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr64715-1.c.jj 2015-03-25 13:42:15.369350086
> +0100
> +++ gcc/testsuite/gcc.dg/pr64715-1.c 2015-03-25 14:15:00.477536803 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/64715 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +extern inline __attribute__ ((always_inline, gnu_inline, artificial,
> nothrow, leaf)) char *
> +strcpy (char *__restrict dest, const char *__restrict src)
> +{
> + return __builtin___strcpy_chk (dest, src, __builtin_object_size (dest, 2 >
> 1));
> +}
> +
> +const char *str1 = "JIHGFEDCBA";
> +void bar (char *);
> +
> +void
> +foo ()
> +{
> + struct A { char buf1[9]; char buf2[1]; } a;
> + strcpy (a.buf1 + (0 + 4), str1 + 5);
> + bar ((char *) &a);
> +}
> +
> +/* { dg-final { scan-tree-dump "__builtin___strcpy_chk\[^;\n\r\]*, 5\\\);"
> "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> --- gcc/testsuite/gcc.dg/pr64715-2.c.jj 2015-03-25 14:46:18.453113325
> +0100
> +++ gcc/testsuite/gcc.dg/pr64715-2.c 2015-03-25 14:47:26.093017440 +0100
> @@ -0,0 +1,19 @@
> +/* PR tree-optimization/64715 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-objsz2" } */
> +
> +void bar (char *, int);
> +
> +void
> +foo (int x)
> +{
> + char p[16], *q;
> + q = p;
> + if (x)
> + q = p + 3;
> + __builtin___strcpy_chk (q, "abcdefghijkl", __builtin_object_size (q, 1));
> + bar (p, x);
> +}
> +
> +/* { dg-final { scan-tree-dump "__builtin_memcpy \\\(\[^;\n\r\]*,
> \"abcdefghijkl\", 13\\\);" "objsz2" } } */
> +/* { dg-final { cleanup-tree-dump "objsz2" } } */
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)
