Simplify sandboxed function calling

Signed-off-by: Bob Weinand <[email protected]>

Author: bwoebi

config.m4

@@ -229,8 +229,7 @@ if test "$PHP_DDTRACE" != "no"; then
     zend_abstract_interface/headers/headers.c \
     zend_abstract_interface/hook/hook.c \
     zend_abstract_interface/json/json.c \
-    zend_abstract_interface/symbols/lookup.c \
-    zend_abstract_interface/symbols/call.c \
+    zend_abstract_interface/sandbox/call.c \
     zend_abstract_interface/uri_normalization/uri_normalization.c \
     zend_abstract_interface/zai_string/string.c \
   "
@@ -294,7 +293,6 @@ EOT
 
   PHP_ADD_INCLUDE([$ext_srcdir/zend_abstract_interface])
   PHP_ADD_BUILD_DIR([$ext_builddir/zend_abstract_interface])
-  PHP_ADD_BUILD_DIR([$ext_builddir/zend_abstract_interface/symbols])
   PHP_ADD_BUILD_DIR([$ext_builddir/zend_abstract_interface/config])
   PHP_ADD_BUILD_DIR([$ext_builddir/zend_abstract_interface/env])
   PHP_ADD_BUILD_DIR([$ext_builddir/zend_abstract_interface/exceptions])

config.w32

