From 05aef8968d59454e3a733a9ca0862b87276ca9f9 Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Wed, 21 Oct 2020 19:08:36 +0200
Subject: [PATCH 1/4] Fix #79177: FFI doesn't handle well PHP exceptions within
callback
When an FII callback function throws an unhandled exception, it makes
not much sense to return value to the C code. Instead, we declare that
the return value is undefined in this case, and let userland deal with
that.
---
ext/ffi/ffi.c | 4 ++++
ext/ffi/tests/bug79177.phpt | 40 +++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
create mode 100644 ext/ffi/tests/bug79177.phpt
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index f350f91d8a3c..a99b2ed8873f 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -886,6 +886,10 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
}
free_alloca(fci.params, use_heap);
+ if (EG(exception)) {
+ return;
+ }
+
ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
if (ret_type->kind != ZEND_FFI_TYPE_VOID) {
zend_ffi_zval_to_cdata(ret, ret_type, &retval);
diff --git a/ext/ffi/tests/bug79177.phpt b/ext/ffi/tests/bug79177.phpt
new file mode 100644
index 000000000000..ae2f315b82f6
--- /dev/null
+++ b/ext/ffi/tests/bug79177.phpt
@@ -0,0 +1,40 @@
+--TEST--
+Bug #79177 (FFI doesn't handle well PHP exceptions within callback)
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+require_once('utils.inc');
+try {
+ ffi_cdef("extern void *zend_printf;", ffi_get_php_dll_name());
+} catch (Throwable $e) {
+ die('skip PHP symbols not available');
+}
+?>
+--FILE--
+<?php
+require_once('utils.inc');
+$php = ffi_cdef("
+typedef char (*zend_write_func_t)(const char *str, size_t str_length);
+extern zend_write_func_t zend_write;
+", ffi_get_php_dll_name());
+
+echo "Before", PHP_EOL;
+
+$originalHandler = clone $php->zend_write;
+$php->zend_write = function($str, $len): string {
+ throw new \RuntimeException('Not allowed');
+};
+try {
+ echo "After", PHP_EOL;
+} catch (\Throwable $exception) {
+ // Do not output anything here, as handler is overridden
+} finally {
+ $php->zend_write = $originalHandler;
+}
+if (isset($exception)) {
+ echo $exception->getMessage(), PHP_EOL;
+}
+?>
+--EXPECT--
+Before
+Not allowed
From 327e6c9d37dbb36c4a0097fcaabc9d58ecfe9323 Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Mon, 26 Oct 2020 15:30:31 +0100
Subject: [PATCH 2/4] Error on unhandled exceptions in FFI callbacks
This has been discussed and agreed upon in a previous PR[1].
[1] <https://github.com/php/php-src/pull/5120>
---
ext/ffi/ffi.c | 3 ++-
ext/ffi/tests/bug79177.phpt | 17 +++++++++++++----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index a99b2ed8873f..51f62b9656c1 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -887,7 +887,8 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
free_alloca(fci.params, use_heap);
if (EG(exception)) {
- return;
+ zend_throw_error(zend_ffi_exception_ce, "Uncaught %s in PHP FFI callback", ZSTR_VAL(EG(exception)->ce->name));
+ zend_exception_error(EG(exception), E_ERROR);
}
ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
diff --git a/ext/ffi/tests/bug79177.phpt b/ext/ffi/tests/bug79177.phpt
index ae2f315b82f6..ed219d792eec 100644
--- a/ext/ffi/tests/bug79177.phpt
+++ b/ext/ffi/tests/bug79177.phpt
@@ -18,14 +18,14 @@ typedef char (*zend_write_func_t)(const char *str, size_t str_length);
extern zend_write_func_t zend_write;
", ffi_get_php_dll_name());
-echo "Before", PHP_EOL;
+echo "Before\n";
$originalHandler = clone $php->zend_write;
$php->zend_write = function($str, $len): string {
throw new \RuntimeException('Not allowed');
};
try {
- echo "After", PHP_EOL;
+ echo "After\n";
} catch (\Throwable $exception) {
// Do not output anything here, as handler is overridden
} finally {
@@ -35,6 +35,15 @@ if (isset($exception)) {
echo $exception->getMessage(), PHP_EOL;
}
?>
---EXPECT--
+--EXPECTF--
Before
-Not allowed
+
+Fatal error: Uncaught RuntimeException: Not allowed in %s:%d
+Stack trace:
+#0 %s(15): {closure}('After\n', 6)
+#1 {main}
+
+Next FFI\Exception: Uncaught RuntimeException in PHP FFI callback in %s:%d
+Stack trace:
+#0 {main}
+ thrown in %s on line %d
From 6503228b0a1dd95e0ebc64b2d814456a3dc5752b Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Wed, 28 Oct 2020 09:58:26 +0100
Subject: [PATCH 3/4] Improve exception message
---
ext/ffi/ffi.c | 2 +-
ext/ffi/tests/bug79177.phpt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 51f62b9656c1..d251f12bbe3c 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -887,7 +887,7 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
free_alloca(fci.params, use_heap);
if (EG(exception)) {
- zend_throw_error(zend_ffi_exception_ce, "Uncaught %s in PHP FFI callback", ZSTR_VAL(EG(exception)->ce->name));
+ zend_throw_error(zend_ffi_exception_ce, "Throwing from FFI callbacks is not allowed");
zend_exception_error(EG(exception), E_ERROR);
}
diff --git a/ext/ffi/tests/bug79177.phpt b/ext/ffi/tests/bug79177.phpt
index ed219d792eec..92d856c8fab7 100644
--- a/ext/ffi/tests/bug79177.phpt
+++ b/ext/ffi/tests/bug79177.phpt
@@ -43,7 +43,7 @@ Stack trace:
#0 %s(15): {closure}('After\n', 6)
#1 {main}
-Next FFI\Exception: Uncaught RuntimeException in PHP FFI callback in %s:%d
+Next FFI\Exception: Throwing from FFI callbacks is not allowed in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
From d2959ffa58e5a52395ae305d9a7b8f238382ee58 Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Wed, 28 Oct 2020 12:01:28 +0100
Subject: [PATCH 4/4] Trigger E_ERROR instead of exception
---
ext/ffi/ffi.c | 3 +--
ext/ffi/tests/bug79177.phpt | 10 ++++------
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index d251f12bbe3c..d119ea050cbf 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -887,8 +887,7 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
free_alloca(fci.params, use_heap);
if (EG(exception)) {
- zend_throw_error(zend_ffi_exception_ce, "Throwing from FFI callbacks is not allowed");
- zend_exception_error(EG(exception), E_ERROR);
+ zend_error(E_ERROR, "Throwing from FFI callbacks is not allowed");
}
ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
diff --git a/ext/ffi/tests/bug79177.phpt b/ext/ffi/tests/bug79177.phpt
index 92d856c8fab7..d764437b2d50 100644
--- a/ext/ffi/tests/bug79177.phpt
+++ b/ext/ffi/tests/bug79177.phpt
@@ -38,12 +38,10 @@ if (isset($exception)) {
--EXPECTF--
Before
-Fatal error: Uncaught RuntimeException: Not allowed in %s:%d
+Warning: Uncaught RuntimeException: Not allowed in %s:%d
Stack trace:
-#0 %s(15): {closure}('After\n', 6)
+#0 %s(%d): {closure}('After\n', 6)
#1 {main}
-
-Next FFI\Exception: Throwing from FFI callbacks is not allowed in %s:%d
-Stack trace:
-#0 {main}
thrown in %s on line %d
+
+Fatal error: Throwing from FFI callbacks is not allowed in %s on line %d