Add line-hooking

.gitlab/benchmarks.yml

@@ -187,7 +187,10 @@ workflow:
       fi
     - git clone --branch dd-trace-php https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/benchmarking-platform /platform && cd /platform
     - ./steps/capture-hardware-software-info.sh
-    - BP_SCENARIO=$SCENARIO bp-runner bp-runner.yml --debug -t
+    # Keep submodules at the selected candidate and baseline revisions.
+    - >-
+      GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=submodule.recurse GIT_CONFIG_VALUE_0=true
+      BP_SCENARIO=$SCENARIO bp-runner bp-runner.yml --debug -t
     - ./steps/analyze-results.sh
     - "./steps/upload-results-to-s3.sh || :"
     - "./steps/post-pr-comment.sh || :"

.gitlab/generate-tracer.php

@@ -546,7 +546,8 @@ function windows_test_c_job($job_name, $thread_safety, $targets) {
     PHP_MAJOR_MINOR: "<?= $major_minor ?>"
     ARCH: "amd64"
     KUBERNETES_POD_ANNOTATIONS_1: "ci.ddbuild.io/enforce-static-cpus=true"
-  timeout: 120m
+  # PHP < 7.4 runs the Valgrind suite serially.
+  timeout: <?= version_compare($major_minor, "7.4", "<") ? 150 : 120 ?>m
   script:
     - make test_extension_ci_valgrind
 <?php after_script("tmp/build_extension", has_test_agent: true); ?>

Makefile

@@ -65,7 +65,7 @@ RUST_SYMBOL_FILES = $(addprefix $(BUILD_DIR)/, \
 	components-rs/libdatadog-php-linux.sym)
 RUST_FILES = $(BUILD_DIR)/Cargo.toml $(BUILD_DIR)/Cargo.lock $(RUST_SYMBOL_FILES) $(shell find components-rs profiling -name '*.c' -o -name '*.h' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find tracer -name '*.rs' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog \( -type l -o -type f \) \( -name '*.rs' -o -path '*/src*' -o -path '*/examples*' -o -name 'Cargo.toml' \) -not -path '*/target/*' -not -path '*/.git/*') $(SIDECAR_RUST_FILES)
 ALL_OBJECT_FILES = $(C_FILES) $(RUST_FILES) $(BUILD_DIR)/Makefile
-TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
+TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '*.inc' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 TEST_STUB_FILES = $(shell find tests/ext -type d -name 'stubs' -exec find '{}' -type f \; | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 INIT_HOOK_TEST_FILES = $(shell find tests/C2PHP -name '*.phpt' -o -name '*.inc' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 M4_FILES = $(shell find m4 -name '*.m4*' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(BUILD_DIR)/config.m4

config.m4

@@ -352,6 +352,7 @@ if test "$PHP_DDTRACE" != "no" && test "$PHP_DDTRACE_PROFILING" = "no"; then
     tracer/user_request.c \
     tracer/weak_resources.c \
     tracer/hook/uhook.c \
+    tracer/hook/uhook_line.c \
     tracer/hook/uhook_legacy.c \
   "
 
@@ -365,6 +366,9 @@ if test "$PHP_DDTRACE" != "no" && test "$PHP_DDTRACE_PROFILING" = "no"; then
     zend_abstract_interface/exceptions/exceptions.c \
     zend_abstract_interface/headers/headers.c \
     zend_abstract_interface/hook/hook.c \
+    zend_abstract_interface/hook/line_hooks.c \
+    zend_abstract_interface/interceptor/line_hook.c \
+    zend_abstract_interface/jit_utils/opcache_symbols.c \
     zend_abstract_interface/json/json.c \
     zend_abstract_interface/sandbox/call.c \
     zend_abstract_interface/uri_normalization/uri_normalization.c \

config.w32

@@ -92,7 +92,7 @@ if (PHP_DDTRACE != 'no') {
 		DDTRACE_TRACER_SOURCES += " handlers_fiber.c";
 	}
 
-	var DDTRACE_HOOK_SOURCES = "uhook.c uhook_legacy.c";
+	var DDTRACE_HOOK_SOURCES = "uhook.c uhook_legacy.c uhook_line.c";
 	if (version >= 800) {
 		DDTRACE_HOOK_SOURCES += " uhook_attributes.c uhook_otel.c";
 	}
@@ -128,11 +128,13 @@ if (PHP_DDTRACE != 'no') {
 	ADD_SOURCES(zai_dirname + "/env", "env.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/exceptions", "exceptions.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/headers", "headers.c", "ddtrace");
-	ADD_SOURCES(zai_dirname + "/hook", "hook.c", "ddtrace");
+	ADD_SOURCES(zai_dirname + "/hook", "hook.c line_hooks.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/json", "json.c", "ddtrace");
     ADD_SOURCES(zai_dirname + "/sandbox", "call.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/zai_string", "string.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/uri_normalization", "uri_normalization.c", "ddtrace");
+	ADD_SOURCES(zai_dirname + "/interceptor", "line_hook.c", "ddtrace");
+	ADD_SOURCES(zai_dirname + "/jit_utils", "opcache_symbols.c", "ddtrace");
 	if (version < 800) {
 		ADD_SOURCES(zai_dirname + "/sandbox/php7", "sandbox.c", "ddtrace");
 		ADD_SOURCES(zai_dirname + "/interceptor/php7", "interceptor.c resolver.c", "ddtrace");

tests/ext/hook/line_hook_abstract_decl_line.phpt

@@ -0,0 +1,46 @@
+--TEST--
+A request landing on an abstract method declaration arms that method, whose synthetic return never runs
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+abstract class C {
+    abstract public function m();
+    public function real() {
+        return 'real';                  // 6
+    }
+}
+class D extends C { public function m() { return 'm'; } }
+
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+
+// An abstract method still gets an op_array: zend_compile_func_decl() calls zend_emit_final_return() unconditionally with CG(zend_lineno) = decl->end_lineno (Zend/zend_compile.c), and with no body end_lineno *is* the declaration line.
+// So C::m is a single ZEND_RETURN attributed to line 4, and it is the innermost op_array carrying the smallest executable lineno >= 4 -- which is where the hook arms.
+// C::m is never invoked (D::m overrides it), so nothing fires.
+// That is the resolver working as specified rather than swallowing the hook: sliding across the function boundary into real()'s body would silently relocate the hook into a different function.
+$a = DDTrace\install_line_hook(__FILE__, 4, $p);
+var_dump($a < 0);
+$o = new D();
+$o->m();
+$o->real();
+echo 'from 4: ', ($log ? implode(',', $log) : '(never fired)'), "\n";
+DDTrace\remove_hook($a);
+
+// Requesting 5 (the real method's signature line) slides into its body at 6, as usual.
+$log = [];
+$b = DDTrace\install_line_hook(__FILE__, 5, $p);
+$o->real();
+echo 'from 5: ', ($log ? implode(',', $log) : '(never fired)'), "\n";
+DDTrace\remove_hook($b);
+
+echo "Done.\n";
+?>
+--EXPECT--
+bool(true)
+from 4: (never fired)
+from 5: 6
+Done.

tests/ext/hook/line_hook_anon_class.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Line hooks resolve into anonymous class methods, including one sharing the declaring line
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+
+function make() {
+    return new class {
+        public function hi($t) {
+            return 'hi:' . $t;      // 9
+        }
+    };
+}
+
+$ids = [];
+foreach ([9] as $ln) { $ids[] = DDTrace\install_line_hook(__FILE__, $ln, $p); }
+
+var_dump(make()->hi('a'));
+var_dump(make()->hi('b'));
+echo implode(',', $log), "\n";
+foreach ($ids as $id) { DDTrace\remove_hook($id); }
+echo "Done.\n";
+?>
+--EXPECT--
+string(4) "hi:a"
+string(4) "hi:b"
+9,9
+Done.

tests/ext/hook/line_hook_anon_class_late_install.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Diagnostic: an anonymous class method, hooked after the class has already been linked
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+
+function make() {
+    return new class {
+        public function hi($t) {
+            return 'hi:' . $t;      // 9
+        }
+    };
+}
+
+// Link the anonymous class first, then install.
+make()->hi('warmup');
+
+$id = DDTrace\install_line_hook(__FILE__, 9, $p);
+var_dump(make()->hi('a'));
+echo 'after link: ', ($log ? implode(',', $log) : '(never fired)'), "\n";
+DDTrace\remove_hook($id);
+echo "Done.\n";
+?>
+--EXPECT--
+string(4) "hi:a"
+after link: 9
+Done.

tests/ext/hook/line_hook_anon_class_linked.phpt

@@ -0,0 +1,56 @@
+--TEST--
+Diagnostic: anonymous class methods, with every class already linked before the hook is installed
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+interface Marker {}
+class Parent1 {}
+trait Tr {}
+
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+
+function bare() {
+    return new class {
+        public function m() { return 'bare'; }          // 12
+    };
+}
+function withParent() {
+    return new class extends Parent1 {
+        public function m() { return 'parent'; }        // 17
+    };
+}
+function withIface() {
+    return new class implements Marker {
+        public function m() { return 'iface'; }         // 22
+    };
+}
+function withTrait() {
+    return new class { use Tr;
+        public function m() { return 'trait'; }         // 27
+    };
+}
+
+// Link every anonymous class before any hook is installed, so resolution is not a timing question.
+foreach (['bare', 'withParent', 'withIface', 'withTrait'] as $fn) { $fn()->m(); }
+
+foreach ([12 => 'bare', 17 => 'withParent', 22 => 'withIface', 27 => 'withTrait'] as $ln => $fn) {
+    $id = DDTrace\install_line_hook(__FILE__, $ln, $p);
+    $log = [];
+    $fn()->m();
+    echo "$fn (line $ln) -> ", ($log ? implode(',', $log) : '(never fired)'), "\n";
+    DDTrace\remove_hook($id);
+}
+
+echo "Done.\n";
+?>
+--EXPECT--
+bare (line 12) -> 12
+withParent (line 17) -> 17
+withIface (line 22) -> 22
+withTrait (line 27) -> 27
+Done.

tests/ext/hook/line_hook_anon_class_misresolve.phpt

@@ -0,0 +1,28 @@
+--TEST--
+A hook aimed at an anonymous class method must not land on an unrelated file-scope line
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+$id = DDTrace\install_line_hook(__FILE__, 8, $p);
+
+$obj = new class {
+    public function hi($t) {
+        return 'hi:' . $t;              // 8 <- wanted
+    }
+};
+
+$marker = 1;                            // 12
+var_dump($obj->hi('a'));
+echo ($log ? implode(',', $log) : '(never fired)'), "\n";
+DDTrace\remove_hook($id);
+echo "Done.\n";
+?>
+--EXPECT--
+string(4) "hi:a"
+8
+Done.

tests/ext/hook/line_hook_arrow_fn.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Arrow functions share their line with the enclosing scope, so both sites arm
+--SKIPIF--
+<?php if (PHP_VERSION_ID < 70400) die('skip requires PHP 7.4'); ?>
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+$log = [];
+$p = function (DDTrace\LineHookData $h) use (&$log) { $log[] = $h->line; };
+
+function build() {
+    return fn($x) => $x * 3;            // 7
+}
+
+$id = DDTrace\install_line_hook(__FILE__, 7, $p);
+
+$f = build();                           // arms fire: build()'s line 7, then the arrow body
+var_dump($f(2));
+var_dump($f(3));
+echo count($log), ': ', implode(',', $log), "\n";
+DDTrace\remove_hook($id);
+echo "Done.\n";
+?>
+--EXPECT--
+int(6)
+int(9)
+3: 7,7,7
+Done.

tests/ext/hook/line_hook_bailout.phpt

@@ -0,0 +1,31 @@
+--TEST--
+An open range still closes when a zend_bailout abandons the frame
+--INI--
+; E_USER_ERROR is deprecated from 8.4, and the notice is not what this test is about. 24575 = E_ALL & ~E_DEPRECATED.
+error_reporting=24575
+fatal_error_backtraces=0
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+function t() {
+    $a = 1;
+    trigger_error('boom', E_USER_ERROR);
+}
+
+// The end line has no opline after it, so this range has no static closing site and depends entirely on the frame-exit guard.
+// E_USER_ERROR is a real zend_bailout, but php_request_shutdown() runs zend_observer_fcall_end_all() as its step 0 -- before zend_deactivate_modules() and so before ddtrace's RSHUTDOWN -- which is what makes the guard fire.
+DDTrace\install_hook('t', null, function () { echo "fn:end\n"; });
+DDTrace\install_line_hook(__FILE__, 4, function () { echo "line:begin\n"; }, 6, function () { echo "line:end\n"; });
+t();
+echo "unreachable\n";
+?>
+--EXPECTF--
+line:begin
+
+Fatal error: boom in %s on line 5
+line:end
+fn:end

tests/ext/hook/line_hook_basename_a.inc

@@ -0,0 +1,5 @@
+<?php
+
+function bn_a() {
+    return 'a';
+}

tests/ext/hook/line_hook_basename_xa.inc

@@ -0,0 +1,5 @@
+<?php
+
+function bn_xa() {
+    return 'xa';
+}

tests/ext/hook/line_hook_basic.phpt

@@ -0,0 +1,45 @@
+--TEST--
+DDTrace\install_line_hook() fires at the requested line and stops on remove_hook()
+--ENV--
+DD_TRACE_GENERATE_ROOT_SPAN=0
+DD_TRACE_LOG_LEVEL=off
+DD_APPSEC_ENABLED=0
+--FILE--
+<?php
+
+function target($n) {
+    $a = $n * 2;
+    $b = $a + 1;
+    return $b;
+}
+
+$hits = 0;
+$id = DDTrace\install_line_hook(__FILE__, 5, function () use (&$hits) { ++$hits; });
+var_dump($id < 0);
+
+var_dump(target(5));
+var_dump(target(7));
+var_dump($hits);
+
+DDTrace\remove_hook($id);
+var_dump(target(9));
+var_dump($hits);
+
+// A range whose end precedes its start is rejected before anything is armed.
+try {
+    DDTrace\install_line_hook(__FILE__, 10, null, 4);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "Done.\n";
+?>
+--EXPECT--
+bool(true)
+int(11)
+int(15)
+int(2)
+int(19)
+int(2)
+Line hook end line 4 is before start line 10
+Done.