On Wed, Feb 26, 2020 at 05:54:03PM -0500, Jason Merrill wrote:
> On 2/26/20 3:44 PM, Marek Polacek wrote:
> > r7-2111 introduced maybe_constant_value in cp_fully_fold.
> > maybe_constant_value uses cxx_eval_outermost_constant_expr, which
> > can clear TREE_CONSTANT:
> > 6510 else if (non_constant_p && TREE_CONSTANT (r))
> > [...]
> > 6529 TREE_CONSTANT (r) = false;
> >
> > In this test the array size is '(long int) "h"'. This used to be
> > TREE_CONSTANT but given the change above, the flag will be cleared
> > when we cp_fully_fold the array size in compute_array_index_type_loc.
>
> I wonder about giving an error at that point; if size is TREE_CONSTANT and
> folded is not, we're in a strange situation already.
That works as well; how about the attached patch?
> > That means we don't emit an error in the
> > 10391 else if (TREE_CONSTANT (size)
> > block in the same function, and we go on. Then we compute ITYPE
> > using cp_build_binary_op and use maybe_constant_value on it and
> > suddenly we have something that's TREE_CONSTANT again.
>
> Why does maybe_constant_value consider (long)"h" - 1 to be a
> constant-expression?
That's because this check in cxx_eval_outermost_constant_expr:
if (CONVERT_EXPR_CODE_P (TREE_CODE (r))
&& ARITHMETIC_TYPE_P (TREE_TYPE (r))
&& TREE_CODE (TREE_OPERAND (r, 0)) == ADDR_EXPR)
{
if (!allow_non_constant)
error ("conversion from pointer type %qT "
"to arithmetic type %qT in a constant expression",
TREE_TYPE (TREE_OPERAND (r, 0)), TREE_TYPE (r));
non_constant_p = true;
}
doesn't work for all subexpressions, as the comment says. As a consequence,
this test
constexpr long int
foo ()
{
return (long int) "foo"
#ifdef FOO
- 1
#endif
;
}
constexpr long int l = foo ();
is accepted with -DFOO but rejected otherwise. Converting a pointer to an
integral type must be done via a reinterpret_cast and that can't be part of
a core constant expression. Do you want a PR for this?
-- >8 --
r7-2111 introduced maybe_constant_value in cp_fully_fold.
maybe_constant_value uses cxx_eval_outermost_constant_expr, which
can clear TREE_CONSTANT:
6510 else if (non_constant_p && TREE_CONSTANT (r))
[...]
6529 TREE_CONSTANT (r) = false;
In this test the array size is '(long int) "h"'. This used to be
TREE_CONSTANT but given the change above, the flag will be cleared
when we cp_fully_fold the array size in compute_array_index_type_loc.
That means we don't emit an error in the
10391 else if (TREE_CONSTANT (size)
block in the same function, and we go on. Then we compute ITYPE
using cp_build_binary_op and use maybe_constant_value on it and
suddenly we have something that's TREE_CONSTANT again. And then we
crash in reshape_init_array_1 in tree_to_uhwi, because what we have
doesn't fit in an unsigned HWI.
icc accepts this code, but since we used to reject it, I see no desire
to make this work, so don't use the folded result when we've lost
the TREE_CONSTANT flag while evaluating the size.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2020-02-26 Marek Polacek <pola...@redhat.com>
PR c++/93789 - ICE with invalid array bounds.
* decl.c (compute_array_index_type_loc): Don't use the folded
size when folding cleared TREE_CONSTANT.
* g++.dg/ext/vla22.C: New test.
---
gcc/cp/decl.c | 11 ++++++++---
gcc/testsuite/g++.dg/ext/vla22.C | 9 +++++++++
2 files changed, 17 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/vla22.C
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 1947c4ddb7f..e3f4b435a49 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10338,9 +10338,14 @@ compute_array_index_type_loc (location_t name_loc,
tree name, tree size,
pedwarn (loc, OPT_Wpedantic,
"size of array is not an integral constant-expression");
}
- /* Use the folded result for VLAs, too; it will have resolved
- SIZEOF_EXPR. */
- size = folded;
+ if (TREE_CONSTANT (size) && !TREE_CONSTANT (folded))
+ /* We might have lost the TREE_CONSTANT flag e.g. when we are
+ folding a conversion from a pointer to integral type. In that
+ case issue an error below and don't treat this as a VLA. */;
+ else
+ /* Use the folded result for VLAs, too; it will have resolved
+ SIZEOF_EXPR. */
+ size = folded;
}
/* Normally, the array-bound will be a constant. */
diff --git a/gcc/testsuite/g++.dg/ext/vla22.C b/gcc/testsuite/g++.dg/ext/vla22.C
new file mode 100644
index 00000000000..2308ee748df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/vla22.C
@@ -0,0 +1,9 @@
+// PR c++/93789 - ICE with invalid array bounds.
+// { dg-do compile }
+// { dg-options "" }
+
+void
+f ()
+{
+ const int tbl[(long) "h"] = { 12 }; // { dg-error "size of array .tbl. is
not an integral constant-expression" }
+}
base-commit: 89f759ac2ebb9f09ce5655ce5d791793922c612d
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
