Delay #[Attribute] arg validation until runtime

Fixes GH-13970
Closes GH-14105

We cannot validate at compile-time for multiple reasons:

* Evaluating the argument naively with zend_get_attribute_value can lead to code
  execution at compile time through the new expression, leading to possible
  reentrance of the compiler.
* Even if the evaluation was possible, it would need to be restricted to the
  current file, because constant values coming from other files can change
  without affecting the current compilation unit. For this reason, validation
  would need to be repeated at runtime anyway.
* Enums cannot be instantiated at compile-time (the actual bug report). This
  could be allowed here, because the value is immediately destroyed. But given
  the other issues, this won't be needed.

Instead, we just move it to runtime entirely. It's only needed for
ReflectionAttribute::newInstance(), which is not particularly a hot path. The
checks are also simple.

NEWS

@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed buffer limit on Windows, replacing read call usage by _read.
     (David Carlier)
 
+- Core:
+  . Fixed bug GH-13970 (Incorrect validation of #[Attribute] flags type for
+    non-compile-time expressions). (ilutov)
+
 - DOM:
   . Fix crashes when entity declaration is removed while still having entity
     references. (nielsdos)

Zend/tests/attributes/021_attribute_flags_type_is_validated.phpt

@@ -6,6 +6,15 @@ Attribute flags type is validated.
 #[Attribute("foo")]
 class A1 { }
 
