Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On Tue, May 31, 2016 at 1:18 PM, Martin Sebor  wrote:
> On 03/31/2016 11:54 AM, Jason Merrill wrote:
>>
>> OK, thanks.
>
> This bug was fixed in 6.1 but it is also a GCC 5 regression and
> the patch hasn't been applied there yet.  Should I go ahead and
> backport it to the 5.x branch?

It seems safe enough to me, but since it touches fold-const let's get
a release manager opinion as well.  Jakub?

Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

On 03/31/2016 11:54 AM, Jason Merrill wrote:

OK, thanks.


This bug was fixed in 6.1 but it is also a GCC 5 regression and
the patch hasn't been applied there yet.  Should I go ahead and
backport it to the 5.x branch?

Martin

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

OK, thanks.

Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

+   /* Avoid folding references to struct members at offset
0 to
+  prevent tests like '>firstmember == 0' from getting
+  eliminated.  When ptr is null, although the -> expression
+  is strictly speaking invalid, GCC retains it as a matter
+  of QoI.  See PR c/44555. */
+   && (TREE_CODE (op0) != ADDR_EXPR
+   || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+   || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+   (TREE_OPERAND (op0, 0), 1))), 0))


Can we look at offset/bitpos here rather than examine the tree structure
of op0?  get_inner_reference already examined it for us.


Good suggestion, thanks!


But you're still examining the tree structure:


+   && (TREE_CODE (op0) != ADDR_EXPR
+   || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+   || bitpos0 != 0)


Here instead of looking at op0 we can check (offset0 == NULL_TREE &&
bitpos0 != 0), which indicates a constant non-zero offset.


+   && TREE_CODE (arg1) == INTEGER_CST
+   && integer_zerop (arg1))


And here you don't need the check for INTEGER_CST.


I've updated the patch to simplify the tests.

I have to confess I don't yet find it straightforward to tell
exactly which tests are essential to avoid an ICE and which
ones can safely be simplified.

Martin
PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-31  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-nullptr-1.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* g++.dg/ext/constexpr-vla3.C: Same.
	* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-31  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* constexpr.c (diag_array_subscript): New function.
	(cxx_eval_array_reference): Detect out of bounds array indices.

gcc/ChangeLog:
2016-03-31  Martin Sebor  

	PR c++/67376
	* fold-const.c (maybe_nonzero_address): New function.
	(fold_comparison): Call it.  Fold equality and relational
	expressions involving null pointers.
	(tree_single_nonzero_warnv_p): Call maybe_nonzero_address.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8ea7111..5d1b8b3 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1837,6 +1837,30 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false)
   return -1;
 }
 
+/* Under the control of CTX, issue a detailed diagnostic for
+   an out-of-bounds subscript INDEX into the expression ARRAY.  */
+
+static void
+diag_array_subscript (const constexpr_ctx *ctx, tree array, tree index)
+{
+  if (!ctx->quiet)
+{
+  tree arraytype = TREE_TYPE (array);
+
+  /* Convert the unsigned array subscript to a signed integer to avoid
+	 printing huge numbers for small negative values.  */
+  tree sidx = fold_convert (ssizetype, index);
+  if (DECL_P (array))
+	{
+	  error ("array subscript value %qE is outside the bounds "
+		 "of array %qD of type %qT", sidx, array, arraytype);
+	  inform (DECL_SOURCE_LOCATION (array), "declared here");
+	}
+  else
+	error ("array subscript value %qE is outside the bounds "
+	   "of array type %qT", sidx, arraytype);
+}
+}
 
 /* Subroutine of cxx_eval_constant_expression.
Attempt to reduce a reference to an array slot.  */
@@ -1885,8 +1909,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
   if (!tree_fits_shwi_p (index)
   || (i = tree_to_shwi (index)) < 0)
 {
-  if (!ctx->quiet)
-	error ("negative array subscript");
+  diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
   return t;
 }
@@ -1898,8 +1921,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
   VERIFY_CONSTANT (nelts);
   if (!tree_int_cst_lt (index, nelts))
 {
-  if (!ctx->quiet)
-	error ("array subscript out of bound");
+  diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
   return t;
 }
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 44fe2a2..eb66143 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8336,6 +8336,20 @@ pointer_may_wrap_p (tree base, tree offset, HOST_WIDE_INT bitpos)
   return 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/30/2016 01:25 PM, Jason Merrill wrote:

On 03/30/2016 12:32 PM, Martin Sebor wrote:

On 03/30/2016 09:30 AM, Jason Merrill wrote:

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as
POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);


I see.  Thanks for clarifying.  The asserts pass.  The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.


Good, then let's add at least the first to one of the tests.


+   /* Avoid folding references to struct members at offset 0 to
+  prevent tests like '>firstmember == 0' from getting
+  eliminated.  When ptr is null, although the -> expression
+  is strictly speaking invalid, GCC retains it as a matter
+  of QoI.  See PR c/44555. */
+   && (TREE_CODE (op0) != ADDR_EXPR
+   || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+   || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+   (TREE_OPERAND (op0, 0), 1))), 0))


Can we look at offset/bitpos here rather than examine the tree structure
of op0?  get_inner_reference already examined it for us.

Also, it looks like you aren't handling the case with the operands
switched, i.e. 0 == p and such.
Presumably all this stuff runs prior to the call to shorten_compare and 
friends which would canonicalize 0 == p into p == 0?


And yes, I still want to separate the canonicalization, warning and 
optimization that's done by shorten_compare and friends.  It just fell 
out of gcc-6 due to lack of time.


jeff





Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/30/2016 06:50 PM, Martin Sebor wrote:

On 03/30/2016 01:25 PM, Jason Merrill wrote:

On 03/30/2016 12:32 PM, Martin Sebor wrote:

On 03/30/2016 09:30 AM, Jason Merrill wrote:

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as
POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);


I see.  Thanks for clarifying.  The asserts pass.  The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.


