Patchwork C++: fix ICE with CONST_DECLs

login
register
mail settings
Submitter Mike Stump
Date June 11, 2013, 4:27 p.m.
Message ID <F78BD9EE-3562-49C8-BD20-4BAE2E6AB900@comcast.net>
Download mbox | patch
Permalink /patch/250573/
State New
Headers show

Comments

Mike Stump - June 11, 2013, 4:27 p.m.
Here is a simple one.  When processing CONST_DECLs after an error, we can ICE.  This avoid the ICE.

Ok?
2013-06-11  Mike Stump  <mikestump@comcast.net>

	* init.c (constant_value_1): Protect CONST_DECLs better in the
          face of errors.
Paolo Carlini - June 11, 2013, 4:39 p.m.
On 06/11/2013 06:27 PM, Mike Stump wrote:
> Here is a simple one.  When processing CONST_DECLs after an error, we can ICE.  This avoid the ICE.
No testcase?

Paolo.
Jason Merrill - June 11, 2013, 6 p.m.
On 06/11/2013 12:39 PM, Paolo Carlini wrote:
> On 06/11/2013 06:27 PM, Mike Stump wrote:
>> Here is a simple one.  When processing CONST_DECLs after an error, we
>> can ICE.  This avoid the ICE.

> No testcase?

Yep; the patch is fine, but needs a testcase.

Jason
Mike Stump - June 13, 2013, 12:26 a.m.
On Jun 11, 2013, at 11:00 AM, Jason Merrill <jason@redhat.com> wrote:
> On 06/11/2013 12:39 PM, Paolo Carlini wrote:
>> On 06/11/2013 06:27 PM, Mike Stump wrote:
>>> Here is a simple one.  When processing CONST_DECLs after an error, we
>>> can ICE.  This avoid the ICE.
> 
>> No testcase?
> 
> Yep; the patch is fine, but needs a test case.

I don't have a test case, it was found via code where overflow was incorrectly set while compiling libstdc++, that test case won't work in the trunk, as the trunk sets overflow correctly…

I've tried all sorts of test cases to try and get it to fail, and net result it won't.  There are a few major paths into the code and most will transform to NULL_TREE or 0.  The test case was something like the below, though this version can't be made to show an error that I've found.  I don't see how to get it to fail.

I've fixed our code to compute overflow correctly and the crash is of course gone.  So, it is possible that it is impossible to get the code to fail.

So, I think I want to withdraw the patch, no need to mess up the code, for cases that can't happen.  If it does happen, someone will validly hit it and we can fix it at that time.  Thoughts?


namespace std
{
  // 22.2.2  The numeric category.
  class __num_base
  {
  public:
    // NB: Code depends on the order of _S_atoms_out elements.
    // Below are the indices into _S_atoms_out.
    enum
      {
	_S_ominus,
	_S_oplus,
	_S_ox,
	_S_oX,
	_S_odigits,
	_S_odigits_end = _S_odigits + 16,
	_S_oudigits = _S_odigits_end,
	_S_oudigits_end = _S_oudigits + 16,
	_S_oe = _S_odigits + 14,  // For scientific notation, 'e'
	_S_oE = _S_oudigits + 14, // For scientific notation, 'E'
	_S_oend = _S_oudigits_end
      };

  };
}
Jason Merrill - June 13, 2013, 12:36 a.m.
Go ahead and apply the patch; it never hurts to make the code more robust.

Jason

Patch

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 44e558e..133a162 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1988,7 +1988,7 @@  constant_value_1 (tree decl, bool integral_p, bool return_aggregate_cst_ok_p)
       init = DECL_INITIAL (decl);
       if (init == error_mark_node)
 	{
-	  if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
+	  if (VAR_P (decl) && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
 	    /* Treat the error as a constant to avoid cascading errors on
 	       excessively recursive template instantiation (c++/9335).  */
 	    return init;