Re: Patch: support callback on allocation overflow instead of calling abort()

2020-10-07 Thread Colin Caine
I agree with what I think Marc is suggesting, that we can get a good solution
here without accounting for all (or perhaps any) memory leaks.

The Julia project doesn't really care about memory leaks in this context anyway.
Here's a comment by one of the language designers, Stefan Karpinski (they're not
subscribed to this list):

>> Since the function could have allocated memory for intermediate computations,
>> how do you avoid memory leaks?
>
> Not much you can do: chances are if you get to the point of OOMing, the
> process is going to crash, we just want the ability to do it more gracefully
> than a C abort call, which is something that a library should really never do.
> If GMP wants to be super careful, it could free anything that was allocated
> before returning an error, but it would also be fine to document that when an
> OOM error is returned some allocated memory may be lost.

(Original: https://github.com/JuliaLang/julia/pull/31215#issuecomment-704279237
)
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs


Re: Patch: support callback on allocation overflow instead of calling abort()

2020-10-07 Thread Colin Caine
Hi Niels,

This sounds hopeful!


> To make any progress with adding a callback like this, we'd need to
> either:
>
> 1. Find out and document how to longjmp out from the callback safely.
>
> 2. Agree and document that when GMP invokes this callback, GMP state
> should be considered invalid. The process must not make any further
> calls to any GMP functions.
>
> And in the latter case, would that be satisfactory for Julia's use?

Yes, this latter case is sufficient for us.

Our users could catch the error and continue anyway, but we can
document, as you can, that this is risky.

On Tue, 6 Oct 2020, 15:59 Niels Möller,  wrote:

> Colin Caine  writes:
>
> > Sorry, this patch was posted to this list last month, but I didn't see it
> > in the archives. See here for related discussion:
> > https://gmplib.org/list-archives/gmp-bugs/2020-September/004865.html
>
> I asked some questions (some off list), and haven't seen satisfactory
> answers. The thing is, it seems fairly safe to use the propsed callback
> to write a friendly error message, backtrace of involved julia
> functions, etc, and then exit the process.
>
> But my impression is that julia may also longjmp out and continue
> execution, and to me, that seems very brittle. You can have memory
> leaks, you may leave GMP data structures in an inconsistent state.
>
> To make any progress with adding a callback like this, we'd need to
> either:
>
> 1. Find out and document how to longjmp out from the callback safely.
>
> 2. Agree and document that when GMP invokes this callback, GMP state
>should be considered invalid. The process must not make any further
>calls to any GMP functions.
>
> And in the latter case, would that be satisfactory for Julia's use?
>
> There are other approaches to avoid these crashes, e.g., I think the
> recent emacs integration uses a (configurable) limit on bignum size, and
> will raise an emacs exception long before hitting GMP's limits, and the
> emacs process can go on running with no issues.
>
> Regards,
> /Niels
>
> --
> Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
> Internet email is subject to wholesale government surveillance.
>
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs


Re: Patch: support callback on allocation overflow instead of calling abort()

2020-10-06 Thread Colin Caine
Sorry, this patch was posted to this list last month, but I didn't see it
in the archives. See here for related discussion:
https://gmplib.org/list-archives/gmp-bugs/2020-September/004865.html

Though this discussion chain about memory leaks is new :)

It's worth noting that this patch is useful to Julia even with memory leaks
(Julia does not patch the leaks out afaict).
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs


Patch: support callback on allocation overflow instead of calling abort()

2020-10-06 Thread Colin Caine
This is used by Julia to raise an OutOfMemory exception rather than having
the Julia process itself abort.

Others on stackoverflow, etc, have experienced similar problems with
undesired aborts(), so this patch would probably be useful to a variety of
users. (see:
https://github.com/JuliaLang/julia/issues/8286#issuecomment-323500953)

More information is available here and in the linked issues and PRs:
https://github.com/JuliaLang/julia/pull/31215

Thanks,
diff --git a/gmp-h.in b/gmp-h.in
--- a/gmp-h.in
+++ b/gmp-h.in
@@ -479,6 +479,13 @@ using std::FILE;
   void *(**) (void *, size_t, size_t),
   void (**) (void *, size_t)) __GMP_NOTHROW;
 
+#define mp_set_alloc_overflow_function __gmp_set_alloc_overflow_function
+__GMP_DECLSPEC void mp_set_alloc_overflow_function (void (*) (void)) __GMP_NOTHROW;
+
+#define mp_get_alloc_overflow_function __gmp_get_alloc_overflow_function
+__GMP_DECLSPEC void mp_get_alloc_overflow_function (void (**) (void)) __GMP_NOTHROW;
+
+
 #define mp_bits_per_limb __gmp_bits_per_limb
 __GMP_DECLSPEC extern const int mp_bits_per_limb;
 