Good, then let's add at least the first to one of the tests.


I've enhanced the new constexpr-nullptr-1.C test to verify this.
I added assertions exercising the relational expressions as well
and for sanity compiled the test with CLang.  It turns out that
it rejects the relational expressions with null pointers like
the one below complaining they aren't constant.

   constexpr int i = 0;
   constexpr const int *p = 
   constexpr int *q = 0;

   static_assert (q < p, "q < p");

I ended up not using a static_assert for the unspecified subset
even though GCC accepts it.  It seems that they really aren't
valid constant expressions (their results are unspecified for
null pointers) and should be rejected.  Do you agree?  (If you
do, I'll add these cases to c++/70248 that's already tracking
another unspecified case that GCC incorrectly accepts).


I agree.


+   /* Avoid folding references to struct members at offset 0 to
+  prevent tests like '>firstmember == 0' from getting
+  eliminated.  When ptr is null, although the -> expression
+  is strictly speaking invalid, GCC retains it as a matter
+  of QoI.  See PR c/44555. */
+   && (TREE_CODE (op0) != ADDR_EXPR
+   || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+   || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+   (TREE_OPERAND (op0, 0), 1))), 0))


Can we look at offset/bitpos here rather than examine the tree structure
of op0?  get_inner_reference already examined it for us.


Good suggestion, thanks!


But you're still examining the tree structure:


+  && (TREE_CODE (op0) != ADDR_EXPR
+  || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+  || bitpos0 != 0)


Here instead of looking at op0 we can check (offset0 == NULL_TREE && 
bitpos0 != 0), which indicates a constant non-zero offset.



+  && TREE_CODE (arg1) == INTEGER_CST
+  && integer_zerop (arg1))


And here you don't need the check for INTEGER_CST.


Also, it looks like you aren't handling the case with the operands
switched, i.e. 0 == p and such.


Based on my testing and reading the code I believe the caller
(fold_binary_loc) arranges for the constant argument to always
come second in comparisons.  I've added a comment to the code
to make it clear.


Great.

Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

On 03/30/2016 01:25 PM, Jason Merrill wrote:

On 03/30/2016 12:32 PM, Martin Sebor wrote:

On 03/30/2016 09:30 AM, Jason Merrill wrote:

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as
POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);


I see.  Thanks for clarifying.  The asserts pass.  The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.


Good, then let's add at least the first to one of the tests.


I've enhanced the new constexpr-nullptr-1.C test to verify this.
I added assertions exercising the relational expressions as well
and for sanity compiled the test with CLang.  It turns out that
it rejects the relational expressions with null pointers like
the one below complaining they aren't constant.

  constexpr int i = 0;
  constexpr const int *p = 
  constexpr int *q = 0;

  static_assert (q < p, "q < p");

I ended up not using a static_assert for the unspecified subset
even though GCC accepts it.  It seems that they really aren't
valid constant expressions (their results are unspecified for
null pointers) and should be rejected.  Do you agree?  (If you
do, I'll add these cases to c++/70248 that's already tracking
another unspecified case that GCC incorrectly accepts).


+   /* Avoid folding references to struct members at offset 0 to
+  prevent tests like '>firstmember == 0' from getting
+  eliminated.  When ptr is null, although the -> expression
+  is strictly speaking invalid, GCC retains it as a matter
+  of QoI.  See PR c/44555. */
+   && (TREE_CODE (op0) != ADDR_EXPR
+   || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+   || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+   (TREE_OPERAND (op0, 0), 1))), 0))


Can we look at offset/bitpos here rather than examine the tree structure
of op0?  get_inner_reference already examined it for us.


Good suggestion, thanks!



Also, it looks like you aren't handling the case with the operands
switched, i.e. 0 == p and such.


Based on my testing and reading the code I believe the caller
(fold_binary_loc) arranges for the constant argument to always
come second in comparisons.  I've added a comment to the code
to make it clear.

Attached is an updated patch retested on x86_64.

Martin
PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-30  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-nullptr-1.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* g++.dg/ext/constexpr-vla3.C: Same.
	* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-30  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* constexpr.c (diag_array_subscript): New function.
	(cxx_eval_array_reference): Detect out of bounds array indices.

gcc/ChangeLog:
2016-03-30  Martin Sebor  

	PR c++/67376
	* fold-const.c (maybe_nonzero_address): New function.
	(fold_comparison): Call it.  Fold equality and relational
	expressions involving null pointers.
	(tree_single_nonzero_warnv_p): Call maybe_nonzero_address.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8ea7111..5d1b8b3 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1837,6 +1837,30 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false)
   return -1;
 }
 
+/* Under the control of CTX, issue a detailed diagnostic for
+   an out-of-bounds subscript INDEX into the expression ARRAY.  */
+
+static void
+diag_array_subscript (const constexpr_ctx *ctx, tree array, tree index)
+{
+  if (!ctx->quiet)
+{
+  tree arraytype = TREE_TYPE (array);
+
+  /* Convert the unsigned array subscript to a signed integer to 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/30/2016 12:32 PM, Martin Sebor wrote:

On 03/30/2016 09:30 AM, Jason Merrill wrote:

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);


I see.  Thanks for clarifying.  The asserts pass.  The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.


Good, then let's add at least the first to one of the tests.


+  /* Avoid folding references to struct members at offset 0 to
+ prevent tests like '>firstmember == 0' from getting
+ eliminated.  When ptr is null, although the -> expression
+ is strictly speaking invalid, GCC retains it as a matter
+ of QoI.  See PR c/44555. */
+  && (TREE_CODE (op0) != ADDR_EXPR
+  || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+  || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+  (TREE_OPERAND (op0, 0), 1))), 0))


Can we look at offset/bitpos here rather than examine the tree structure 
of op0?  get_inner_reference already examined it for us.


Also, it looks like you aren't handling the case with the operands 
switched, i.e. 0 == p and such.


Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

