Avoid resolving unresolved functions and classes (#2450)

This only may pose problems with DDTrace\Trace attributes. In theory having the hook installed twice should not pose problems, but adding a hook to an unresolved class is at the very least not a supported scenario. Checking for the classes being actually linked. This only affects PHP 8.0 and PHP 8.1. Signed-off-by: Bob Weinand <[email protected]>
DataDog · Jan 9, 2024 · 41cb273 · 41cb273
1 parent 77781a4
commit 41cb273
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 4 deletions.
diff --git a/tests/ext/traced_attribute.phpt b/tests/ext/traced_attribute.phpt
@@ -1,5 +1,5 @@
 --TEST--
-Set DDTrace\start_span() properties
+Test tracing via attributes
 --SKIPIF--
 <?php if (PHP_VERSION_ID < 80000) die('skip: No attributes pre-PHP 8'); ?>
 --ENV--

diff --git a/tests/ext/traced_attribute_delayed.phpt b/tests/ext/traced_attribute_delayed.phpt
@@ -0,0 +1,57 @@
+--TEST--
+Test delayed resolution of tracing attributes
+--SKIPIF--
+<?php if (PHP_VERSION_ID < 80000) die('skip: No attributes pre-PHP 8'); ?>
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+--FILE--
+<?php
+
+include __DIR__ . '/sandbox/dd_dumper.inc';
+
+$eval = <<<'EVAL'
+eval("class Bar {}");
+
+class Foo extends Bar {
+    #[DDTrace\Trace(name: "simpleclass")]
+    static function simple($arg) { return $arg; }
+}
+EVAL;
+
+$oldId = DDTrace\install_hook("somefunction", function() {});
+eval($eval);
+$newId = DDTrace\install_hook("somefunction", function() {});
+
+Foo::simple(1);
+
+var_dump($newId - $oldId - 1); // we check how many hooks were installed in between, to ensure that it was installed only once
+
+$eval = <<<'EVAL'
+if (time()) {
+    #[DDTrace\Trace(name: "simplefunc")]
+    function simple($arg) { return $arg; }
+}
+EVAL;
+
+$oldId = DDTrace\install_hook("somefunction", function() {});
+eval($eval);
+$newId = DDTrace\install_hook("somefunction", function() {});
+
+simple(1);
+
+var_dump($newId - $oldId - 1); // we check how many hooks were installed in between, to ensure that it was installed only once
+
+dd_dump_spans();
+
+?>
+--EXPECTF--
+int(1)
+int(1)
+spans(\DDTrace\SpanData) (2) {
+  simpleclass (traced_attribute_delayed.php, simpleclass, cli)
+    _dd.p.dm => -0
+    _dd.p.tid => %s
+  simplefunc (traced_attribute_delayed.php, simplefunc, cli)
+    _dd.p.dm => -0
+    _dd.p.tid => %s
+}
diff --git a/zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c b/zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c
@@ -13,16 +13,25 @@ static void zai_interceptor_add_new_entries(HashPosition classpos, HashPosition
     for (zend_class_entry *ce;
             (ce = zend_hash_get_current_data_ptr_ex(CG(class_table), &classpos));
             zend_hash_move_forward_ex(CG(class_table), &classpos)) {
-        zend_hash_get_current_key_ex(CG(class_table), &lcname, &index, &classpos);
-        zai_hook_resolve_class(ce, lcname);
+        if (ce->ce_flags & ZEND_ACC_LINKED) {
+            zend_hash_get_current_key_ex(CG(class_table), &lcname, &index, &classpos);
+            zai_hook_resolve_class(ce, lcname);
+        }
     }
 
     zend_hash_move_forward_ex(CG(function_table), &funcpos); // move past previous end
     for (zend_function *func;
             (func = zend_hash_get_current_data_ptr_ex(CG(function_table), &funcpos));
             zend_hash_move_forward_ex(CG(function_table), &funcpos)) {
         zend_hash_get_current_key_ex(CG(function_table), &lcname, &index, &funcpos);
-        zai_hook_resolve_function(func, lcname);
+#if PHP_VERSION_ID < 80100
+        // check for unlinked function: "rtd_key" (i.e. lcname) starts with NUL-byte
+        // On PHP 8.1+ these are attached to the op_array instead and not present in CG(function_table).
+        if (ZSTR_VAL(lcname)[0])
+#endif
+        {
+            zai_hook_resolve_function(func, lcname);
+        }
     }
 }