Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JIT crash with instrumented generators #2797

Merged
merged 1 commit into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions tests/ext/sandbox/install_hook/trace_generator_jit.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
generator hooking works with JIT
--SKIPIF--
<?php if (!file_exists(ini_get("extension_dir") . "/opcache.so")) die('skip: opcache.so does not exist in extension_dir'); ?>
<?php if (PHP_VERSION_ID < 80000) die('skip: JIT is only on PHP 8'); ?>
--INI--
opcache.enable=1
opcache.enable_cli = 1
opcache.jit_buffer_size=512M
opcache.jit=1255
zend_extension=opcache.so
--FILE--
<?php

function gen() {
yield 1;
yield 2;
return 3;
};

$hooks[] = \DDTrace\hook_function("gen", [
"posthook" => function($args, $retval) {
var_dump($retval);
}
]);

for ($i = 0; $i < 2; ++$i) {
foreach (gen() as $val) {
var_dump($val);
}
}

?>
--EXPECT--
int(1)
int(1)
int(2)
int(2)
int(1)
int(1)
int(2)
int(2)
12 changes: 11 additions & 1 deletion tests/randomized/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ Note that the generated scenarios can be run individually. In the directory `.tm

### Analyzing a core dump generated in CI

Run a script `circleci_core.sh` which will install the used extension, and launch a gdb shell with the core dump:

```
tests/randomized/circleci_core.sh https://app.circleci.com/pipelines/github/DataDog/dd-trace-php/17113/workflows/196b5740-f3ce-4faf-8731-e9a4b15d114a/jobs/4964970/steps
```

The only argument to that script is the URL of the job.

#### Alternative: Manual steps

Run a container with the most recent version of the proper docker image for the specific version of PHP. For example, assuming PHP 8.0:

```
Expand Down Expand Up @@ -172,7 +182,7 @@ Then load it in `gdb`:
gdb --core=/tmp/core php-fpm|httpd|php
```

### Analyzing using ASAN
#### Manual steps using ASAN

In some cases it might be tricky to find the memory corruption, but you can
always use an address sanitizer to try and find problems.
Expand Down
59 changes: 59 additions & 0 deletions tests/randomized/circleci_core.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#!/bin/bash

# e.g. https://app.circleci.com/pipelines/github/DataDog/dd-trace-php/17113/workflows/196b5740-f3ce-4faf-8731-e9a4b15d114a/jobs/4964970/steps
URL=${1}

if ! [[ $URL =~ workflows/([0-9a-f-]+)/jobs/([0-9]+) ]]; then
echo "Invalid URL?"
exit 1
fi

CIRCLE_WORKFLOW_ID=${BASH_REMATCH[1]}
CIRCLE_JOB_ID=${BASH_REMATCH[2]}

TEST_NAME=$(curl -s -X GET "https://circleci.com/api/v2/project/gh/DataDog/dd-trace-php/job/$CIRCLE_JOB_ID" -H "Accept: application/json" | grep -Eo '"name":"randomized[^"]+')
TEST_NAME=${TEST_NAME:8}

echo "Found test run: $TEST_NAME"

COREFILES=($(curl -s -X GET "https://circleci.com/api/v2/project/gh/DataDog/dd-trace-php/$CIRCLE_JOB_ID/artifacts" -H "Accept: application/json" | grep -Eo 'https://[^"]+core'))

if [[ -z $COREFILES ]]; then
echo "Found no core files..."
exit 1
fi

num=1
for file in "${COREFILES[@]}"; do
echo "$((num++)): $(echo "$file" | grep -Eo 'randomized-[^/]+')"
done

while : ; do
echo -n "Select one core file: "
read num
if [[ $num -gt ${#COREFILES[@]} || $num -le 0 ]]; then
echo "Invalid number $num"
else
break
fi
done

COREFILE=${COREFILES[$((num - 1))]}
corefilename=$(echo "$COREFILE" | grep -Eo 'randomized-[^/]+')
container="datadog/dd-trace-ci:php-randomizedtests-$(if [[ $COREFILE == *buster* ]]; then echo buster; else echo centos7; fi)-${corefilename: -2:1}.${corefilename: -1}-2"

if [[ $TEST_NAME == *asan* ]]; then
ARTIFACTS_JOB='package extension zts-debug-asan'
else
ARTIFACTS_JOB='package extension'
fi

PACKAGE_ARCH=$(if [[ $TEST_NAME == *arm* ]]; then echo aarch64; else echo x86_64; fi)

parent_job_id=$(curl -s -X GET "https://circleci.com/api/v2/workflow/$CIRCLE_WORKFLOW_ID/job" -H "Accept: application/json" | grep -Eo '\{[^}]*"'"$ARTIFACTS_JOB"'"[^}]*' | grep -Eo '"job_number":[^,]+' | tail -c +14)
ARTIFACTS_RESULT=$(curl -s -X GET "https://circleci.com/api/v2/project/github/DataDog/dd-trace-php/$parent_job_id/artifacts" -H "Accept: application/json")
artifact_url=$(echo "$ARTIFACTS_RESULT" | grep -Eo '\{[^}]*"dd-library-php-[^"]+'"${PACKAGE_ARCH}"'-linux-gnu.tar.gz"[^}]*' | grep -Eo '"url":"[^"]+' | tail -c +8)
setup_url=$(echo "$ARTIFACTS_RESULT" | grep -Eo '\{[^}]*"datadog-setup.php"[^}]*' | grep -Eo '"url":"[^"]+' | tail -c +8)

set -x
docker run --platform=linux/$(if [[ $TEST_NAME == *arm* ]]; then echo arm64; else echo amd64; fi) --rm -ti "$container" bash -c "curl -Lo /tmp/datadog-setup.php '$setup_url'; curl -Lo /tmp/${artifact_url##*/} '$artifact_url'; curl -Lo /tmp/core '$COREFILE'; php /tmp/datadog-setup.php --php-bin all --file /tmp/${artifact_url##*/} --enable-profiling; $(if [[ $TEST_NAME == *asan* ]]; then echo "switch-php debug-zts-asan; "; fi)exec bash --rcfile <(echo 'gdb \$(file /tmp/core | grep -Po "'from\\s\\x27\\K[^\\x27:]+'") --core=/tmp/core -ix /usr/local/src/php/.gdbinit')"
3 changes: 3 additions & 0 deletions zend_abstract_interface/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ add_subdirectory(env)
add_subdirectory(exceptions)
add_subdirectory(config)
add_subdirectory(json)
if(PhpConfig_VERNUM GREATER_EQUAL "80000")
add_subdirectory(jit_utils)
endif()
add_subdirectory(symbols)
add_subdirectory(hook)
add_subdirectory(interceptor)
Expand Down
3 changes: 3 additions & 0 deletions zend_abstract_interface/hook/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ target_include_directories(
target_compile_features(zai_hook PUBLIC c_std_99)

target_link_libraries(zai_hook PUBLIC Tea::Php Zai::Symbols)
if(PhpConfig_VERNUM GREATER_EQUAL "80000")
target_link_libraries(zai_hook PUBLIC Zai::JitUtils)
endif()

set_target_properties(zai_hook PROPERTIES EXPORT_NAME Hook
VERSION ${PROJECT_VERSION})
Expand Down
8 changes: 8 additions & 0 deletions zend_abstract_interface/hook/hook.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "../tsrmls_cache.h"
#include <hook/hook.h>
#include <hook/table.h>
#include <jit_utils/jit_blacklist.h>


/* {{{ */
Expand Down Expand Up @@ -399,6 +400,13 @@ static void zai_hook_resolve_hooks_entry(zai_hooks_entry *hooks, zend_function *
#endif
}
hooks->is_generator = (resolved->common.fn_flags & ZEND_ACC_GENERATOR) != 0;
if (hooks->is_generator) {
#if ZAI_JIT_BLACKLIST_ACTIVE
// Generator observers may replace the EX(opline) by a custom op. The tracing JIT will not like this. Blacklist them.
// Note that we cannot defer the blacklisting until the observer hook is invoked as that one will not be called before ZEND_GENERATOR_CREATE.
zai_jit_blacklist_function_inlining(&resolved->op_array);
#endif
}
#endif
if ((resolved->common.fn_flags & ZEND_ACC_CLOSURE) == 0)
{
Expand Down
3 changes: 3 additions & 0 deletions zend_abstract_interface/hook/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ add_executable(hooks
)

target_link_libraries(hooks PUBLIC catch2_main Tea::Tea Zai::Symbols Zai::Hook)
if(PhpConfig_VERNUM GREATER_EQUAL "80000")
target_link_libraries(hooks PUBLIC Zai::JitUtils)
endif()

file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/stubs
DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
Expand Down
22 changes: 22 additions & 0 deletions zend_abstract_interface/jit_utils/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
add_library(zai_jit_utils jit_blacklist.c)

target_include_directories(
zai_jit_utils PUBLIC $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}>
$<INSTALL_INTERFACE:include>)

target_compile_features(zai_jit_utils PUBLIC c_std_99)

target_link_libraries(zai_jit_utils PUBLIC Tea::Php dl)

set_target_properties(zai_jit_utils PROPERTIES EXPORT_NAME JitUtils
VERSION ${PROJECT_VERSION})

add_library(Zai::JitUtils ALIAS zai_jit_utils)

install(
FILES ${CMAKE_CURRENT_SOURCE_DIR}/jit_blacklist.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/jit_utils/)

target_link_libraries(zai_zend_abstract_interface INTERFACE zai_jit_utils)

install(TARGETS zai_jit_utils EXPORT ZendAbstractInterfaceTargets)
Loading