diff --git a/gmp-impl.h b/gmp-impl.h
--- a/gmp-impl.h
+++ b/gmp-impl.h
@@ -696,10 +696,12 @@ struct tmp_debug_entry_t {
 __GMP_DECLSPEC extern void * (*__gmp_allocate_func) (size_t);
 __GMP_DECLSPEC extern void * (*__gmp_reallocate_func) (void *, size_t, size_t);
 __GMP_DECLSPEC extern void   (*__gmp_free_func) (void *, size_t);
+__GMP_DECLSPEC extern void   (*__gmp_alloc_overflow_func)(void);
 
 __GMP_DECLSPEC void *__gmp_default_allocate (size_t);
 __GMP_DECLSPEC void *__gmp_default_reallocate (void *, size_t, size_t);
 __GMP_DECLSPEC void __gmp_default_free (void *, size_t);
+__GMP_DECLSPEC void __gmp_default_alloc_overflow (void);
 
 #define __GMP_ALLOCATE_FUNC_TYPE(n,type) \
   ((type *) (*__gmp_allocate_func) ((n) * sizeof (type)))
@@ -727,6 +729,12 @@ struct tmp_debug_entry_t {
 	(ptr, (oldsize) * sizeof (type), (newsize) * sizeof (type));	\
   } while (0)
 
+#define __GMP_ALLOC_OVERFLOW_FUNC()  \
+  do {   \
+(*__gmp_alloc_overflow_func) (); \
+fprintf (stderr, "unexpected return from alloc_overflow\n"); \
+abort ();\
+  } while (0)
 
 /* Dummy for non-gcc, code involving it will go dead. */
 #if ! defined (__GNUC__) || __GNUC__ < 2
diff --git a/memory.c b/memory.c
--- a/memory.c
+++ b/memory.c
@@ -38,6 +38,7 @@ see https://www.gnu.org/licenses/.  */
 void * (*__gmp_allocate_func) (size_t) = __gmp_default_allocate;
 void * (*__gmp_reallocate_func) (void *, size_t, size_t) = __gmp_default_reallocate;
 void   (*__gmp_free_func) (void *, size_t) = __gmp_default_free;
+void   (*__gmp_alloc_overflow_func) (void) = __gmp_default_alloc_overflow;
 
 
 /* Default allocation functions.  In case of failure to allocate/reallocate
@@ -144,3 +145,10 @@ void
 #endif
   free (blk_ptr);
 }
+
+void
+__gmp_default_alloc_overflow(void)
+{
+fprintf (stderr, "gmp: overflow in mpz type\n");
+abort();
+}
diff --git a/mp_get_fns.c b/mp_get_fns.c
--- a/mp_get_fns.c
+++ b/mp_get_fns.c
@@ -46,3 +46,11 @@ mp_get_memory_functions (void *(**alloc_
   if (free_func != NULL)
 *free_func = __gmp_free_func;
 }
+
+void
+mp_get_alloc_overflow_function(
+void (**alloc_overflow_func) (void)) __GMP_NOTHROW
+{
+  if (alloc_overflow_func != NULL)
+*alloc_overflow_func = __gmp_alloc_overflow_func;
+}
diff --git a/mp_set_fns.c b/mp_set_fns.c
--- a/mp_set_fns.c
+++ b/mp_set_fns.c
@@ -48,3 +48,12 @@ mp_set_memory_functions (void *(*alloc_f
   __gmp_reallocate_func = realloc_func;
   __gmp_free_func = free_func;
 }
+
+void
+mp_set_alloc_overflow_function(
+ void (*alloc_overflow_func) (void)) __GMP_NOTHROW
+{
+  if (alloc_overflow_func == 0)
+alloc_overflow_func = __gmp_default_alloc_overflow;
+  __gmp_alloc_overflow_func = alloc_overflow_func;
+}
diff --git a/mpz/init2.c b/mpz/init2.c
--- a/mpz/init2.c
+++ b/mpz/init2.c
@@ -45,8 +45,7 @@ mpz_init2 (mpz_ptr x, mp_bitcnt_t bits)
 {
   if (UNLIKELY (new_alloc > INT_MAX))
 	{
-	  fprintf (stderr, "gmp: overflow in mpz type\n");
-	  abort ();
+	  __GMP_ALLOC_OVERFLOW_FUNC ();
 	}
 }
 
diff --git a/mpz/realloc.c b/mpz/realloc.c
--- a/mpz/realloc.c
+++ b/mpz/realloc.c
@@ -45,16 +45,14 @@ void *
 {
   if (UNLIKELY (new_alloc > ULONG_MAX / GMP_NUMB_BITS))
 	{
-	  fprintf (stderr, "gmp: overflow in mpz type\n");
-	  abort ();
+	  __GMP_ALLOC_OVERFLOW_FUNC ();
 	}
 }
   else
 {
   if (UNLIKELY (new_alloc > INT_MAX))
 	{
-	  fprintf (stderr, "gmp: overflow in mpz type\n");
-	  abort ();
+	  __GMP_ALLOC_OVERFLOW_FUNC ();
 	}
 }
 
diff --git a/mpz/realloc2.c b/mpz/realloc2.c
--- a/mpz/realloc2.c
+++ b/mpz/realloc2.c
@@ -45,8 +45,7 @@ mpz_realloc2 (mpz_ptr m, mp_bitcnt_t bit
 {
   if (UNLIKELY (new_alloc > INT_MAX))
 	{
-	  fprintf (stderr, "gmp: overflow in mpz type\n");
-	  abort