On 2/3/26 3:53 PM, Jakub Jelinek wrote:
Hi!We can overflow buffer in eval_data_member_spec on some initializers. The code has 2 loops, one to figure out the needed length of the buffer and diagnose errors unsigned HOST_WIDE_INT l = 0; bool ntmbs = false; FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (f), k, field, value) if (!tree_fits_shwi_p (value)) goto fail; else if (field == NULL_TREE) { if (integer_zerop (value)) { ntmbs = true; break; } ++l; } else if (TREE_CODE (field) == RANGE_EXPR) { tree lo = TREE_OPERAND (field, 0); tree hi = TREE_OPERAND (field, 1); if (!tree_fits_uhwi_p (lo) || !tree_fits_uhwi_p (hi)) goto fail; if (integer_zerop (value)) { l = tree_to_uhwi (lo); ntmbs = true; break; } l = tree_to_uhwi (hi) + 1; } else if (tree_fits_uhwi_p (field)) { l = tree_to_uhwi (field); if (integer_zerop (value)) { ntmbs = true; break; } ++l; } else goto fail; if (!ntmbs || l > INT_MAX - 1) goto fail; which assumes that if there are designators, they need to be ascending, so no { [10] = 1, [2] = 3, [6] = 4, [4] = 5 } like in C11 and stops on the first '\0' seen (but remember in l the index of that. Here we need the 3 integer_zerop calls because the exact setting of l depends on if it is an elt without index, or with RANGE_EXPR index, or with normal INTEGER_CST index. And then there is a second loop where it just stores the values into the allocated buffer and does rely on the checking the first loop did. Now, for CONSTRUCTORs without indexes or with RANGE_EXPR only it also correctly stops at '\0', but I forgot to check that in the last case where index is INTEGER_CST: char *namep; unsigned len = l; if (l < 64) namep = XALLOCAVEC (char, l + 1); else namep = XNEWVEC (char, l + 1); memset (namep, 0, l + 1); l = 0; FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (f), k, field, value) if (field == NULL_TREE) { if (integer_zerop (value)) break; namep[l] = tree_to_shwi (value); ++l; } else if (TREE_CODE (field) == RANGE_EXPR) { tree lo = TREE_OPERAND (field, 0); tree hi = TREE_OPERAND (field, 1); if (integer_zerop (value)) break; unsigned HOST_WIDE_INT m = tree_to_uhwi (hi); for (l = tree_to_uhwi (lo); l <= m; ++l) namep[l] = tree_to_shwi (value); } else { l = tree_to_uhwi (field); namep[l++] = tree_to_shwi (value); } namep[len] = '\0'; So, I could add if (integer_zerop (value) break; into the else block, but then, in this loop there is no reason not to check integer_zerop (value) first because it is handled in all 3 cases the same. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
2026-02-03 Jakub Jelinek <[email protected]> PR c++/123920 * reflect.cc (eval_data_member_spec): Break out of the loop if value is integer_zerop even in the field && TREE_CODE (field) != RANGE_EXPR case and use a single test for integer_zerop (value) in the whole loop. --- gcc/cp/reflect.cc.jj 2026-01-30 17:44:17.045951022 +0100 +++ gcc/cp/reflect.cc 2026-02-02 11:48:19.364444672 +0100 @@ -5703,10 +5703,10 @@ eval_data_member_spec (location_t loc, c memset (namep, 0, l + 1); l = 0; FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (f), k, field, value) - if (field == NULL_TREE) + if (integer_zerop (value)) + break; + else if (field == NULL_TREE) { - if (integer_zerop (value)) - break; namep[l] = tree_to_shwi (value); ++l; } @@ -5714,8 +5714,6 @@ eval_data_member_spec (location_t loc, c { tree lo = TREE_OPERAND (field, 0); tree hi = TREE_OPERAND (field, 1); - if (integer_zerop (value)) - break; unsigned HOST_WIDE_INT m = tree_to_uhwi (hi); for (l = tree_to_uhwi (lo); l <= m; ++l) namep[l] = tree_to_shwi (value); Jakub
