PR tree-opt/33616: ICE on callee-copied constructor arguments

[Eric Botcazou]

> We try to take the address of the original CONSTRUCTOR (rather than
> taking the address of a temporary as in the caller-copied case) and
> expand_expr_addr_expr_1 assumes that all CONSTRUCTORs are constant:
>
>   /* If we are taking the address of a constant and are at the top level,
>      we have to use output_constant_def since we can't call force_const_mem
>      at top level.  */
>   /* ??? This should be considered a front-end bug.  We should not be
>      generating ADDR_EXPR of something that isn't an LVALUE.  The only
>      exception here is STRING_CST.  */
>   if (TREE_CODE (exp) == CONSTRUCTOR
>
>       || CONSTANT_CLASS_P (exp))
>
>     return XEXP (expand_expr_constant (exp, 0, modifier), 0);
>
> However, the comment doesn't really seem to fit this case:
> the CONSTRUCTOR _is_ an lvalue.

No, a CONSTRUCTOR is not an lvalue, it cannot be the LHS of an assignment.

> In the post-gimple world, all real constant CONSTRUCTORs (i.e. those that
> can be put in a readonly data segment) will already have been turned
> into decls, even for -O0.  And it seems perfectly valid to
> take the address of the non-constant constructor above.

Yes, taking the address of a CONSTRUCTOR is allowed and the effect is to 
create a temporary initialized with the contents of the constructor.

> The patch therefore treats CONSTRUCTORs in the same way as
> decls and language-specific codes, namely by passing them
> through expand_expr.  This will yield a MEM for the
> constructor contents.

I think that's OK because of initializer_constant_valid_p:

    case ADDR_EXPR:
    case FDESC_EXPR:
      value = staticp (TREE_OPERAND (value, 0));
      if (value)
	{
[...]
	  /* "&{...}" requires a temporary to hold the constructed
	     object.  */
	  if (TREE_CODE (value) == CONSTRUCTOR)
	    return NULL_TREE;
	}
      return value;

that is to say, expand_expr_addr_expr_1 is not supposed to be passed ADDR_EXPR 
of CONSTRUCTOR at top-level (any longer, this was allowed before).

And I think that you should remove the ??? comment

  /* ??? This should be considered a front-end bug.  We should not be
     generating ADDR_EXPR of something that isn't an LVALUE.  The only
     exception here is STRING_CST.  */

now that the blatant non-LVALUE case has been eliminated.

-- 
Eric Botcazou