On 03/30/2016 09:30 AM, Jason Merrill wrote:

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);


I see.  Thanks for clarifying.  The asserts pass.  The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.

Martin

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/29/2016 11:57 PM, Martin Sebor wrote:

Are we confident that arr[0] won't make it here as POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?


I want to be sure that we don't fold these conditions to false.

constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;

static_assert([0] == 0);
static_assert(&(ap->ar[0]) == 0);

Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

On 03/29/2016 12:54 PM, Jason Merrill wrote:

On 03/28/2016 06:04 PM, Martin Sebor wrote:

+   && compare_tree_int (arg1, 0) == 0)


This can be integer_zerop.


Sure.




+case GE_EXPR:
+case EQ_EXPR:
+case LE_EXPR:
+  return boolean_false_node;
+case GT_EXPR:
+case LT_EXPR:
+case NE_EXPR:
+  return boolean_true_node;


EQ and NE make sense, but I would expect both > and >= to be true, < and
<= to be false.


I was convinced I had a reason for this but it doesn't seem
to affect regression test results so I must have been wrong.

Relational expressions involving object and null pointers are
undefined in C and I thought unspecified in C++, but given that
GCC evaluates (0 < p) to true it looks like you're right and C++
does seem to require all the others to evaluate as you said.

With the decision to remove the nullptr changes I tried to keep
the amount of testing of null pointers to the minimum necessary
to exercise the fix for comment #10 on 67376.  In light of your
expectation I've added a test to better exercise the relational
expressions involving pointers to struct data members.
Interestingly, stepping through it revealed that the problem
cases you pointed out above are actually handled by
generic_simplify() and never end up in fold_comparison().

Attached is an updated patch.


Are we confident that arr[0] won't make it here as POINTER_PLUS_EXPR or
some such?


I'm as confident as I can be given that this is my first time
working in this area.  Which piece of code or what assumption
in particular are you concerned about?

Martin
PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-29  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-nullptr-1.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* g++.dg/ext/constexpr-vla3.C: Same.
	* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-29  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* constexpr.c (diag_array_subscript): New function.
	(cxx_eval_array_reference): Detect out of bounds array indices.

gcc/ChangeLog:
2016-03-29  Martin Sebor  

	PR c++/67376
	* fold-const.c (maybe_nonzero_address): New function.
	(fold_comparison): Call it.  Fold equality and relational
	expressions involving null pointers.
	(tree_single_nonzero_warnv_p): Call maybe_nonzero_address.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8ea7111..2415094 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1837,6 +1837,30 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false)
   return -1;
 }
 
+/* Under the control of CTX, issue a detailed diagnostic for
+   an out-of-bounds subscript INDEX into the expression ARRAY.  */
+
+static void
+diag_array_subscript (const constexpr_ctx *ctx, tree array, tree index)
+{
+  if (!ctx->quiet)
+{
+  tree arraytype = TREE_TYPE (array);
+
+  /* Convert the unsigned array subscript to a signed integer to avoid
+	 printing huge numbers for small negative values.  */
+  tree sidx = fold_convert (ssizetype, index);
+  if (DECL_P (array))
+	{
+	  error ("array subscript value %qE is outside the bounds "
+		 "of array %qD of type %qT", sidx, array, arraytype);
+	  inform (DECL_SOURCE_LOCATION (array), "declared here");
+	}
+  else
+	error ("array subscript value %qE is outside the bounds "
+	   "of array type %qT", sidx, arraytype);
+}
+}
 
 /* Subroutine of cxx_eval_constant_expression.
Attempt to reduce a reference to an array slot.  */
@@ -1861,6 +1885,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
 	false,
 	non_constant_p, overflow_p);
   VERIFY_CONSTANT (index);
+
   if (lval && ary == oldary && index == oldidx)
 return t;
   else if (lval)
@@ -1885,8 +1910,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
   if (!tree_fits_shwi_p (index)
   || (i = tree_to_shwi (index)) < 0)
 {
-  if (!ctx->quiet)
-	error ("negative array subscript");
+  diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
   return t;
 }
@@ -1898,8 +1922,7 @@ cxx_eval_array_reference (const 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/28/2016 06:04 PM, Martin Sebor wrote:

+  && compare_tree_int (arg1, 0) == 0)


This can be integer_zerop.


+   case GE_EXPR:
+   case EQ_EXPR:
+   case LE_EXPR:
+ return boolean_false_node;
+   case GT_EXPR:
+   case LT_EXPR:
+   case NE_EXPR:
+ return boolean_true_node;


EQ and NE make sense, but I would expect both > and >= to be true, < and 
<= to be false.


Are we confident that arr[0] won't make it here as POINTER_PLUS_EXPR or 
some such?


Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

I think let's defer the fix for c++/60760 (i.e. the nullptr_p bits)
until stage 1, when it can be combined with the POINTER_PLUS_EXPR fix,
and put the rest of this patch in now.


I can split up the patch into two and post the subset without
the fix for c++/60760, though I don't expect to be done with
it after I get back (next week).

I'd like to understand your concern with the fix for c++/60760.
Is it that it's incomplete (doesn't reject taking the address
of the first member of a struct, as in >first_member),
or are you worried that the changes may not be stable enough?


More the latter; it seems like significant new code and doesn't fix a
regression.


Attached is an updated patch without the fix for c++/60760, retested
on x86_64.

Martin
PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-18  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* g++.dg/ext/constexpr-vla3.C: Same.
	* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-18  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/70228
	* constexpr.c (diag_array_subscript): New function.
	(cxx_eval_array_reference): Detect out of bounds array indices.

gcc/ChangeLog:
2016-03-18  Martin Sebor  

	PR c++/67376
	* fold-const.c (maybe_nonzero_address): New function.
	(fold_comparison): Call it.  Fold equality and relational
	expressions involving null pointers.
	(tree_single_nonzero_warnv_p): Call maybe_nonzero_address.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 7776cac..c900080 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1837,6 +1837,30 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false)
   return -1;
 }
 
