Fix potential crash when setting invalid declare value

[sgolemon]

Using a non-literal expression in a declare value can cause the
compiler to crash trying to turn that AST node into a usable zval.

There was an existing test for such values using 'encoding',
but that didn't crash because it's handled by the lexer
rather than being compiled.

Trying to use a non-literal with ticks reproduces the crash.

[krakjoe]

@sgolemon other declare tests are failing

[sgolemon]

Ah, yeah. Those tests skipped on my build because I don't have mbstring. Will update.

[nikic]

This should be handled in the compiler, either by checking whether you have an AST_ZVAL or whether the result of evaluation is a zval. The latter will allow constant expressions. It probably best not to allow constant expressions, as this would make it opcache-dependent (whether constants can be substituted or not).

[sgolemon]

Yeah, that was the other half of my email: http://news.php.net/php.internals/98335

I'm fine either way, and the compiler check for ZEND_AST_ZVAL will certainly be less likely to break existing code, but I think we should look at something like this PR for future PHP versions (target 7.3 maybe?) . I really don't think we need to support constant expressions in declares.

[sgolemon]

Compiler-only version of this patch is at: #2396

[nikic]

@sgolemon Even for 7.3, what is the advantage of handling this in the parser? The implementation looks more complicated, and additionally generates a lower-quality diagnostic.

[sgolemon]

Correctness? We don't want anything other than integers and quoted strings so we shouldn't accept them. Again, don't actually care much, and I do agree the error message in #2396 is way more clear.

[php-pulls]

Comment on behalf of pollita at php.net:

Closed in favor of #2396