]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c++: Fix UB in eval_data_member_spec [PR123920]
authorJakub Jelinek <jakub@redhat.com>
Tue, 3 Feb 2026 08:19:19 +0000 (09:19 +0100)
committerJakub Jelinek <jakub@gcc.gnu.org>
Tue, 3 Feb 2026 08:19:19 +0000 (09:19 +0100)
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.

gcc/cp/reflect.cc

index 24b8d74481049e670d63a035886297f34b625a9f..5b6ad8be14b679ceff9bcd5c5b5e9974581fea9a 100644 (file)
@@ -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);