On Mon, Nov 3, 2025 at 7:21 AM Richard Biener <[email protected]> wrote:
>
> The following fixes path isolation changing the immediate use list of
> an SSA name that is currently iterated over via FOR_EACH_IMM_USE_STMT.
> This happens when it duplicates a BB within this and creates/modifies
> stmts that contain SSA uses of the name and calls update_stmt which
> re-builds SSA operands, including removal of SSA uses and re-inserting
> them. This is not safe as that might cause missed iterated uses but
> more importantly could cause the 'next' use to be removed.
>
> For the case in question the fix is to collect interesting uses in
> a vec and do the processing outside of the FOR_EACH_IMM_USE_STMT
> iteration.
>
> * gimple-ssa-isolated-paths.cc (check_loadstore): Set
> the volatile flag on the stmt manually.
> (find_implicit_erroneous_behavior): Move code transform
> outside of FOR_EACH_IMM_USE_STMT iteration.
> ---
> gcc/gimple-ssa-isolate-paths.cc | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/gimple-ssa-isolate-paths.cc b/gcc/gimple-ssa-isolate-paths.cc
> index ca1daf1d168..ffa04d15052 100644
> --- a/gcc/gimple-ssa-isolate-paths.cc
> +++ b/gcc/gimple-ssa-isolate-paths.cc
> @@ -56,7 +56,7 @@ check_loadstore (gimple *stmt, tree op, tree, void *data)
> {
> TREE_THIS_VOLATILE (op) = 1;
> TREE_SIDE_EFFECTS (op) = 1;
> - update_stmt (stmt);
> + gimple_set_has_volatile_ops (stmt, true);
> return true;
> }
> return false;
> @@ -762,6 +762,7 @@ find_implicit_erroneous_behavior (void)
>
> /* We've got a NULL PHI argument. Now see if the
> PHI's result is dereferenced within BB. */
> + auto_vec <gimple *, 4> uses_in_bb;
> FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> {
> /* We only care about uses in BB. Catching cases in
> @@ -774,18 +775,23 @@ find_implicit_erroneous_behavior (void)
> ? gimple_location (use_stmt)
> : phi_arg_loc;
>
> - if (stmt_uses_name_in_undefined_way (use_stmt, lhs, loc)
> - && (duplicate || can_duplicate_block_p (bb)))
> + if (stmt_uses_name_in_undefined_way (use_stmt, lhs, loc))
> {
> - duplicate = isolate_path (bb, duplicate, e,
> - use_stmt, lhs, false);
> -
> - /* When we remove an incoming edge, we need to
> - reprocess the Ith element. */
> - next_i = i;
> - cfg_altered = true;
> + if (!can_duplicate_block_p (bb))
> + break;
> + uses_in_bb.safe_push (use_stmt);
I think can_duplicate_block_p can be pulled further up. I think when
you did r13-3352-g5ad3cc1ecc3c7c you put it closest to the point of
duplication rather than earliest as possible.
Also it seems like handle_return_addr_local_phi_arg has the same issue
as find_implicit_erroneous_behavior with respect to
FOR_EACH_IMM_USE_STMT. Though maybe that one is hard to hit.
Thanks,
Andrew
> }
> }
> + for (gimple *use_stmt : uses_in_bb)
> + {
> + duplicate = isolate_path (bb, duplicate, e,
> + use_stmt, lhs, false);
> +
> + /* When we remove an incoming edge, we need to
> + reprocess the Ith element. */
> + next_i = i;
> + cfg_altered = true;
> + }
> }
> }
> }
> --
> 2.51.0
>
