aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2026-02-03 09:19:19 +0100
committerJakub Jelinek <jakub@gcc.gnu.org>2026-02-03 09:19:19 +0100
commita08ac15e8cc2c5b55a6bb1acdf279e34ebc9251e (patch)
treebc6ddeb73435eb20813b32a07ec8edad5eda1351
parent31ca0008418821367eaec1fa7750c18442d18c5e (diff)
downloadgcc-trunk.zip
gcc-trunk.tar.gz
gcc-trunk.tar.bz2
c++: Fix UB in eval_data_member_spec [PR123920]HEADtrunkmaster
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. 2026-02-03 Jakub Jelinek <jakub@redhat.com> 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.
-rw-r--r--gcc/cp/reflect.cc8
1 files changed, 3 insertions, 5 deletions
diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc
index 24b8d74..5b6ad8b 100644
--- a/gcc/cp/reflect.cc
+++ b/gcc/cp/reflect.cc
@@ -5704,10 +5704,10 @@ eval_data_member_spec (location_t loc, const constexpr_ctx *ctx,
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;
}
@@ -5715,8 +5715,6 @@ eval_data_member_spec (location_t loc, const constexpr_ctx *ctx,
{
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);