Skip to content

Fix GAPBIND14_TRY memory leak #1145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
james-d-mitchell merged 2 commits into from
Mar 13, 2026
Merged

Fix GAPBIND14_TRY memory leak #1145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5b19a9
Fix GAPBIND14_TRY memory leak
Joseph-Edwards Mar 11, 2026
6d664a4
Only use __builtin_unreachable if available
Joseph-Edwards Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,27 @@ AC_LANG([C++])

AX_CXX_COMPILE_STDCXX_14(,[mandatory])

dnl compiler builtins
AC_DEFUN([CHECK_COMPILER_BUILTIN],
[AC_MSG_CHECKING([for $1])
AC_LINK_IFELSE(
[AC_LANG_PROGRAM(
[[]],
[$1[($2)];
]
)],
[AS_VAR_SET([[have_]$1], [yes])],
[AS_VAR_SET([[have_]$1], [no])]
)
AC_MSG_RESULT(AS_VAR_GET([[have_]$1]))
AS_IF([test yes = AS_VAR_GET([[have_]$1])],
[AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]$1), 1,
[Define to 1 if the system has the `]$1[' built-in function])], []
)])

CHECK_COMPILER_BUILTIN([__builtin_unreachable],[]);


dnl ##
dnl ## Locate the GAP root dir
dnl ##
Expand Down
60 changes: 53 additions & 7 deletions gapbind14/include/gapbind14/cpp_fn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,61 @@
#include <tuple> // for tuple, tuple_element_t
#include <type_traits> // for true_type

#include "gap_include.hpp" // for UInt

#define GAPBIND14_TRY(something) \
try { \
something; \
} catch (std::exception const& e) { \
ErrorQuit(e.what(), 0L, 0L); \
#include "../../../src/semigroups-config.hpp" // for SEMIGRUOPS_HAVE___BUILTIN_UNREACHABLE
#include "gap_include.hpp" // for UInt

namespace gapbind14::detail {
// Inspired by the possible implementation of std::unreachable (C++23)
// https://en.cppreference.com/w/cpp/utility/unreachable.html
[[noreturn]] inline void unreachable() {
// Uses compiler specific extensions if possible.
// Even if no extension is used, undefined behavior is still raised by
// an empty function body and the noreturn attribute.
#if defined(_MSC_VER) && !defined(__clang__) // MSVC
__assume(false);
#elif defined(SEMIGROUPS_HAVE___BUILTIN_UNREACHABLE)
__builtin_unreachable();
#endif
}
} // namespace gapbind14::detail

// Care needs to be taken when calling the function ErrorQuit, since it handles
// errors in a C-style way with longjmp. This can cause issues with the
// destructors of non-trivially-destructable objects. In particular, if
// ErrorQuit is called within a catch block, the exception object's destructor
// is sometimes not called. This can leak memory.

// To combat this issue, we avoid calling ErrorQuit inside a catch block, thus
// allowing the exception object to be destroyed as normal. Instead, we store
// the information of the error message in a static std::string, and pass that
// to ErrorQuit outside of the catch block.

// JDE originally thought that the object storing the error message would need
// to be trivially-destructable, but it seems that the static string does the
// job.

static std::string gapbind14_try_error_message{};
static bool gapbind14_try_found_an_error = false;

#define GAPBIND14_TRY(something) \
gapbind14_try_found_an_error = false; \
try { \
something; \
} catch (std::exception const& e) { \
gapbind14_try_found_an_error = true; \
gapbind14_try_error_message = e.what(); \
} \
if (gapbind14_try_found_an_error) { \
ErrorQuit(gapbind14_try_error_message.data(), 0L, 0L); \
}

// This second macro exists to suppress warnings of the type "control reaches
// end of non-void function". It should be used whenever <something> begins with
// the word 'return'.
#define GAPBIND14_TRY_WITH_RETURN(something) \
GAPBIND14_TRY(something); \
gapbind14::detail::unreachable();

namespace gapbind14 {

constexpr size_t MAX_FUNCTIONS = 96;
Expand Down
16 changes: 8 additions & 8 deletions gapbind14/include/gapbind14/tame_free_fn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ namespace gapbind14 {
TSFINAE> {
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(wild<Wild>(N)()));
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(wild<Wild>(N)()));
}

template <size_t N, typename Wild, typename TSFINAE = Obj>
Expand All @@ -228,7 +228,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<0>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0))));
}

Expand All @@ -243,7 +243,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<1>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(wild<Wild>(N)(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(wild<Wild>(N)(
to_cpp<to_cpp_0_type>()(arg0), to_cpp<to_cpp_1_type>()(arg1))));
}

Expand All @@ -260,7 +260,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<2>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0),
to_cpp<to_cpp_1_type>()(arg1),
to_cpp<to_cpp_2_type>()(arg2))));
Expand All @@ -281,7 +281,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<3>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0),
to_cpp<to_cpp_1_type>()(arg1),
to_cpp<to_cpp_2_type>()(arg2),
Expand All @@ -305,7 +305,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<4>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0),
to_cpp<to_cpp_1_type>()(arg1),
to_cpp<to_cpp_2_type>()(arg2),
Expand Down Expand Up @@ -333,7 +333,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<5>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0),
to_cpp<to_cpp_1_type>()(arg1),
to_cpp<to_cpp_2_type>()(arg2),
Expand Down Expand Up @@ -369,7 +369,7 @@ namespace gapbind14 {
typename CppFunction<Wild>::params_type::template get<6>;
using to_gap_type
= gapbind14::to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(wild<Wild>(N)(to_cpp<to_cpp_0_type>()(arg0),
to_cpp<to_cpp_1_type>()(arg1),
to_cpp<to_cpp_2_type>()(arg2),
Expand Down
16 changes: 8 additions & 8 deletions gapbind14/include/gapbind14/tame_mem_fn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ namespace gapbind14 {
class_type *ptr = detail::obj_cpp_ptr<class_type>(arg0);

using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(
GAPBIND14_TRY_WITH_RETURN(
return to_gap_type()(CppFunction<Wild>()(wild_mem_fn<Wild>(N), ptr)));
}

Expand All @@ -313,7 +313,7 @@ namespace gapbind14 {
using to_cpp_1_type =
typename CppFunction<Wild>::params_type::template get<0>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(CppFunction<Wild>()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(CppFunction<Wild>()(
wild_mem_fn<Wild>(N), ptr, to_cpp<to_cpp_1_type>()(arg1))));
}

Expand All @@ -332,7 +332,7 @@ namespace gapbind14 {
using to_cpp_2_type =
typename CppFunction<Wild>::params_type::template get<1>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand All @@ -356,7 +356,7 @@ namespace gapbind14 {
using to_cpp_3_type =
typename CppFunction<Wild>::params_type::template get<2>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand All @@ -383,7 +383,7 @@ namespace gapbind14 {
using to_cpp_4_type =
typename CppFunction<Wild>::params_type::template get<3>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand Down Expand Up @@ -419,7 +419,7 @@ namespace gapbind14 {
using to_cpp_5_type =
typename CppFunction<Wild>::params_type::template get<4>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand Down Expand Up @@ -459,7 +459,7 @@ namespace gapbind14 {
using to_cpp_6_type =
typename CppFunction<Wild>::params_type::template get<5>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand Down Expand Up @@ -503,7 +503,7 @@ namespace gapbind14 {
using to_cpp_7_type =
typename CppFunction<Wild>::params_type::template get<6>;
using to_gap_type = to_gap<typename CppFunction<Wild>::return_type>;
GAPBIND14_TRY(return to_gap_type()(
GAPBIND14_TRY_WITH_RETURN(return to_gap_type()(
CppFunction<Wild>()(wild_mem_fn<Wild>(N),
ptr,
to_cpp<to_cpp_1_type>()(arg1),
Expand Down
Loading