On 01/28/2016 01:33 PM, Marek Polacek wrote:
This patch fixes some problems with VLAs and constexprs.  (The runtime aspect
of this matter is being tracked in PR69517.)  It does two things:

1) Changes build_vec_init slightly to prevent the compiler from getting into
    infinite recursion.  Currently, we emit the following sequence when we're
    initializing e.g. n = 1; int a[n] = { 1, 2 }; (cleanup_point junk omitted):

   int a[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)];
   (void) (int[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)] *)   int * D.2256;
   (void) (D.2256 = (int *) &a)
     int * D.2257;
   (void) (D.2257 = D.2256)
     long int D.2258;
   (void) (D.2258 = (long int) (SAVE_EXPR <(ssizetype) n + -1>)) // == 0
   (void) (*D.2257 = 1)
   (void)  ++D.2257
   (void)  --D.2258     // == -1
   (void) (*D.2257 = 2)
   (void)  ++D.2257
   (void)  --D.2258     // == -2

   while (1)
     {
       if (D.2258 == -1)        // never triggers
        goto <D.2261>;
       (void) (*D.2257 = 0)
       (void)  ++D.2257
       (void)  --D.2258;
     }
   <D.2261>:;
   ...

   So we first write the elements from the initializer and then we 
default-initialize
   any remaining elements.  But the iterator == -1 check is never true for 
invalid
   code, which means the compiler will get stuck in the while loop forever, 
leading
   to crash -- it tries to cxx_eval_* the body of the loop over and over again.

   Fixed by changing == -1 into <= -1; this should only ever happen for invalid 
code,
   but we don't want to ICE in any case.

   This also "fixes" the problem in PR69517 -- the program could get into 
infinite
   recursion as well, because it was evaluating the sequence above at runtime. 
But
   since it's invoking UB, it doesn't matter much.

2) Moves the check for "array subscript out of bound" a tad above, because for
    invalid VLAs we can't rely on the bool "found" -- e.g. for the example above
    it will find all indexes in the initializer, so "found" is true, which means
    we wouldn't get to the out-of-bounds check below.

I haven't had time to study the code but I did apply the patch
and play with it a bit.  It sure does fix the infinite loop/
recursion problem!

That said, while the test case below is rejected with the array
being of type int (as it should be), it's accepted as is, with
the struct.  I don't know enough to tell if it's because of
the change you mention or if it's a latent bug.

The test case is also accepted with an invalid argument to foo
(negative or excessive), both which should be rejected as well.

You're clearly good and efficient at fixing things.  I seem to
have a knack for breaking them.  Please let me know if I should
open a new bug for this.

  struct A {
    constexpr A (int = 0) { }
    constexpr operator int () const { return 0; }
  };

  constexpr int foo (int n) {
    A a [n] = { 1, 2, 3, 4, 5, 6 };
    return a [99];
  }

  struct B { unsigned b: foo (1) + 1; };

Martin

Reply via email to