+/* Under the control of CTX, issue a detailed diagnostic for
+   an out-of-bounds subscript INDEX into the expression ARRAY.  */
+
+static void
+diag_array_subscript (const constexpr_ctx *ctx, tree array, tree index)
+{
+  if (!ctx->quiet)
+{
+  tree arraytype = TREE_TYPE (array);
+
+  /* Convert the unsigned array subscript to a signed integer to avoid
+	 printing huge numbers for small negative values.  */
+  tree sidx = fold_convert (ssizetype, index);
+  if (DECL_P (array))
+	{
+	  error ("array subscript value %qE is outside the bounds "
+		 "of array %qD of type %qT", sidx, array, arraytype);
+	  inform (DECL_SOURCE_LOCATION (array), "declared here");
+	}
+  else
+	error ("array subscript value %qE is outside the bounds "
+	   "of array type %qT", sidx, arraytype);
+}
+}
 
 /* Subroutine of cxx_eval_constant_expression.
Attempt to reduce a reference to an array slot.  */
@@ -1861,6 +1885,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
 	false,
 	non_constant_p, overflow_p);
   VERIFY_CONSTANT (index);
+
   if (lval && ary == oldary && index == oldidx)
 return t;
   else if (lval)
@@ -1885,8 +1910,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
   if (!tree_fits_shwi_p (index)
   || (i = tree_to_shwi (index)) < 0)
 {
-  if (!ctx->quiet)
-	error ("negative array subscript");
+  diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
   return t;
 }
@@ -1898,8 +1922,7 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
   VERIFY_CONSTANT (nelts);
   if (!tree_int_cst_lt (index, nelts))
 {
-  if (!ctx->quiet)
-	error ("array subscript out of bound");
+  diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
   return t;
 }
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 44fe2a2..3f65243 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8336,6 +8336,20 @@ pointer_may_wrap_p (tree base, tree offset, HOST_WIDE_INT bitpos)
   return total.to_uhwi () > (unsigned HOST_WIDE_INT) size;
 }
 
+/* Return a positive integer when the symbol DECL is known to have
+   a nonzero address, zero when it's known not to (e.g., it's a weak
+   symbol), and a negative integer when the symbol is not yet in the
+   symbol table and so whether or not its address is zero is unknown.  */
+static int
+maybe_nonzero_address (tree decl)
+{
+  if (DECL_P (decl) && decl_in_symtab_p 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/22/2016 04:01 PM, Martin Sebor wrote:

On 03/22/2016 12:52 PM, Jason Merrill wrote:

On 03/21/2016 06:09 PM, Jeff Law wrote:

On 03/21/2016 11:54 AM, Jason Merrill wrote:

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.

Do we know where the folding is happening for this case and is it
something we can reasonably defer?ie, is this just a case we
missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?


Yes, why is it already folded?



Let's pull that out into a separate BZ and tackle it for gcc-7.


I need to understand the issue before I agree to defer it.

It turns out that the problem is with how cp_build_binary_op calls
cp_pointer_int_sum and thus the c-common pointer_int_sum, which folds.

The POINTER_PLUS_EXPRs thus created have been a source of many issues
with constexpr evaluation, since it's impossible to reconstruct the
original expression, especially because POINTER_PLUS_EXPR uses an
unsigned second operand.  Deferring lowering to POINTER_PLUS_EXPR would
help a lot.  But it would indeed be a significant risk at this point.

I think let's defer the fix for c++/60760 (i.e. the nullptr_p bits)
until stage 1, when it can be combined with the POINTER_PLUS_EXPR fix,
and put the rest of this patch in now.


I can split up the patch into two and post the subset without
the fix for c++/60760, though I don't expect to be done with
it after I get back (next week).

I'd like to understand your concern with the fix for c++/60760.
Is it that it's incomplete (doesn't reject taking the address
of the first member of a struct, as in >first_member),
or are you worried that the changes may not be stable enough?


More the latter; it seems like significant new code and doesn't fix a 
regression.


Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

On 03/22/2016 12:52 PM, Jason Merrill wrote:

On 03/21/2016 06:09 PM, Jeff Law wrote:

On 03/21/2016 11:54 AM, Jason Merrill wrote:

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.

Do we know where the folding is happening for this case and is it
something we can reasonably defer?ie, is this just a case we missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?


Yes, why is it already folded?



Let's pull that out into a separate BZ and tackle it for gcc-7.


I need to understand the issue before I agree to defer it.

It turns out that the problem is with how cp_build_binary_op calls
cp_pointer_int_sum and thus the c-common pointer_int_sum, which folds.

The POINTER_PLUS_EXPRs thus created have been a source of many issues
with constexpr evaluation, since it's impossible to reconstruct the
original expression, especially because POINTER_PLUS_EXPR uses an
unsigned second operand.  Deferring lowering to POINTER_PLUS_EXPR would
help a lot.  But it would indeed be a significant risk at this point.

I think let's defer the fix for c++/60760 (i.e. the nullptr_p bits)
until stage 1, when it can be combined with the POINTER_PLUS_EXPR fix,
and put the rest of this patch in now.


I can split up the patch into two and post the subset without
the fix for c++/60760, though I don't expect to be done with
it after I get back (next week).

I'd like to understand your concern with the fix for c++/60760.
Is it that it's incomplete (doesn't reject taking the address
of the first member of a struct, as in >first_member),
or are you worried that the changes may not be stable enough?

Martin

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/21/2016 06:09 PM, Jeff Law wrote:

On 03/21/2016 11:54 AM, Jason Merrill wrote:

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.

Do we know where the folding is happening for this case and is it
something we can reasonably defer?ie, is this just a case we missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?


Yes, why is it already folded?



Let's pull that out into a separate BZ and tackle it for gcc-7.


