From 17a294cec4bb99d37ed01b99787fad483326792f Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Wed, 26 May 2021 08:56:41 +1200 Subject: [PATCH] Replace remaining PHP errors with PHP exceptions `SWIG_ErrorCode()`, `SWIG_ErrorMsg()`, `SWIG_FAIL()` and `goto thrown;` are no longer supported (these are really all internal implementation details and none are documented aside from brief mentions in CHANGES for the first three). I wasn't able to find any uses at least in FOSS code via code search tools. If you are using these: Use `SWIG_PHP_Error(code,msg);` instead of `SWIG_ErrorCode(code); SWIG_ErrorMsg(msg);` (which will throw a PHP exception in SWIG >= 4.1 and do the same as the individual calls in older SWIG). `SWIG_FAIL();` and `goto thrown;` can typically be replaced with `SWIG_fail;`. This will probably also work with older SWIG, but please test with your wrappers if this is important to you. Fixes #2014 --- CHANGES.current | 30 +++++++++++++++++++--- Lib/exception.i | 2 +- Lib/php/director.swg | 4 +-- Lib/php/php.swg | 6 ++--- Lib/php/phprun.swg | 7 ++--- Lib/php/typemaps.i | 2 +- Source/Modules/php.cxx | 58 +++++------------------------------------- 7 files changed, 41 insertions(+), 68 deletions(-) diff --git a/CHANGES.current b/CHANGES.current index 388df8698..9ab912b15 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -8,11 +8,33 @@ Version 4.1.0 (in progress) =========================== 2021-05-04: olly - [PHP] #2014 Throw exceptions instead of using errors + [PHP] #2014 Throw PHP exceptions instead of using PHP errors - Parameter type errors and some other cases in SWIG-generated wrappers - now throw a PHP exception, which is how PHP's native parameter handling - deals with similar situations. + PHP exceptions can be caught and handled if desired, but if they + aren't caught then PHP exits in much the same way as it does for a + PHP error. + + In particular this means parameter type errors and some other cases + in SWIG-generated wrappers now throw a PHP exception, which matches + how PHP's native parameter handling deals with similar situations. + + `SWIG_ErrorCode()`, `SWIG_ErrorMsg()`, `SWIG_FAIL()` and `goto thrown;` + are no longer supported (these are really all internal implementation + details and none are documented aside from brief mentions in CHANGES + for the first three). I wasn't able to find any uses in user interface + files at least in FOSS code via code search tools. + + If you are using these: + + Use `SWIG_PHP_Error(code,msg);` instead of `SWIG_ErrorCode(code); + SWIG_ErrorMsg(msg);` (which will throw a PHP exception in SWIG >= 4.1 + and do the same as the individual calls in older SWIG). + + `SWIG_FAIL();` and `goto thrown;` can typically be replaced with + `SWIG_fail;`. This will probably also work with older SWIG, but + please test with your wrappers if this is important to you. + + *** POTENTIAL INCOMPATIBILITY *** 2021-05-17: adr26 [Python] #1985 Fix memory leaks: diff --git a/Lib/exception.i b/Lib/exception.i index 020ee1150..7508b409b 100644 --- a/Lib/exception.i +++ b/Lib/exception.i @@ -25,7 +25,7 @@ code == SWIG_DivisionByZero ? zend_ce_division_by_zero_error : \ code == SWIG_SyntaxError ? zend_ce_parse_error : \ code == SWIG_OverflowError ? zend_ce_arithmetic_error : \ - NULL, msg, code); goto thrown; } while (0) + NULL, msg, code); SWIG_fail; } while (0) %} #endif diff --git a/Lib/php/director.swg b/Lib/php/director.swg index ead731a48..68be6a3ba 100644 --- a/Lib/php/director.swg +++ b/Lib/php/director.swg @@ -137,8 +137,8 @@ namespace Swig { swig_msg += " "; swig_msg += msg; } - SWIG_ErrorCode() = code; - SWIG_ErrorMsg() = swig_msg.c_str(); + // Don't replace an already active PHP exception. + if (!EG(exception)) zend_throw_exception(NULL, swig_msg.c_str(), code); } virtual ~DirectorException() throw() { diff --git a/Lib/php/php.swg b/Lib/php/php.swg index 3b579b9fc..6e4ee2d2f 100644 --- a/Lib/php/php.swg +++ b/Lib/php/php.swg @@ -96,7 +96,7 @@ %{ if (SWIG_ConvertPtr($input, (void **) &tmp, $&1_descriptor, 0) < 0 || tmp == NULL) { zend_type_error("Expected $&1_descriptor for argument $argnum of $symname"); - goto thrown; + SWIG_fail; } $result = *tmp; %} @@ -115,7 +115,7 @@ %{ if (SWIG_ConvertPtrAndOwn($input, (void **)&$result, $1_descriptor, SWIG_POINTER_DISOWN, &own) < 0) { zend_type_error("Expected $1_descriptor for argument $argnum of $symname"); - goto thrown; + SWIG_fail; } swig_acquire_ownership_obj((void*)$result, own); %} @@ -134,7 +134,7 @@ %{ if (SWIG_ConvertPtr($input, (void **) &tmp, $1_descriptor, 0) < 0 || tmp == NULL) { zend_type_error("Expected $1_descriptor for argument $argnum of $symname"); - goto thrown; + SWIG_fail; } $result = tmp; %} diff --git a/Lib/php/phprun.swg b/Lib/php/phprun.swg index 880c98f41..a3569a783 100644 --- a/Lib/php/phprun.swg +++ b/Lib/php/phprun.swg @@ -58,15 +58,12 @@ static zend_always_inline void *zend_object_alloc(size_t obj_size, zend_class_en #define SWIG_fail goto fail -// If there's an active PHP exception, just return so it can propagate. -#define SWIG_FAIL() do { if (!EG(exception)) zend_error_noreturn(SWIG_ErrorCode(), "%s", SWIG_ErrorMsg()); goto thrown; } while (0) - static const char *default_error_msg = "Unknown error occurred"; static int default_error_code = E_ERROR; #define SWIG_PHP_Arg_Error_Msg(argnum,extramsg) "Error in argument " #argnum " "#extramsg -#define SWIG_PHP_Error(code,msg) do { SWIG_ErrorCode() = code; SWIG_ErrorMsg() = msg; SWIG_fail; } while (0) +#define SWIG_PHP_Error(code,msg) do { zend_throw_exception(NULL, msg, code); SWIG_fail; } while (0) #define SWIG_contract_assert(expr,msg) \ do { if (!(expr)) zend_printf("Contract Assert Failed %s\n", msg); } while (0) @@ -102,7 +99,7 @@ SWIG_SetPointerZval(zval *z, void *ptr, swig_type_info *type, int newobject) { } if (!type->clientdata) { - zend_error(E_ERROR, "Type: %s not registered with zend", type->name); + zend_type_error("Type: %s not registered with zend", type->name); return; } diff --git a/Lib/php/typemaps.i b/Lib/php/typemaps.i index aaea0a26a..94b351113 100644 --- a/Lib/php/typemaps.i +++ b/Lib/php/typemaps.i @@ -277,7 +277,7 @@ INT_TYPEMAP(unsigned long long); if (!(Z_ISREF($input) && Z_ISNULL_P(Z_REFVAL($input)))) { /* wasn't a pre/ref/thing, OR anything like an int thing */ zend_type_error("Expected reference or NULL for argument $arg of $symname"); - SWIG_FAIL; + return; } } force=0; diff --git a/Source/Modules/php.cxx b/Source/Modules/php.cxx index 40076fc61..d1ef52b48 100644 --- a/Source/Modules/php.cxx +++ b/Source/Modules/php.cxx @@ -359,29 +359,7 @@ public: /* Initialize the rest of the module */ - Printf(s_oinit, " ZEND_INIT_MODULE_GLOBALS(%s, %s_init_globals, NULL);\n", module, module); - /* start the header section */ - Printf(s_header, "ZEND_BEGIN_MODULE_GLOBALS(%s)\n", module); - Printf(s_header, "const char *error_msg;\n"); - Printf(s_header, "int error_code;\n"); - Printf(s_header, "ZEND_END_MODULE_GLOBALS(%s)\n", module); - Printf(s_header, "ZEND_DECLARE_MODULE_GLOBALS(%s)\n", module); - Printf(s_header, "#define SWIG_ErrorMsg() ZEND_MODULE_GLOBALS_ACCESSOR(%s, error_msg)\n", module); - Printf(s_header, "#define SWIG_ErrorCode() ZEND_MODULE_GLOBALS_ACCESSOR(%s, error_code)\n", module); - - Printf(s_header, "static void %s_init_globals(zend_%s_globals *globals ) {\n", module, module); - Printf(s_header, " globals->error_msg = default_error_msg;\n"); - Printf(s_header, " globals->error_code = default_error_code;\n"); - Printf(s_header, "}\n"); - - Printf(s_header, "static void SWIG_ResetError(void) {\n"); - Printf(s_header, " SWIG_ErrorMsg() = default_error_msg;\n"); - Printf(s_header, " SWIG_ErrorCode() = default_error_code;\n"); - Printf(s_header, "}\n"); - - Append(s_header, "\n"); - Printf(s_header, "#define SWIG_name \"%s\"\n", module); Printf(s_header, "#ifdef __cplusplus\n"); Printf(s_header, "extern \"C\" {\n"); @@ -831,7 +809,7 @@ public: Printv(f->code, dispatch, "\n", NIL); Printf(f->code, "zend_throw_exception(zend_ce_type_error, \"No matching function for overloaded '%s'\", 0);\n", symname); - Printv(f->code, "thrown:\n", NIL); + Printv(f->code, "fail:\n", NIL); Printv(f->code, "return;\n", NIL); Printv(f->code, "}\n", NIL); Wrapper_print(f, s_wrappers); @@ -947,12 +925,8 @@ public: Printf(f->code, "add_property_zval_ex(ZEND_THIS, ZSTR_VAL(arg2), ZSTR_LEN(arg2), &args[1]);\n}\n"); } - Printf(f->code, "thrown:\n"); - Printf(f->code, "return;\n"); - - /* Error handling code */ Printf(f->code, "fail:\n"); - Append(f->code, "SWIG_FAIL();\n"); + Printf(f->code, "return;\n"); Printf(f->code, "}\n\n\n"); @@ -984,12 +958,8 @@ public: Printf(f->code, "RETVAL_NULL();\n}\n"); } - Printf(f->code, "thrown:\n"); - Printf(f->code, "return;\n"); - - /* Error handling code */ Printf(f->code, "fail:\n"); - Append(f->code, "SWIG_FAIL();\n"); + Printf(f->code, "return;\n"); Printf(f->code, "}\n\n\n"); @@ -1020,12 +990,8 @@ public: Printf(f->code, "RETVAL_FALSE;\n}\n"); } - Printf(f->code, "thrown:\n"); - Printf(f->code, "return;\n"); - - /* Error handling code */ Printf(f->code, "fail:\n"); - Append(f->code, "SWIG_FAIL();\n"); + Printf(f->code, "return;\n"); Printf(f->code, "}\n\n\n"); Wrapper_print(f, s_wrappers); @@ -1243,8 +1209,6 @@ public: // NOTE: possible we ignore this_ptr as a param for native constructor - Printf(f->code, "SWIG_ResetError();\n"); - if (numopt > 0) { // membervariable wrappers do not have optional args Wrapper_add_local(f, "arg_count", "int arg_count"); Printf(f->code, "arg_count = ZEND_NUM_ARGS();\n"); @@ -1420,14 +1384,9 @@ public: } if (!static_setter) { - Printf(f->code, "thrown:\n"); - Printf(f->code, "return;\n"); - - /* Error handling code */ Printf(f->code, "fail:\n"); Printv(f->code, cleanup, NIL); - Append(f->code, "SWIG_FAIL();\n"); - + Printf(f->code, "return;\n"); Printf(f->code, "}\n"); } @@ -2186,7 +2145,7 @@ public: Delete(outarg); } - Append(w->code, "thrown:\n"); + Append(w->code, "fail: ;\n"); if (!is_void) { if (!(ignored_method && !pure_virtual)) { String *rettype = SwigType_str(returntype, 0); @@ -2197,12 +2156,7 @@ public: } Delete(rettype); } - } else { - Append(w->code, "return;\n"); } - - Append(w->code, "fail:\n"); - Append(w->code, "SWIG_FAIL();\n"); Append(w->code, "}\n"); // We expose protected methods via an extra public inline method which makes a straight call to the wrapped class' method