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

[Jeff Law <law at redhat dot com>]

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<msebor@redhat.com>
>
> 	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<msebor@redhat.com>
>
> 	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<msebor@redhat.com>
>
> 	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_hasher> *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 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.



Jeff