From 94a8bdca79d3eb510d0dc1ef6a4c15e2094bb36d Mon Sep 17 00:00:00 2001 From: "John K. Luebs" Date: Mon, 26 Feb 2024 12:15:33 -0600 Subject: [PATCH] Better exception safety for Lua binding --- .clang-format | 2 +- luakiwi/luacompat.h | 8 ----- luakiwi/luakiwi-int.h | 82 ++++++++++++++++++++++++++----------------- luakiwi/luakiwi.cpp | 64 +++++++++++++++++++++------------ 4 files changed, 92 insertions(+), 64 deletions(-) diff --git a/.clang-format b/.clang-format index 76a9a41..f9132f8 100644 --- a/.clang-format +++ b/.clang-format @@ -65,7 +65,7 @@ PointerAlignment: Left ReferenceAlignment: Left # New in v13. int &name ==> int& name ReflowComments: false SeparateDefinitionBlocks: Always # New in v14. -SortIncludes: false +SortIncludes: true SortUsingDeclarations: true SpaceAfterCStyleCast: false SpaceAfterLogicalNot: false diff --git a/luakiwi/luacompat.h b/luakiwi/luacompat.h index aed16f4..34d8a0d 100644 --- a/luakiwi/luacompat.h +++ b/luakiwi/luacompat.h @@ -17,14 +17,6 @@ extern "C" { #if !defined(LUA_VERSION_NUM) || LUA_VERSION_NUM == 501 - #define LUA_OPADD 0 - #define LUA_OPSUB 1 - #define LUA_OPMUL 2 - #define LUA_OPDIV 3 - #define LUA_OPMOD 4 - #define LUA_OPPOW 5 - #define LUA_OPUNM 6 - static int lua_absindex(lua_State* L, int i) { if (i < 0 && i > LUA_REGISTRYINDEX) i += lua_gettop(L) + 1; diff --git a/luakiwi/luakiwi-int.h b/luakiwi/luakiwi-int.h index e477b5a..ebad5fd 100644 --- a/luakiwi/luakiwi-int.h +++ b/luakiwi/luakiwi-int.h @@ -141,49 +141,57 @@ template inline const KiwiErr* wrap_err(F&& f) { static const constexpr KiwiErr kKiwiErrUnhandledCxxException { KiwiErrUnknown, - "An unhandled C++ exception occurred."}; + "An unhandled C++ exception occurred." + }; try { f(); } catch (const UnsatisfiableConstraint&) { static const constexpr KiwiErr err { KiwiErrUnsatisfiableConstraint, - "The constraint cannot be satisfied."}; + "The constraint cannot be satisfied." + }; return &err; } catch (const UnknownConstraint&) { static const constexpr KiwiErr err { KiwiErrUnknownConstraint, - "The constraint has not been added to the solver."}; + "The constraint has not been added to the solver." + }; return &err; } catch (const DuplicateConstraint&) { static const constexpr KiwiErr err { KiwiErrDuplicateConstraint, - "The constraint has already been added to the solver."}; + "The constraint has already been added to the solver." + }; return &err; } catch (const UnknownEditVariable&) { static const constexpr KiwiErr err { KiwiErrUnknownEditVariable, - "The edit variable has not been added to the solver."}; + "The edit variable has not been added to the solver." + }; return &err; } catch (const DuplicateEditVariable&) { static const constexpr KiwiErr err { KiwiErrDuplicateEditVariable, - "The edit variable has already been added to the solver."}; + "The edit variable has already been added to the solver." + }; return &err; } catch (const BadRequiredStrength&) { static const constexpr KiwiErr err { KiwiErrBadRequiredStrength, - "A required strength cannot be used in this context."}; + "A required strength cannot be used in this context." + }; return &err; } catch (const InternalSolverError& ex) { static const constexpr KiwiErr base { KiwiErrInternalSolverError, - "An internal solver error occurred."}; + "An internal solver error occurred." + }; return new_error(&base, ex); } catch (std::bad_alloc&) { static const constexpr KiwiErr err {KiwiErrAlloc, "A memory allocation failed."}; @@ -208,9 +216,12 @@ inline const KiwiErr* wrap_err(P&& s, R&& ref, F&& f) { template inline T* make_unmanaged(Args... args) { - auto* o = new T(std::forward(args)...); - o->m_refcount = 1; - return o; + auto* p = new (std::nothrow) T(std::forward(args)...); + if (lk_unlikely(!p)) + return nullptr; + + p->m_refcount = 1; + return p; } template @@ -238,30 +249,35 @@ inline ConstraintData* kiwi_constraint_new( strength = kiwi::strength::required; } - std::vector terms; - terms.reserve(static_cast( - (lhs && lhs->term_count > 0 ? lhs->term_count : 0) - + (rhs && rhs->term_count > 0 ? rhs->term_count : 0) - )); + try { + std::vector terms; - if (lhs) { - for (int i = 0; i < lhs->term_count; ++i) { - const auto& t = lhs->terms[i]; - terms.emplace_back(Variable(t.var), t.coefficient); - } - } - if (rhs) { - for (int i = 0; i < rhs->term_count; ++i) { - const auto& t = rhs->terms[i]; - terms.emplace_back(Variable(t.var), -t.coefficient); - } - } + terms.reserve(static_cast( + (lhs && lhs->term_count > 0 ? lhs->term_count : 0) + + (rhs && rhs->term_count > 0 ? rhs->term_count : 0) + )); - return make_unmanaged( - Expression(std::move(terms), (lhs ? lhs->constant : 0.0) - (rhs ? rhs->constant : 0.0)), - static_cast(op), - strength - ); + if (lhs) { + for (int i = 0; i < lhs->term_count; ++i) { + const auto& t = lhs->terms[i]; + terms.emplace_back(Variable(t.var), t.coefficient); + } + } + if (rhs) { + for (int i = 0; i < rhs->term_count; ++i) { + const auto& t = rhs->terms[i]; + terms.emplace_back(Variable(t.var), -t.coefficient); + } + } + return make_unmanaged( + Expression(std::move(terms), (lhs ? lhs->constant : 0.0) - (rhs ? rhs->constant : 0.0)), + static_cast(op), + strength + ); + + } catch (...) { + return nullptr; + } } inline const KiwiErr* kiwi_solver_add_constraint(Solver& s, ConstraintData* constraint) { diff --git a/luakiwi/luakiwi.cpp b/luakiwi/luakiwi.cpp index b84c02b..bf63335 100644 --- a/luakiwi/luakiwi.cpp +++ b/luakiwi/luakiwi.cpp @@ -1,9 +1,7 @@ -#include "ljkiwi.hpp" #include #include #include -#include "luacompat.h" #include "luakiwi-int.h" namespace { @@ -13,9 +11,7 @@ namespace { enum TypeId { NOTYPE, VAR = 1, TERM, EXPR, CONSTRAINT, SOLVER, ERROR, NUMBER }; -const int ERR_KIND_TAB = NUMBER + 1; -const int VAR_SUB_FN = ERR_KIND_TAB + 1; -const int CONTEXT_TAB_MAX = VAR_SUB_FN + 1; +enum { ERR_KIND_TAB = NUMBER + 1, VAR_SUB_FN, MEM_ERR_MSG, CONTEXT_TAB_MAX }; constexpr const char* const lkiwi_error_kinds[] = { "KiwiErrNone", @@ -218,13 +214,19 @@ inline KiwiSolver* get_solver(lua_State* L, int idx) { return static_cast(check_arg(L, idx, SOLVER)); } -// note this expects the 2nd upvalue to have the variable weak table -VariableData* var_new(lua_State* L, VariableData* var) { - *static_cast(lua_newuserdata(L, sizeof(VariableData*))) = var; - +VariableData** var_new(lua_State* L) { + auto** varp = static_cast(lua_newuserdata(L, sizeof(VariableData*))); push_type(L, VAR); lua_setmetatable(L, -2); + return varp; +} +// note this expects the 2nd upvalue to have the variable weak table +VariableData* var_register(lua_State* L, VariableData* var) { + if (lk_unlikely(!var)) { + lua_rawgeti(L, lua_upvalueindex(1), MEM_ERR_MSG); + lua_error(L); + } #if defined(LUA_VERSION_NUM) && LUA_VERSION_NUM == 501 // a true compatibility shim has performance implications here lua_pushlightuserdata(L, var); @@ -260,10 +262,13 @@ inline ConstraintData* constraint_new( double strength ) { auto** c = static_cast(lua_newuserdata(L, sizeof(ConstraintData*))); - *c = kiwi_constraint_new(lhs, rhs, op, strength); - push_type(L, CONSTRAINT); lua_setmetatable(L, -2); + + if (lk_unlikely(!(*c = kiwi_constraint_new(lhs, rhs, op, strength)))) { + lua_rawgeti(L, lua_upvalueindex(1), MEM_ERR_MSG); + lua_error(L); + } return *c; } @@ -540,11 +545,15 @@ constexpr const struct luaL_Reg kiwi_var_m[] = { {"eq", lkiwi_eq}, {"le", lkiwi_le}, {"ge", lkiwi_ge}, - {0, 0}}; + {0, 0} +}; int lkiwi_var_new(lua_State* L) { const char* name = luaL_optstring(L, 1, ""); - var_new(L, make_unmanaged(name)); + + auto* varp = var_new(L); + var_register(L, *varp = make_unmanaged(name)); + return 1; } @@ -662,8 +671,10 @@ int lkiwi_term_m_index(lua_State* L) { #else lua_rawgetp(L, lua_upvalueindex(2), term->var); #endif - if (lua_isnil(L, -1)) - var_new(L, term->var); + if (lua_isnil(L, -1)) { + auto* varp = var_new(L); + var_register(L, *varp = retain_unmanaged(term->var)); + } return 1; } else if (len == 11 && memcmp("coefficient", k, len) == 0) { lua_pushnumber(L, term->coefficient); @@ -692,7 +703,8 @@ constexpr const struct luaL_Reg kiwi_term_m[] = { {"eq", lkiwi_eq}, {"le", lkiwi_le}, {"ge", lkiwi_ge}, - {0, 0}}; + {0, 0} +}; int lkiwi_term_new(lua_State* L) { auto* var = get_var(L, 1); @@ -908,7 +920,8 @@ constexpr const struct luaL_Reg kiwi_expr_m[] = { {"eq", lkiwi_eq}, {"le", lkiwi_le}, {"ge", lkiwi_ge}, - {0, 0}}; + {0, 0} +}; int lkiwi_expr_new(lua_State* L) { int nterms = lua_gettop(L) - 1; @@ -1060,7 +1073,8 @@ constexpr const struct luaL_Reg kiwi_constraint_m[] = { {"expression", lkiwi_constraint_expression}, {"add_to", lkiwi_constraint_add_to}, {"remove_from", lkiwi_constraint_remove_from}, - {0, 0}}; + {0, 0} +}; int lkiwi_constraint_new(lua_State* L) { const auto* lhs = get_expr_opt(L, 1); @@ -1133,7 +1147,8 @@ constexpr const struct luaL_Reg lkiwi_constraints[] = { {"pair_ratio", lkiwi_constraints_pair_ratio}, {"pair", lkiwi_constraints_pair}, {"single", lkiwi_constraints_single}, - {0, 0}}; + {0, 0} +}; void lkiwi_mod_constraints_new(lua_State* L, int ctx_i) { luaL_newlibtable(L, lkiwi_constraints); @@ -1192,7 +1207,8 @@ int lkiwi_error_m_tostring(lua_State* L) { constexpr const struct luaL_Reg lkiwi_error_m[] = { {"__tostring", lkiwi_error_m_tostring}, - {0, 0}}; + {0, 0} +}; int lkiwi_error_mask(lua_State* L) { int invert = lua_toboolean(L, 2); @@ -1444,7 +1460,8 @@ constexpr const struct luaL_Reg kiwi_solver_m[] = { {"set_error_mask", lkiwi_solver_set_error_mask}, {"__tostring", lkiwi_solver_m_tostring}, {"__gc", lkiwi_solver_m_gc}, - {0, 0}}; + {0, 0} +}; int lkiwi_solver_new(lua_State* L) { lua_Integer error_mask; @@ -1541,7 +1558,8 @@ constexpr const struct luaL_Reg lkiwi[] = { {"eq", lkiwi_eq}, {"le", lkiwi_le}, {"ge", lkiwi_ge}, - {0, 0}}; + {0, 0} +}; int no_member_mt_index(lua_State* L) { luaL_error(L, "attempt to access non-existent member '%s'", lua_tostring(L, 2)); @@ -1619,6 +1637,8 @@ extern "C" LJKIWI_EXPORT int luaopen_ljkiwi(lua_State* L) { int ctx_i = lua_gettop(L); compat_init(L, ctx_i); + lua_pushliteral(L, "kiwi library memory allocation error"); + lua_rawseti(L, ctx_i, MEM_ERR_MSG); no_member_mt_new(L); register_type(L, "kiwi.Var", ctx_i, VAR, kiwi_var_m);