I need to understand the issue before I agree to defer it.

It turns out that the problem is with how cp_build_binary_op calls 
cp_pointer_int_sum and thus the c-common pointer_int_sum, which folds.


The POINTER_PLUS_EXPRs thus created have been a source of many issues 
with constexpr evaluation, since it's impossible to reconstruct the 
original expression, especially because POINTER_PLUS_EXPR uses an 
unsigned second operand.  Deferring lowering to POINTER_PLUS_EXPR would 
help a lot.  But it would indeed be a significant risk at this point.


I think let's defer the fix for c++/60760 (i.e. the nullptr_p bits) 
until stage 1, when it can be combined with the POINTER_PLUS_EXPR fix, 
and put the rest of this patch in now.


Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/21/2016 11:54 AM, Jason Merrill wrote:

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.

Do we know where the folding is happening for this case and is it
something we can reasonably defer?ie, is this just a case we missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?


Yes, why is it already folded?

Let's pull that out into a separate BZ and tackle it for gcc-7.

jeff

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jason Merrill]

On 03/18/2016 01:04 PM, Jeff Law wrote:

On 03/17/2016 03:16 PM, Martin Sebor wrote:

The difficulty I've run into with detecting these problems in later
phases is that some invalid expressions have already been simplified
by the front end.  The example that applies here (even though this
is still the front end) is this:

Yea.  I was hoping that the delayed folding work would be helping in
getting a more faithful representation out of the front-ends.


It should.


   constexpr int* p = 0;
   constexpr bool b0 = [0] == 0;   // accepted
   constexpr bool b1 = [1] == 0;   // rejected

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.

Do we know where the folding is happening for this case and is it
something we can reasonably defer?ie, is this just a case we missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?


Yes, why is it already folded?

Jason

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/14/2016 03:25 PM, Martin Sebor wrote:

The attached patch fixes the outstanding cases mentioned in comment
10 on bug c++/67376.  While testing the fix I uncovered a number of
other related problems without which the test would have been
incomplete.  They include:

PR c++/70170 - [6 regression] bogus not a constant expression error
 comparing pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer
 error on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed
 in constant expressions

In addition, I include a fix for the issue below that I also came
across while testing the patch and that makes root causing constexpr
problems due to out-of-bounds array subscripts easier:
PR c++/70228 - insufficient detail in diagnostics for a constexpr
 out of bounds array subscript

In a discussion of bug 70170 between those CC'd Marek posted
a prototype patch for match.pd.  While the patch seems to do
the right thing as far as the bug goes, like my own first attempt
at a fix in const-fold.c it caused a couple of regressions (in
pr21294.c and in pr44555.c).  Since I'm not yet familiar enough
with match.pd, in the interest of time I solved those regressions
in const-fold.c rather than in match.pd.

Tested on x86_64.

Martin

gcc-67376.patch


PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed in constant
expressions
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
array subscript

gcc/testsuite/ChangeLog:
2016-03-14  Martin Sebor

PR c++/67376
PR c++/70170
PR c++/70172
PR c++/60760
PR c++/70228
* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
* g++.dg/cpp0x/constexpr-array-ptr11.C: New test.
* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
* g++.dg/cpp0x/constexpr-string.C: Same.
* g++.dg/cpp0x/constexpr-wstring2.C: Same.
* g++.dg/cpp0x/pr65398.C: Same.
* g++.dg/ext/constexpr-vla1.C: Same.
* g++.dg/ext/constexpr-vla2.C: Same.
* g++.dg/ext/constexpr-vla3.C: Same.
* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-14  Martin Sebor

PR c++/67376
PR c++/70170
PR c++/70172
PR c++/60760
PR c++/70228
(cxx_eval_binary_expression): Add argument.
(cxx_eval_component_reference): Same.
(cxx_eval_constant_expression): Same.
(cxx_eval_indirect_ref): Same.
(cxx_eval_outermost_constant_expr): Same.
(diag_array_subscript): New function.
* constexpr.c (cxx_eval_call_expression): Adjust.
(cxx_eval_conditional_expression): Same.
(cxx_eval_array_reference): Detect null pointers.
(cxx_eval_statement_list): Adjust.

gcc/ChangeLog:
2016-03-14  Martin Sebor

PR c++/67376
* fold-const.c (fold_comparison): Fold equality and relational
expressions involving null pointers.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 5f97c9d..5ec5034 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -918,7 +918,8 @@ struct constexpr_ctx {
  static GTY (()) hash_table *constexpr_call_table;

  static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
- bool, bool *, bool *, tree * = NULL);
+ bool, bool *, bool *, bool * = NULL,
+  tree * = NULL);
I didn't look deeply, but do you end up fixing all (most) of the callers 
of cxx_eval_constant_expression?  If so, then you don't need the default 
initialization.


At a high level, I'm curious your thoughts on emitting these errors out 
of the front-end rather than after analysis.  I'm thinking in particular 
about constant propagation,  unreachable code elimination and DCE.  The 
former can expose cases that are difficult to catch in the front-end, 
while the latter two might remove an out-of-range comparison/reference.



 diff --git a/gcc/fold-const.c b/gcc/fold-const.c