@@ -104,8 +104,8 @@ if (PHP_DDTRACE != 'no') {
 	ADD_SOURCES(zai_dirname + "/headers", "headers.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/hook", "hook.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 + "/symbols", "call.c lookup.c", "ddtrace");
 	ADD_SOURCES(zai_dirname + "/uri_normalization", "uri_normalization.c", "ddtrace");
 	if (version < 800) {
 		ADD_SOURCES(zai_dirname + "/sandbox/php7", "sandbox.c", "ddtrace");

ext/compatibility.h

@@ -63,6 +63,29 @@
 #define ZVAL_VARARG_PARAM(list, arg_num) (&(((zval *)list)[arg_num]))
 #define IS_TRUE_P(x) (Z_TYPE_P(x) == IS_TRUE)
 
+static zend_always_inline zend_fcall_info dd_fcall_info(int argc, zval *args, zval *rv) {
+    return (zend_fcall_info){
+            .param_count = argc,
+            .params = args,
+            .retval = rv,
+            .size = sizeof(zend_fcall_info),
+#if PHP_VERSION_ID < 80000
+            .initialized = 1,
+#endif
+#if PHP_VERSION_ID >= 80000
+            .named_params = NULL,
+#endif
+    };
+}
+
+static zend_always_inline void dd_get_closure_to_fcc(zval *closure, zend_fcall_info_cache *fcc) {
+#if PHP_VERSION_ID < 80000
+    Z_OBJ_HANDLER_P(closure, get_closure)(closure, &fcc->called_scope, &fcc->function_handler, &fcc->object, 1);
+#else
+    Z_OBJ_HANDLER_P(closure, get_closure)(Z_OBJ_P(closure), &fcc->called_scope, &fcc->function_handler, &fcc->object, 1);
+#endif
+}
+
 static inline zval *ddtrace_assign_variable(zval *variable_ptr, zval *value) {
 #if PHP_VERSION_ID < 70400
     return zend_assign_to_variable(variable_ptr, value, IS_TMP_VAR);
@@ -424,6 +447,15 @@ static zend_always_inline void zend_array_release(zend_array *array)
     }
 }
 
+static inline void *zend_hash_find_ptr_lc(const HashTable *ht, zend_string *key) {
+    void *result;
+    zend_string *lc_key = zend_string_tolower(key);
+    result = zend_hash_find_ptr(ht, lc_key);
+    zend_string_release(lc_key);
+    return result;
+}
+
+
 #define ZEND_UNREACHABLE() ZEND_ASSUME(0)
 
 #define ZEND_ARG_SEND_MODE(arg_info) (arg_info)->pass_by_reference

ext/exception_serialize.c

@@ -411,13 +411,12 @@ static void ddtrace_collect_exception_debug_data(zend_object *exception, zend_st
         ddog_CharSlice class_slice = DDOG_CHARSLICE_C("");
         zval *class_name = zend_hash_find(Z_ARR_P(frame), ZSTR_KNOWN(ZEND_STR_CLASS));
         if (class_name && Z_TYPE_P(class_name) == IS_STRING) {
-            ce = zai_symbol_lookup_class_global(zai_str_from_zstr(Z_STR_P(class_name)));
+            ce = zend_hash_find_ptr_lc(EG(class_table), Z_STR_P(class_name));
             class_slice = dd_zend_string_to_CharSlice(Z_STR_P(class_name));
         }
         zval *func_name = zend_hash_find(Z_ARR_P(frame), ZSTR_KNOWN(ZEND_STR_FUNCTION));
         if (func_name && Z_TYPE_P(func_name) == IS_STRING) {
-            zai_str wtf = zai_str_from_zstr(Z_STR_P(func_name));
-            func = zai_symbol_lookup_function(ce ? ZAI_SYMBOL_SCOPE_CLASS : ZAI_SYMBOL_SCOPE_GLOBAL, ce, &wtf);
+            func = zend_hash_find_ptr_lc(ce ? &ce->function_table : EG(function_table), Z_STR_P(func_name));
             func_slice = dd_zend_string_to_CharSlice(Z_STR_P(func_name));
         }
 

ext/hook/uhook.c

@@ -37,8 +37,8 @@ typedef struct {
 } dd_closure_list;
 
 typedef struct {
-    zend_object *begin;
-    zend_object *end;
+    dd_uhook_callback begin;
+    dd_uhook_callback end;
     bool running;
     zend_long id;
 
@@ -75,6 +75,33 @@ typedef struct {
     dd_hook_data *hook_data;
 } dd_uhook_dynamic;
 
+void dd_uhook_callback_apply_scope(dd_uhook_callback *cb, zend_class_entry *scope) {
+    zend_function *func = (zend_function *)zend_get_closure_method_def(cb->closure);
+    if (!cb->fcc.function_handler) {
+        cb->is_static = func->common.fn_flags & ZEND_ACC_STATIC;
+        if (cb->is_static) {
+            goto end;
+        }
+    }
+    if (GC_REFCOUNT(cb->closure) == 1) {
+        func->common.scope = scope;
+        if (!(func->common.fn_flags & ZEND_ACC_HEAP_RT_CACHE)) {
+            func->op_array.fn_flags |= ZEND_ACC_HEAP_RT_CACHE;
+            ZEND_MAP_PTR_INIT(func->op_array.run_time_cache, emalloc(func->op_array.cache_size));
+        }
+    } else {
+        zval zv;
+        zend_create_closure(&zv, func, scope, scope, NULL);
+        OBJ_RELEASE(cb->closure);
+        cb->closure = Z_OBJ(zv);
+        func = (zend_function *)zend_get_closure_method_def(cb->closure);
+    }
+    memset(ZEND_MAP_PTR_GET(func->op_array.run_time_cache), 0, func->op_array.cache_size);
+end:
+    cb->fcc.function_handler = func;
+    cb->fcc.called_scope = func->common.scope;
+}
+
 static zend_object *dd_hook_data_create(zend_class_entry *class_type) {
     dd_hook_data *hook_data = ecalloc(1, sizeof(*hook_data));
     zend_object_std_init(&hook_data->std, class_type);
@@ -192,20 +219,18 @@ void dd_uhook_report_sandbox_error(zend_execute_data *execute_data, zend_object
     })
 }
 
-static bool dd_uhook_call_hook(zend_execute_data *execute_data, zend_object *closure, dd_hook_data *hook_data) {
-    zval closure_zv, hook_data_zv;
-    ZVAL_OBJ(&closure_zv, closure);
+static bool dd_uhook_call_hook(zend_execute_data *execute_data, dd_uhook_callback *callback, dd_hook_data *hook_data) {
+    zval hook_data_zv;
     ZVAL_OBJ(&hook_data_zv, &hook_data->std);
 
-    bool has_this = getThis() != NULL;
     zval rv;
     zai_sandbox sandbox;
     zai_sandbox_open(&sandbox);
-    bool success = zai_symbol_call(has_this ? ZAI_SYMBOL_SCOPE_OBJECT : ZAI_SYMBOL_SCOPE_GLOBAL, has_this ? &EX(This) : NULL,
-                                   ZAI_SYMBOL_FUNCTION_CLOSURE, &closure_zv,
-                                   &rv, 1 | ZAI_SYMBOL_SANDBOX, &sandbox, &hook_data_zv);
+    dd_uhook_callback_ensure_scope(callback, execute_data);
+    zend_fcall_info fci = dd_fcall_info(1, &hook_data_zv, &rv);
+    bool success = zai_sandbox_call(&sandbox, &fci, &callback->fcc);
     if (!success || PG(last_error_message)) {
-        dd_uhook_report_sandbox_error(execute_data, closure);
+        dd_uhook_report_sandbox_error(execute_data, callback->closure);
     }
     zai_sandbox_close(&sandbox);
     zval_ptr_dtor(&rv);
@@ -298,7 +323,7 @@ static bool dd_uhook_begin(zend_ulong invocation, zend_execute_data *execute_dat
         ZVAL_ARR(&dyn->hook_data->property_args, dd_uhook_collect_args(execute_data));
     }
 
-    if (def->begin && !def->running) {
+    if (def->begin.closure && !def->running) {
         dyn->hook_data->execute_data = execute_data;
         // We support it for PHP 8 for now, given we need this for PHP 8.1+ right now.
         // Bringing it to PHP 7.1-7.4 is possible, but not done yet.
@@ -310,10 +335,10 @@ static bool dd_uhook_begin(zend_ulong invocation, zend_execute_data *execute_dat
         }
 #endif
 
-        LOGEV(HOOK_TRACE, dd_uhook_log_invocation(log, execute_data, "begin", def->begin););
+        LOGEV(HOOK_TRACE, dd_uhook_log_invocation(log, execute_data, "begin", def->begin.closure););
 
         def->running = true;
-        dd_uhook_call_hook(execute_data, def->begin, dyn->hook_data);
+        dd_uhook_call_hook(execute_data, &def->begin, dyn->hook_data);
         def->running = false;
         dyn->hook_data->retval_ptr = NULL;
     }
@@ -395,7 +420,7 @@ static void dd_uhook_end(zend_ulong invocation, zend_execute_data *execute_data,
 
     bool keep_span = true;
 
-    if (def->end && !def->running && get_DD_TRACE_ENABLED()) {
+    if (def->end.closure && !def->running && get_DD_TRACE_ENABLED()) {
         zval tmp;
 
         /* If the profiler doesn't handle a potential pending interrupt before
@@ -432,11 +457,11 @@ static void dd_uhook_end(zend_ulong invocation, zend_execute_data *execute_data,
         }
         zval_ptr_dtor(&tmp);
 
-        LOGEV(HOOK_TRACE, dd_uhook_log_invocation(log, execute_data, "end", def->end););
+        LOGEV(HOOK_TRACE, dd_uhook_log_invocation(log, execute_data, "end", def->end.closure););
 
         def->running = true;
         dyn->hook_data->retval_ptr = retval;
-        keep_span = dd_uhook_call_hook(execute_data, def->end, dyn->hook_data);
+        keep_span = dd_uhook_call_hook(execute_data, &def->end, dyn->hook_data);
         dyn->hook_data->retval_ptr = NULL;
         def->running = false;
     }
@@ -485,11 +510,11 @@ static void dd_uhook_end(zend_ulong invocation, zend_execute_data *execute_data,
 
 static void dd_uhook_dtor(void *data) {
     dd_uhook_def *def = data;
-    if (def->begin) {
-        OBJ_RELEASE(def->begin);
+    if (def->begin.closure) {
+        OBJ_RELEASE(def->begin.closure);
     }
-    if (def->end) {
-        OBJ_RELEASE(def->end);
+    if (def->end.closure) {
+        OBJ_RELEASE(def->end.closure);
     }
     if (def->function) {
         zend_string_release(def->function);
@@ -581,13 +606,15 @@ PHP_FUNCTION(DDTrace_install_hook) {
     dd_uhook_def *def = emalloc(sizeof(*def));
     def->closure = NULL;
     def->running = false;
-    def->begin = begin ? Z_OBJ_P(begin) : NULL;
-    if (def->begin) {
-        GC_ADDREF(def->begin);
+    def->begin.fcc.function_handler = NULL;
+    def->begin.closure = begin ? Z_OBJ_P(begin) : NULL;
+    if (def->begin.closure) {
+        GC_ADDREF(def->begin.closure);
     }
-    def->end = end ? Z_OBJ_P(end) : NULL;
-    if (def->end) {
-        GC_ADDREF(def->end);
+    def->end.fcc.function_handler = NULL;
+    def->end.closure = end ? Z_OBJ_P(end) : NULL;
+    if (def->end.closure) {
+        GC_ADDREF(def->end.closure);
     }
     def->id = -1;
 

ext/hook/uhook.h

@@ -4,6 +4,12 @@
 #include <php.h>
 #include <sandbox/sandbox.h>
 
+typedef struct {
+    zend_object *closure;
+    zend_fcall_info_cache fcc;
+    bool is_static;
+} dd_uhook_callback;
+
 HashTable *dd_uhook_collect_args(zend_execute_data *execute_data);
 void dd_uhook_report_sandbox_error(zend_execute_data *execute_data, zend_object *closure);
 void dd_uhook_log_invocation(void (*log)(const char *, ...), zend_execute_data *execute_data, const char *type, zend_object *closure);
@@ -14,6 +20,27 @@ void zai_uhook_rshutdown();
 void zai_uhook_minit(int module_number);
 void zai_uhook_mshutdown();
 
+void dd_uhook_callback_apply_scope(dd_uhook_callback *cb, zend_class_entry *scope);
+static inline void dd_uhook_callback_ensure_scope(dd_uhook_callback *cb, zend_execute_data *execute_data) {
+    zend_class_entry *scope;
+    if (!cb->fcc.function_handler) {
+        scope = zend_get_called_scope(execute_data);
+        goto apply_scope;
+    } else if (!cb->is_static) {
+        bool has_this;
+        scope = zend_get_called_scope(execute_data);
+        if (scope != cb->fcc.called_scope) {
+apply_scope:
+            dd_uhook_callback_apply_scope(cb, scope);
+            has_this = !cb->is_static && getThis() != NULL;
+        } else {
+            has_this = getThis() != NULL;
+        }
+
+        cb->fcc.object = has_this ? Z_OBJ(EX(This)) : NULL;
+    }
+}
+
 PHP_FUNCTION(DDTrace_trace_function);
 PHP_FUNCTION(DDTrace_trace_method);
 PHP_FUNCTION(DDTrace_hook_function);