Merge pull request #78 from DataDog/fix/passback_of_overriding_method…

…_result Test passback of return value and fix it working for PHP 5.6
DataDog · Oct 22, 2018 · 5145b5f · 5145b5f
2 parents 1d6d58d + 1829e96
commit 5145b5f
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 9 deletions.
diff --git a/src/ext/dispatch.c b/src/ext/dispatch.c
@@ -68,15 +68,17 @@ typedef struct _zend_closure {
     HashTable *debug_info;
 } zend_closure;
 
-static void execute_fcall(ddtrace_dispatch_t *dispatch, zend_execute_data *execute_data, zval *return_value TSRMLS_DC) {
+static void execute_fcall(ddtrace_dispatch_t *dispatch, zend_execute_data *execute_data,
+                          zval **return_value_ptr TSRMLS_DC) {
     zend_fcall_info fci;
     zend_fcall_info_cache fcc;
     char *error = NULL;
-    zval closure, rv;
+    zval closure, rv, *rv_ptr;
     INIT_ZVAL(closure);
     INIT_ZVAL(rv);
+    rv_ptr = &rv;
 
-    zval *result = return_value ? return_value : &rv;
+    zval **result_ptr = return_value_ptr ? return_value_ptr : &rv_ptr;
     zval *this = NULL;
 
     zend_function *func;
@@ -115,10 +117,10 @@ static void execute_fcall(ddtrace_dispatch_t *dispatch, zend_execute_data *execu
         goto _exit_cleanup;
     }
 
-    ddtrace_setup_fcall(execute_data, &fci, &result TSRMLS_CC);
+    ddtrace_setup_fcall(execute_data, &fci, result_ptr TSRMLS_CC);
 
     if (zend_call_function(&fci, &fcc TSRMLS_CC) == SUCCESS) {
-        if (!return_value) {
+        if (!return_value_ptr) {
             zval_dtor(&rv);
         }
     }
@@ -196,11 +198,14 @@ static zend_always_inline zend_bool wrap_and_run(zend_execute_data *execute_data
         dispatch->flags ^= BUSY_FLAG;  // guard against recursion, catching only topmost execution
 
 #if PHP_VERSION_ID < 70000
-        zval *return_value = (RETURN_VALUE_USED(opline) ? EX_TMP_VAR(execute_data, opline->result.var)->var.ptr : &rv);
-        execute_fcall(dispatch, execute_data, return_value TSRMLS_CC);
+        zval *return_value = NULL;
+        execute_fcall(dispatch, execute_data, &return_value TSRMLS_CC);
+        if (return_value != NULL) {
+            EX_TMP_VAR(execute_data, opline->result.var)->var.ptr = return_value;
+        }
 #else
         zval *return_value = (RETURN_VALUE_USED(opline) ? EX_VAR(EX(opline)->result.var) : &rv);
-        execute_fcall(dispatch, EX(call), return_value TSRMLS_CC);
+        execute_fcall(dispatch, EX(call), &return_value TSRMLS_CC);
 #endif
 
         dispatch->flags ^= BUSY_FLAG;

diff --git a/src/ext/dispatch_compat_php5.c b/src/ext/dispatch_compat_php5.c
@@ -85,7 +85,7 @@ zend_function *ddtrace_function_get(const HashTable *table, zval *name) {
     zend_hash_find(table, key, Z_STRLEN_P(name) + 1, (void **)&fptr);
 
     DD_PRINTF("Looking for key %s (length: %d, h: 0x%lX) in table", key, Z_STRLEN_P(name),
-              zend_inline_hash_func(key, key_len));
+              zend_inline_hash_func(key, Z_STRLEN_P(name) + 1));
     DD_PRINT_HASH(table);
     DD_PRINTF("Found: %s", fptr != NULL ? "true" : "false");
 

diff --git a/tests/ext/return_value_passed.phpt b/tests/ext/return_value_passed.phpt
@@ -0,0 +1,20 @@
+--TEST--
+Returs value from both original and overriding methods
+--FILE--
+<?php
+class Test {
+    public function method(){
+        return "original";
+    }
+}
+
+$no = 1;
+dd_trace("Test", "method", function() use ($no){
+    return $this->method() . "-override " . $no . PHP_EOL;
+});
+
+echo (new Test())->method();
+
+?>
+--EXPECT--
+original-override 1