index 696b4a6..376aa09 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8639,6 +8639,37 @@ fold_comparison (location_t loc, enum tree_code code, 
tree type,
base1 = build_fold_addr_expr_loc (loc, base1);
  return fold_build2_loc (loc, code, type, base0, base1);
}
+
+  /* Comparison between an ordinary (non-weak) symbol and a null
+pointer can be 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

  static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
-  bool, bool *, bool *, tree * = NULL);
+  bool, bool *, bool *, bool * = NULL,
+  tree * = NULL);

I didn't look deeply, but do you end up fixing all (most) of the callers
of cxx_eval_constant_expression?  If so, then you don't need the default
initialization.


Thanks for the comments.  The patch only modifies about 10 out
of the 70 or so calls to the function in the file and the default
argument helps avoid making the rest of the changes.  (I have some
ideas for improving the APIs of these functions that I'd like to
run by Jason when we're done with the 6.0 work.)


At a high level, I'm curious your thoughts on emitting these errors out
of the front-end rather than after analysis.  I'm thinking in particular
about constant propagation,  unreachable code elimination and DCE.  The
former can expose cases that are difficult to catch in the front-end,
while the latter two might remove an out-of-range comparison/reference.


The difficulty I've run into with detecting these problems in later
phases is that some invalid expressions have already been simplified
by the front end.  The example that applies here (even though this
is still the front end) is this:

  constexpr int* p = 0;
  constexpr bool b0 = [0] == 0;   // accepted
  constexpr bool b1 = [1] == 0;   // rejected

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.


  diff --git a/gcc/fold-const.c b/gcc/fold-const.c

index 696b4a6..376aa09 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8639,6 +8639,37 @@ fold_comparison (location_t loc, enum tree_code
code, tree type,
  base1 = build_fold_addr_expr_loc (loc, base1);
return fold_build2_loc (loc, code, type, base0, base1);
  }
+
+  /* Comparison between an ordinary (non-weak) symbol and a null
+ pointer can be eliminated since such sybols must have a non
+ null address.  */

Hmm, I thought we already had code to do this somewhere.   It looks like
it's moved around quite a bit.  I think you want to be using
symtab_node::nonzero_address to determine if a given symbol must bind to
a nonzero address.


Thanks for the hint.  I had looked for existing functions but
couldn't find one that worked.  decl_with_nonnull_addr_p() in
c-common.c looked promising but it's not accessible here and
it doesn't do the right thing when HAS_DECL_ASSEMBLER_NAME_P()
is false (it ICEs).

There are a few functions in the fold-const.c itself that make
use of symtab_node::nonzero_address(), either directly or
indirectly (tree_expr_nonzero_p and tree_expr_nonzero_warnv_p)
but none is usable in this context (they don't handle VAR_DECL,
and adding that handling appears more involved than the current
approach).

In the end, I added a new function, maybe_nonzero_address(),
that calls symtab_node::nonzero_address(), and that I factored
out of tree_single_nonzero_warnv_p() that I was then able to
use in fold_comparison().

I've made a few other small adjustments to the patch to avoid
one false positive, and a few test cases, and tweak the expected
diagnostics now that Marek has fixed 70194.

I've also wrote myself a small sed script to replace blocks of
8 spaces with tabs and ran the patch through it.  I'll integrate
it into my workflow so I hopefully don't have to worry about this
ever again.

Martin

PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed in constant
	expressions
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-17  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/60760
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-nullptr.C: Add test cases.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/14/2016 04:13 PM, Jakub Jelinek wrote:

On Mon, Mar 14, 2016 at 03:25:07PM -0600, Martin Sebor wrote:

PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed in constant
expressions
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
array subscript


Can you please check up the formatting in the patch?
Seems e.g. you've replaced tons of tabs with 8 spaces etc. (check your
editor setting, and check the patch with contrib/check-GNU-style.sh).
There is some trailing whitespace too, spaces before [, etc.
Jakub, do you have any comments on the substance of the patch?  If so, 
it would help immensely if you could provide them so that Martin could 
address technical issues at the same time as he fixes up whitespace nits.


jeff

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/17/2016 03:16 PM, Martin Sebor wrote:



gcc-67376.patch


PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed in constant
expressions
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
array subscript

gcc/testsuite/ChangeLog:
2016-03-17  Martin Sebor

PR c++/67376
PR c++/70170
PR c++/70172
PR c++/60760
PR c++/70228
* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
* g++.dg/cpp0x/constexpr-nullptr.C: Add test cases.
* g++.dg/cpp0x/constexpr-string.C: Same.
* g++.dg/cpp0x/constexpr-wstring2.C: Same.
* g++.dg/cpp0x/pr65398.C: Same.
* g++.dg/ext/constexpr-vla1.C: Same.
* g++.dg/ext/constexpr-vla2.C: Same.
* g++.dg/ext/constexpr-vla3.C: Same.
* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-17  Martin Sebor

PR c++/67376
PR c++/70170
PR c++/70172
PR c++/60760
PR c++/70228
* constexpr.c (cxx_eval_binary_expression): Add argument.
(cxx_eval_component_reference): Same.
(cxx_eval_constant_expression): Same.
(cxx_eval_indirect_ref): Same.
(cxx_eval_outermost_constant_expr): Same.
(diag_array_subscript): New function.
(cxx_eval_call_expression): Adjust.
(cxx_eval_conditional_expression): Same.
(cxx_eval_array_reference): Detect null pointers.
(cxx_eval_statement_list): Adjust.

gcc/ChangeLog:
2016-03-17  Martin Sebor

PR c++/67376
* fold-const.c (maybe_nonzero_address): New function.
(fold_comparison): Call it.  Fold equality and relational
expressions involving null pointers.
(tree_single_nonzero_warnv_p): Call maybe_nonzero_address.

Index: gcc/cp/constexpr.c
===
--- gcc/cp/constexpr.c  (revision 234306)
+++ gcc/cp/constexpr.c  (working copy)
@@ -1839,11 +1874,26 @@ cxx_eval_array_reference (const constexp



@@ -3300,10 +3357,21 @@ cxx_eval_constant_expression (const cons
+
+  if (TREE_CODE (t) == INTEGER_CST
+ && TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE
+ && !integer_zerop (t))
+   {
+ if (!ctx->quiet)
+   error ("null pointer arithmetic in %qE", t);
+ if (nullptr_p)
+   *nullptr_p = true;
+   }

Something looks odd here.

You're testing !integer_zerop, so in T is going to be non-NULL, but you 
mentioned null pointer arithmetic in the error message.  Am I missing 
something here?





@@ -3738,15 +3812,32 @@ cxx_eval_constant_expression (const cons
Index: gcc/fold-const.c
@@ -8639,6 +8653,38 @@ fold_comparison (location_t loc, enum tr
base1 = build_fold_addr_expr_loc (loc, base1);
  return fold_build2_loc (loc, code, type, base0, base1);
}
+
+  /* Comparison between an ordinary (non-weak) symbol and a null
+pointer can be eliminated since such sybols must have a non
+null address.  */
+  else if (DECL_P (base0)
+  && maybe_nonzero_address (base0) > 0
+  // && (!HAS_DECL_ASSEMBLER_NAME_P (base0) || !DECL_WEAK (base0))

Please remove the commented out line.



+  /* Avoid folding references to struct members at offset 0 to
+ prevent tests like '>firstmember == 0' from getting
+ eliminated.  When ptr is null, although the -> expression
+ is strictly speaking invalid, GCC retains it as a matter
+ of QoI.  See PR c/44555. */
+  && (TREE_CODE (op0) != ADDR_EXPR
+  || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+  || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+  (TREE_OPERAND (op0, 0), 1))), 0))
+  && TREE_CODE (arg1) == INTEGER_CST
+  && compare_tree_int (arg1, 0) == 0)
+   {
+ switch (code)
+   {
+   case GE_EXPR:
+   case EQ_EXPR:
+   case LE_EXPR:
+ return boolean_false_node;
+   case GT_EXPR:
+   case LT_EXPR:
+   case NE_EXPR:
+ return boolean_true_node;
+   default: gcc_unreachable ();
Can you put the gcc_unreachable on a new line?  I know there's a few 
cases in the sources where it's on the default: line, but the vast 
majority have it on its own 

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jeff Law]

On 03/17/2016 03:16 PM, Martin Sebor wrote:

  static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
-  bool, bool *, bool *, tree * = NULL);
+  bool, bool *, bool *, bool * = NULL,
+  tree * = NULL);

