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.
> 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);
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
