For further explain of this patch, the `check_for_plus_in_loops`
function, as its comment describes, operates on a specific assumption:
if some pointer we are computing object size of is being increased
within a loop, assume all the SSA variables participating in that loop
have minimum object sizes 0.

The `check_for_plus_in_loops` function is only called by the
`compute_builtin_object_size` function when the `osi.reexamine` bit is
marked in previous passes.

Before commit 51b85dfeb19652bf3e0aaec08828ba7cee1e641c, when a pointer
was added to a pointer inside a loop, the reexamine bit would not be
marked because `size_valid_p(op1, object_size_type)` evaluated to false.
In this situation, `check_for_plus_in_loops` would not be called, and
therefore the `gcc_assert(TREE_CODE(cst) == INTEGER_CST)` assertion
within it would not be triggered.

This behavior changed after that commit. I have written a minimal
Internal Compiler Error (ICE) reproduction for the Alpine Linux
issue[1].

I believe it is safe to skip the check for pluses in loops if
`TREE_CODE(cst)` is not `INTEGER_CST` and to leave the reexamination
task for the next pass.

Looking forward to the feedback on this patch.

[1] https://gitlab.alpinelinux.org/alpine/aports/-/issues/17416#note_537040

------

In addition, I also noticed that there is another
`gcc_assert(TREE_CODE(cst) == INTEGER_CST)` expression in the
`check_for_plus_in_loops_1` function, but it appears to be dead code in
the following common scenario.

Statements other than then `var = basevar(variable) + cst(integer not
zero)` pattern are filtered out by `check_for_plus_in_loops` function.

For a statement `var = basevar + cst`, the data flow would always be:

check_for_plus_in_loops(osi, var)
  osi->depths[SSA_NAME_VERSION (basevar)] = 1;
  check_for_plus_in_loops_1(osi, var, 2) // depth = 2
    osi->depths[SSA_NAME_VERSION (var)] == 0 // continue

    // in the GIMPLE_ASSIGN branch of `switch (gimple_code (stmt))`
    check_for_plus_in_loops_1 (osi, basevar, depth + !integer_zerop (cst));
    // check_for_plus_in_loops_1(osi, basevar, 3)
      if osi->depths[SSA_NAME_VERSION (basevar)] != 0 // 1 != 0
        return; // end

However, I want to focus on fixing the ICE first and leave this other
code untouched for now.

-- 
Linsen Zhou
https://lin.moe

Attachment: signature.asc
Description: PGP signature

Reply via email to