On Wed, Mar 12, 2025 at 12:01 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The PR119204 r15-7955 fix caused some regressions.
> The problem is that the fold_builtin* APIs document that expr is
> either a CALL_EXPR of the call or NULL, so using TREE_TYPE (expr)
> can crash e.g. during constexpr evaluation etc.
>
> As can be seen in the surrounding patch, for the neighbouring builtins
> (both modf and strpbrk) fold_builtin_2 passes down type, which is the
> result type, TREE_TYPE (TREE_TYPE (fndecl)) and those builtins use it
> to build the return value, while strspn was always building size_type_node
> and strcspn had this change from that to TREE_TYPE (expr).
> The patch passes type to these two and uses it there as well.
>
> The patch keeps passing expr because it is used in the
> check_nul_terminated_array calls done for both strspn and strcspn,
> those calls clearly can deal with NULL expr but prefer if it is non-NULL
> for some warning.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I think it would be useful to add the C testcase that fails during DOM
from PR 119226 since that is another path to get the ICE.
Thanks,
Andrew Pinski
>
> 2025-03-12 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/119204
> PR middle-end/119219
> * builtins.cc (fold_builtin_2): Pass type as another argument
> to fold_builtin_strspn and fold_builtin_strcspn.
> (fold_builtin_strspn): Add type argument, use it instead of
> size_type_node.
> (fold_builtin_strcspn): Add type argument, use it instead of
> TREE_TYPE (expr).
>
> --- gcc/builtins.cc.jj 2025-03-11 12:05:42.561533453 +0100
> +++ gcc/builtins.cc 2025-03-11 15:36:47.774314011 +0100
> @@ -176,8 +176,8 @@ static tree fold_builtin_iseqsig (locati
> static tree fold_builtin_varargs (location_t, tree, tree*, int);
>
> static tree fold_builtin_strpbrk (location_t, tree, tree, tree, tree);
> -static tree fold_builtin_strspn (location_t, tree, tree, tree);
> -static tree fold_builtin_strcspn (location_t, tree, tree, tree);
> +static tree fold_builtin_strspn (location_t, tree, tree, tree, tree);
> +static tree fold_builtin_strcspn (location_t, tree, tree, tree, tree);
>
> static rtx expand_builtin_object_size (tree);
> static rtx expand_builtin_memory_chk (tree, rtx, machine_mode,
> @@ -10800,10 +10800,10 @@ fold_builtin_2 (location_t loc, tree exp
> return fold_builtin_modf (loc, arg0, arg1, type);
>
> case BUILT_IN_STRSPN:
> - return fold_builtin_strspn (loc, expr, arg0, arg1);
> + return fold_builtin_strspn (loc, expr, arg0, arg1, type);
>
> case BUILT_IN_STRCSPN:
> - return fold_builtin_strcspn (loc, expr, arg0, arg1);
> + return fold_builtin_strcspn (loc, expr, arg0, arg1, type);
>
> case BUILT_IN_STRPBRK:
> return fold_builtin_strpbrk (loc, expr, arg0, arg1, type);
> @@ -11304,7 +11304,7 @@ fold_builtin_strpbrk (location_t loc, tr
> form of the builtin function call. */
>
> static tree
> -fold_builtin_strspn (location_t loc, tree expr, tree s1, tree s2)
> +fold_builtin_strspn (location_t loc, tree expr, tree s1, tree s2, tree type)
> {
> if (!validate_arg (s1, POINTER_TYPE)
> || !validate_arg (s2, POINTER_TYPE))
> @@ -11320,8 +11320,7 @@ fold_builtin_strspn (location_t loc, tre
> if ((p1 && *p1 == '\0') || (p2 && *p2 == '\0'))
> /* Evaluate and ignore both arguments in case either one has
> side-effects. */
> - return omit_two_operands_loc (loc, size_type_node, size_zero_node,
> - s1, s2);
> + return omit_two_operands_loc (loc, type, size_zero_node, s1, s2);
> return NULL_TREE;
> }
>
> @@ -11344,7 +11343,7 @@ fold_builtin_strspn (location_t loc, tre
> form of the builtin function call. */
>
> static tree
> -fold_builtin_strcspn (location_t loc, tree expr, tree s1, tree s2)
> +fold_builtin_strcspn (location_t loc, tree expr, tree s1, tree s2, tree type)
> {
> if (!validate_arg (s1, POINTER_TYPE)
> || !validate_arg (s2, POINTER_TYPE))
> @@ -11360,8 +11359,7 @@ fold_builtin_strcspn (location_t loc, tr
> {
> /* Evaluate and ignore argument s2 in case it has
> side-effects. */
> - return omit_one_operand_loc (loc, TREE_TYPE (expr),
> - size_zero_node, s2);
> + return omit_one_operand_loc (loc, type, size_zero_node, s2);
> }
>
> /* If the second argument is "", return __builtin_strlen(s1). */
> @@ -11375,7 +11373,7 @@ fold_builtin_strcspn (location_t loc, tr
> if (!fn)
> return NULL_TREE;
>
> - return fold_convert_loc (loc, TREE_TYPE (expr),
> + return fold_convert_loc (loc, type,
> build_call_expr_loc (loc, fn, 1, s1));
> }
> return NULL_TREE;
>
> Jakub
>