+#[A1]
+class Foo {}
+
+try {
+    (new ReflectionClass(Foo::class))->getAttributes()[0]->newInstance();
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
 ?>
---EXPECTF--
-Fatal error: Attribute::__construct(): Argument #1 ($flags) must be of type int, string given in %s
+--EXPECT--
+Attribute::__construct(): Argument #1 ($flags) must be of type int, string given

Zend/tests/attributes/022_attribute_flags_value_is_validated.phpt

@@ -6,6 +6,15 @@ Attribute flags value is validated.
 #[Attribute(-1)]
 class A1 { }
 
+#[A1]
+class Foo { }
+
+try {
+    var_dump((new ReflectionClass(Foo::class))->getAttributes()[0]->newInstance());
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
 ?>
---EXPECTF--
-Fatal error: Invalid attribute flags specified in %s
+--EXPECT--
+Invalid attribute flags specified

Zend/tests/attributes/023_ast_node_in_validation.phpt

@@ -6,6 +6,15 @@ Attribute flags value is validated.
 #[Attribute(Foo::BAR)]
 class A1 { }
 
+#[A1]
+class Bar { }
+
+try {
+    var_dump((new ReflectionClass(Bar::class))->getAttributes()[0]->newInstance());
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
 ?>
---EXPECTF--
-Fatal error: Class "Foo" not found in %s on line %d
+--EXPECT--
+Class "Foo" not found

Zend/tests/attributes/032_attribute_validation_scope.phpt

@@ -1,9 +1,19 @@
 --TEST--
-Validation for "Attribute" does not use a scope when evaluating constant ASTs
+Validation for "Attribute" uses the class scope when evaluating constant ASTs
 --FILE--
 <?php
 #[Attribute(parent::x)]
 class x extends y {}
+
+class y {
+    protected const x = Attribute::TARGET_CLASS;
+}
+
+#[x]
+class z {}
+
+var_dump((new ReflectionClass(z::class))->getAttributes()[0]->newInstance());
 ?>
---EXPECTF--
-Fatal error: Cannot access "parent" when no class scope is active in %s on line %d
+--EXPECT--
+object(x)#1 (0) {
+}

Zend/tests/attributes/033_attribute_flags_type_is_not_validated_at_comp_time.phpt

@@ -0,0 +1,12 @@
+--TEST--
+Attribute flags type is not validated at compile time.
+--FILE--
+<?php
+
+#[Attribute("foo")]
+class A1 { }
+
+?>
+===DONE===
+--EXPECT--
+===DONE===

Zend/zend_attributes.c

@@ -35,31 +35,39 @@ static zend_object_handlers attributes_object_handlers_sensitive_parameter_value
 static HashTable internal_attributes;
 
 void validate_attribute(zend_attribute *attr, uint32_t target, zend_class_entry *scope)
+{
+}
+
+uint32_t zend_attribute_attribute_get_flags(zend_attribute *attr, zend_class_entry *scope)
 {
 	// TODO: More proper signature validation: Too many args, incorrect arg names.
 	if (attr->argc > 0) {
 		zval flags;
 
-		/* As this is run in the middle of compilation, fetch the attribute value without
-		 * specifying a scope. The class is not fully linked yet, and we may seen an
-		 * inconsistent state. */
-		if (FAILURE == zend_get_attribute_value(&flags, attr, 0, NULL)) {
-			return;
+		if (FAILURE == zend_get_attribute_value(&flags, attr, 0, scope)) {
+			ZEND_ASSERT(EG(exception));
+			return 0;
 		}
 
 		if (Z_TYPE(flags) != IS_LONG) {
-			zend_error_noreturn(E_ERROR,
+			zend_throw_error(NULL,
 				"Attribute::__construct(): Argument #1 ($flags) must be of type int, %s given",
 				zend_zval_type_name(&flags)
 			);
+			zval_ptr_dtor(&flags);
+			return 0;
 		}
 
-		if (Z_LVAL(flags) & ~ZEND_ATTRIBUTE_FLAGS) {
-			zend_error_noreturn(E_ERROR, "Invalid attribute flags specified");
+		uint32_t flags_l = Z_LVAL(flags);
+		if (flags_l & ~ZEND_ATTRIBUTE_FLAGS) {
+			zend_throw_error(NULL, "Invalid attribute flags specified");
+			return 0;
 		}
 
-		zval_ptr_dtor(&flags);
+		return flags_l;
 	}
+
+	return ZEND_ATTRIBUTE_TARGET_ALL;
 }
 
 static void validate_allow_dynamic_properties(

Zend/zend_attributes.h

@@ -85,6 +85,8 @@ ZEND_API zend_attribute *zend_add_attribute(
 		HashTable **attributes, zend_string *name, uint32_t argc,
 		uint32_t flags, uint32_t offset, uint32_t lineno);
 
+uint32_t zend_attribute_attribute_get_flags(zend_attribute *attr, zend_class_entry *scope);
+
 END_EXTERN_C()
 
 static zend_always_inline zend_attribute *zend_add_class_attribute(zend_class_entry *ce, zend_string *name, uint32_t argc)

ext/reflection/php_reflection.c

@@ -6739,16 +6739,9 @@ ZEND_METHOD(ReflectionAttribute, newInstance)
 	}
 
 	if (ce->type == ZEND_USER_CLASS) {
-		uint32_t flags = ZEND_ATTRIBUTE_TARGET_ALL;
-
-		if (marker->argc > 0) {
-			zval tmp;
-
-			if (FAILURE == zend_get_attribute_value(&tmp, marker, 0, ce)) {
-				RETURN_THROWS();
-			}
-
-			flags = (uint32_t) Z_LVAL(tmp);
+		uint32_t flags = zend_attribute_attribute_get_flags(marker, ce);
+		if (EG(exception)) {
+			RETURN_THROWS();
 		}
 
 		if (!(attr->target & flags)) {

ext/zend_test/tests/gh13970.phpt

@@ -0,0 +1,22 @@
+--TEST--
+GH-13970 (Incorrect validation of #[\Attribute]'s first parameter)
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+#[Attribute(\ZendTestUnitEnum::Foo)]
+class Foo {}
+
+#[Foo]
+function test() {}
+
+$reflection = new ReflectionFunction('test');
+
+try {
+    var_dump($reflection->getAttributes()[0]->newInstance());
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--EXPECT--
+Attribute::__construct(): Argument #1 ($flags) must be of type int, ZendTestUnitEnum given