Re: [PATCH 2/2] Generate constant at start of loop, without UB

[Jørgen Kvalsvik]

On 09/04/2024 09:40, Richard Biener wrote:

On Mon, 8 Apr 2024, Jørgen Kvalsvik wrote:

Generating the constants used for recording the edges taken for
condition coverage would trigger undefined behavior when an expression
had exactly 64 (== sizeof (1ULL)) conditions, as it would generate the
constant for the next iteration at the end of the loop body, even if there
was never a next iteration. By moving the check and constant generation
to the top of the loop and hoisting the increment flag there is no
opportunity for UB.

OK.

Thanks,
Richard.

Thanks, committed.

        PR 114627

gcc/ChangeLog:

        * tree-profile.cc (instrument_decisions): Generate constant
        at the start of loop.
---
  gcc/tree-profile.cc | 20 +++++++++++++++-----
  1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index 33ff550a7bc..e58f5c83472 100644
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -1049,6 +1049,7 @@ instrument_decisions (array_slice<basic_block> expr, 
size_t condno,
      zerocounter[2] = zero;

unsigned xi = 0;

+    bool increment = false;
      tree rhs = build_int_cst (gcov_type_node, 1ULL << xi);
      for (basic_block current : expr)
      {
@@ -1057,7 +1058,14 @@ instrument_decisions (array_slice<basic_block> expr, 
size_t condno,
            candidates.safe_push (zerocounter);
        counters prev = resolve_counters (candidates);

- int increment = 0;

+       if (increment)
+       {
+           xi += 1;
+           gcc_checking_assert (xi < sizeof (uint64_t) * BITS_PER_UNIT);
+           rhs = build_int_cst (gcov_type_node, 1ULL << xi);
+           increment = false;
+       }
+
        for (edge e : current->succs)
        {
            counters next = prev;
@@ -1072,7 +1080,7 @@ instrument_decisions (array_slice<basic_block> expr, 
size_t condno,
                    tree m = build_int_cst (gcov_type_node, masks[2*xi + k]);
                    next[2] = emit_bitwise_op (e, prev[2], BIT_IOR_EXPR, m);
                }
-               increment = 1;
+               increment = true;
            }
            else if (e->flags & EDGE_COMPLEX)
            {
@@ -1085,11 +1093,13 @@ instrument_decisions (array_slice<basic_block> expr, 
size_t condno,
            }
            table.get_or_insert (e->dest).safe_push (next);
        }
-       xi += increment;
-       if (increment)
-           rhs = build_int_cst (gcov_type_node, 1ULL << xi);
      }

+ /* Since this is also the return value, the number of conditions, make sure

+       to include the increment of the last basic block.  */
+    if (increment)
+       xi += 1;
+
      gcc_assert (xi == bitmap_count_bits (core));

const tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);