I didn't look deeply, but do you end up fixing all (most) of the callers
of cxx_eval_constant_expression?  If so, then you don't need the default
initialization.


Thanks for the comments.  The patch only modifies about 10 out
of the 70 or so calls to the function in the file and the default
argument helps avoid making the rest of the changes.  (I have some
ideas for improving the APIs of these functions that I'd like to
run by Jason when we're done with the 6.0 work.)

OK.  Then let's keep the default initialization.




The difficulty I've run into with detecting these problems in later
phases is that some invalid expressions have already been simplified
by the front end.  The example that applies here (even though this
is still the front end) is this:
Yea.  I was hoping that the delayed folding work would be helping in 
getting a more faithful representation out of the front-ends.




   constexpr int* p = 0;
   constexpr bool b0 = [0] == 0;   // accepted
   constexpr bool b1 = [1] == 0;   // rejected

Both b0 and b1 are invalid and should be diagnosed, but only b1
is.  b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).

But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.
Do we know where the folding is happening for this case and is it 
something we can reasonably defer?ie, is this just a case we missed 
as part of the deferred folding work and hence should have its own 
distinct BZ to track?



Hmm, I thought we already had code to do this somewhere.   It looks like
it's moved around quite a bit.  I think you want to be using
symtab_node::nonzero_address to determine if a given symbol must bind to
a nonzero address.


Thanks for the hint.  I had looked for existing functions but
couldn't find one that worked.  decl_with_nonnull_addr_p() in
c-common.c looked promising but it's not accessible here and
it doesn't do the right thing when HAS_DECL_ASSEMBLER_NAME_P()
is false (it ICEs).
Yea, I found the same mis-mash of bits that didn't look directly usable 
for the problem you're tackling.  What's odd is I would have sworn that 
we had code to do exactly what you wanted, but I wasn't able to find it, 
either as a distinct routine or open-coded.




In the end, I added a new function, maybe_nonzero_address(),
that calls symtab_node::nonzero_address(), and that I factored
out of tree_single_nonzero_warnv_p() that I was then able to
use in fold_comparison().

Sounds good.



I've made a few other small adjustments to the patch to avoid
one false positive, and a few test cases, and tweak the expected
diagnostics now that Marek has fixed 70194.

I've also wrote myself a small sed script to replace blocks of
8 spaces with tabs and ran the patch through it.  I'll integrate
it into my workflow so I hopefully don't have to worry about this
ever again.
I'll try to take a look at the updated patch shortly.  It may still hit 
too much of the C++ front-end for me to be comfortable reviewing -- 
we'll see.


jeff

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jakub Jelinek]

