Re: [PATCH] 69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align, with small alignment

[Jakub Jelinek <jakub at redhat dot com>]

On Sun, Feb 14, 2016 at 07:16:13PM -0700, Martin Sebor wrote:
> +    case BUILT_IN_ALLOCA_WITH_ALIGN:
> +      {
> +	/* Get the requested alignment (in bits) if it's a constant
> +	   integer expression.  */
> +	HOST_WIDE_INT align =
> +	  TREE_CODE (args [1]) == INTEGER_CST ? tree_to_uhwi (args [1]) : 0;

Formatting.  = needs to be on the next line.

> +	/* Determine the exact power of 2 of the requested alignment.  */
> +	int alignpow = align ? tree_log2 (args [1]) : 0;
> +
> +	/* Reject invalid alignments.  */
> +	if (alignpow < 3 || MAX_STACK_ALIGNMENT < align)

This looks wrong.  Both the hardcoding of 3 (IMHO you should instead
do alignpow == -1 || align < BITS_PER_UNIT for the low boundary.
And MAX_STACK_ALIGNMENT certainly is not the upper bound of the alignment,
that is from what I understand the code something so that the alignment
in bits fits into unsigned int.  And perhaps if you aren't going to
use alignpow for anything, you can just check the low and high boundaries
and otherwise check (align & (align - 1)) == 0, or integer_pow2p (args [1]).

> +	  {
> +	    error_at (EXPR_LOC_OR_LOC (args [1], input_location),

No space before [.

> +		      "second argument to function %qE must be a constant "
> +		      "integer power of 2 between %qi and %qwu bits",
> +		      fndecl, CHAR_TYPE_SIZE,

And here, you are printing unrelated value to the one that you've checked.

> +		      (unsigned HOST_WIDE_INT)MAX_STACK_ALIGNMENT);
> +	    return false;
> +	  }
> +      return true;
> +      }
> +

	Jakub