On Wed, Mar 16, 2016 at 01:38:21PM -0600, Jeff Law wrote:
> On 03/14/2016 04:13 PM, Jakub Jelinek wrote:
> >On Mon, Mar 14, 2016 at 03:25:07PM -0600, Martin Sebor wrote:
> >>PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
> >>of array fails inside constant expression
> >>PR c++/70170 - [6 regression] bogus not a constant expression error 
> >>comparing
> >>pointer to array to null
> >>PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
> >>on invalid constexpr initialization
> >>PR c++/60760 - arithmetic on null pointers should not be allowed in constant
> >>expressions
> >>PR c++/70228 - insufficient detail in diagnostics for a constexpr out of 
> >>bounds
> >>array subscript
> >
> >Can you please check up the formatting in the patch?
> >Seems e.g. you've replaced tons of tabs with 8 spaces etc. (check your
> >editor setting, and check the patch with contrib/check-GNU-style.sh).
> >There is some trailing whitespace too, spaces before [, etc.
> Jakub, do you have any comments on the substance of the patch?  If so, it
> would help immensely if you could provide them so that Martin could address
> technical issues at the same time as he fixes up whitespace nits.

No, I'll defer technical comments to Jason.  The formatting is just
something that caught my eye during the 10 seconds or so spent on reading the
patch flying by.

Jakub

Re: [PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Jakub Jelinek]

On Mon, Mar 14, 2016 at 03:25:07PM -0600, Martin Sebor wrote:
> PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
>   of array fails inside constant expression
> PR c++/70170 - [6 regression] bogus not a constant expression error comparing
>   pointer to array to null
> PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
>   on invalid constexpr initialization
> PR c++/60760 - arithmetic on null pointers should not be allowed in constant
>   expressions
> PR c++/70228 - insufficient detail in diagnostics for a constexpr out of 
> bounds
>   array subscript

Can you please check up the formatting in the patch?
Seems e.g. you've replaced tons of tabs with 8 spaces etc. (check your
editor setting, and check the patch with contrib/check-GNU-style.sh).
There is some trailing whitespace too, spaces before [, etc.

Jakub

[PATCH] c++/67376 Comparison with pointer to past-the-end, of array fails inside constant expression

[Martin Sebor]

The attached patch fixes the outstanding cases mentioned in comment
10 on bug c++/67376.  While testing the fix I uncovered a number of
other related problems without which the test would have been
incomplete.  They include:

PR c++/70170 - [6 regression] bogus not a constant expression error
comparing pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer
error on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed
in constant expressions

In addition, I include a fix for the issue below that I also came
across while testing the patch and that makes root causing constexpr
problems due to out-of-bounds array subscripts easier:
PR c++/70228 - insufficient detail in diagnostics for a constexpr
out of bounds array subscript

In a discussion of bug 70170 between those CC'd Marek posted
a prototype patch for match.pd.  While the patch seems to do
the right thing as far as the bug goes, like my own first attempt
at a fix in const-fold.c it caused a couple of regressions (in
pr21294.c and in pr44555.c).  Since I'm not yet familiar enough
with match.pd, in the interest of time I solved those regressions
in const-fold.c rather than in match.pd.

Tested on x86_64.

Martin
PR c++/67376 - [5/6 regression] Comparison with pointer to past-the-end
	of array fails inside constant expression
PR c++/70170 - [6 regression] bogus not a constant expression error comparing
	pointer to array to null
PR c++/70172 - incorrect reinterpret_cast from integer to pointer error
	on invalid constexpr initialization
PR c++/60760 - arithmetic on null pointers should not be allowed in constant
	expressions
PR c++/70228 - insufficient detail in diagnostics for a constexpr out of bounds
	array subscript

gcc/testsuite/ChangeLog:
2016-03-14  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/60760
	PR c++/70228
	* g++.dg/cpp0x/constexpr-array-ptr10.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr11.C: New test.
	* g++.dg/cpp0x/constexpr-array-ptr9.C: New test.
	* g++.dg/cpp0x/constexpr-array5.C: Adjust text of expected diagnostic.
	* g++.dg/cpp0x/constexpr-string.C: Same.
	* g++.dg/cpp0x/constexpr-wstring2.C: Same.
	* g++.dg/cpp0x/pr65398.C: Same.
	* g++.dg/ext/constexpr-vla1.C: Same.
	* g++.dg/ext/constexpr-vla2.C: Same.
	* g++.dg/ext/constexpr-vla3.C: Same.
	* g++.dg/ubsan/pr63956.C: Same.

gcc/cp/ChangeLog:
2016-03-14  Martin Sebor  

	PR c++/67376
	PR c++/70170
	PR c++/70172
	PR c++/60760
	PR c++/70228
	(cxx_eval_binary_expression): Add argument.
	(cxx_eval_component_reference): Same.
	(cxx_eval_constant_expression): Same.
	(cxx_eval_indirect_ref): Same.
	(cxx_eval_outermost_constant_expr): Same.
	(diag_array_subscript): New function.
	* constexpr.c (cxx_eval_call_expression): Adjust.
	(cxx_eval_conditional_expression): Same.
	(cxx_eval_array_reference): Detect null pointers.
	(cxx_eval_statement_list): Adjust.

gcc/ChangeLog:
2016-03-14  Martin Sebor  

	PR c++/67376
	* fold-const.c (fold_comparison): Fold equality and relational
	expressions involving null pointers.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 5f97c9d..5ec5034 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -918,7 +918,8 @@ struct constexpr_ctx {
 static GTY (()) hash_table *constexpr_call_table;
 
 static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
-	  bool, bool *, bool *, tree * = NULL);
+	  bool, bool *, bool *, bool * = NULL,
+  tree * = NULL);
 
 /* Compute a hash value for a constexpr call representation.  */
 
@@ -1390,7 +1391,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	  tree jump_target = NULL_TREE;
 	  cxx_eval_constant_expression (ctx, body,
 	lval, non_constant_p, overflow_p,
-	_target);
+NULL, _target);
 
 	  if (DECL_CONSTRUCTOR_P (fun))
 	/* This can be null for a subobject constructor call, in
@@ -1607,20 +1608,21 @@ cxx_eval_unary_expression (const constexpr_ctx *ctx, tree t,
 static tree
 cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t,
 			bool /*lval*/,
-			bool *non_constant_p, bool *overflow_p)
+			bool *non_constant_p, bool *overflow_p,
+bool *nullptr_p)
 {
   tree r = NULL_TREE;
   tree orig_lhs = TREE_OPERAND (t, 0);
   tree orig_rhs = TREE_OPERAND (t, 1);
   tree lhs, rhs;
   lhs = cxx_eval_constant_expression (ctx, orig_lhs, /*lval*/false,
-  non_constant_p, overflow_p);
+  non_constant_p, overflow_p, nullptr_p);
   /* Don't VERIFY_CONSTANT here, it's unnecessary and will break pointer
  subtraction.  */
   if (*non_constant_p)
 return t;
   rhs = cxx_eval_constant_expression (ctx, orig_rhs, /*lval*/false,
-  non_constant_p, overflow_p);
+  non_constant_p, overflow_p, nullptr_p);
   if (*non_constant_p)
 return