public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns
@ 2021-05-03  8:54 Tom de Vries
  2021-05-03 11:18 ` Andrew Burgess
  2021-05-03 20:10 ` Ludovic Courtès
  0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2021-05-03  8:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ludovic Courtès

Hi,

Since gdb commit 880ae75a2b7 "gdb delay guile initialization until
gdbscm_finish_initialization" I'm running into:
...
(gdb) print My_Var > 10.0^M
free(): invalid pointer^M
ERROR: GDB process no longer exists
GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT
UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0
...

The problem is that both gdb and libguile try to set the libgmp memory functions,
and since the gdb commit the ones from libguile are effective, which results
in gdb freeing some memory in a way that is not compatible with the way that
memory was actually allocated.

The fact that libguile tries to set the libgmp memory functions is a bug which
should be fixed starting version v3.0.6.

Meanwhile, work around this in gdb by not allowing libguile to set the libgomp
memory functions.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/guile] Don't allow libguile to change libgmp mem fns

gdb/ChangeLog:

2021-05-03  Tom de Vries  <tdevries@suse.de>

	PR guile/27806
	* guile/guile.c (gdbscm_initialize): Save and restore libgmp memory
	functions.

---
 gdb/guile/guile.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index bdf15cd498b..6ee8b3f47ce 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
   {
     gdb::block_signals blocker;
 
+    /* There are libguile versions (f.i. v3.0.5) that by default call
+       mp_get_memory_functions during initialization to install custom
+       libgmp memory functions.  This is considered a bug and should be
+       fixed starting v3.0.6.
+       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
+       gdbscm_finish_initialization", that bug had no effect for gdb,
+       because gdb subsequently called mp_get_memory_functions to install
+       its own custom functions in _initialize_gmp_utils.  However, since
+       aforementioned gdb commit the initialization order is reversed,
+       allowing libguile to install a custom malloc that is incompatible
+       with the custom free as used in gmp-utils.c, resulting in a
+       "double free or corruption (out)" error.
+       Work around the libguile bug by saving the libgmp memory functions
+       before guile initialization, and restoring them afterwards.  */
+    void *(*alloc_func) (size_t);
+    void *(*realloc_func) (void *, size_t, size_t);
+    void (*free_func) (void *, size_t);
+    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);
+
     /* scm_with_guile is the most portable way to initialize Guile.  Plus
        we need to initialize the Guile support while in Guile mode (e.g.,
        called from within a call to scm_with_guile).  */
     scm_with_guile (call_initialize_gdb_module, NULL);
+
+    /* Restore libgmp memory functions.  */
+    mp_set_memory_functions (alloc_func, realloc_func, free_func);
   }
 
   /* Set Guile's backtrace to match the "set guile print-stack" default.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns
  2021-05-03  8:54 [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns Tom de Vries
@ 2021-05-03 11:18 ` Andrew Burgess
  2021-05-03 12:20   ` Tom de Vries
  2021-05-03 20:10 ` Ludovic Courtès
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2021-05-03 11:18 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches, Ludovic Courtès

* Tom de Vries <tdevries@suse.de> [2021-05-03 10:54:29 +0200]:

> Hi,
> 
> Since gdb commit 880ae75a2b7 "gdb delay guile initialization until
> gdbscm_finish_initialization" I'm running into:
> ...
> (gdb) print My_Var > 10.0^M
> free(): invalid pointer^M
> ERROR: GDB process no longer exists
> GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT
> UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0
> ...
> 
> The problem is that both gdb and libguile try to set the libgmp memory functions,
> and since the gdb commit the ones from libguile are effective, which results
> in gdb freeing some memory in a way that is not compatible with the way that
> memory was actually allocated.
> 
> The fact that libguile tries to set the libgmp memory functions is a bug which
> should be fixed starting version v3.0.6.
> 
> Meanwhile, work around this in gdb by not allowing libguile to set the libgomp
> memory functions.

Thanks for looking into this, and sorry for causing the breakage.

I had a read through the bug, and this solution seems to make sense,
however, I had two thoughts.

First, I already had to solve a similar problem for Python when doing
this work.  For Python the issue related to which signal handlers were
installed.

Though the problem was Python specific, I figured that it was cleaner
to make the solution generic, so I placed the fix in
gdb/extension.c:ext_lang_initialization - look for the use of
scoped_default_sigint.

I wonder if this fix should similarly be placed at this level?  I'll
leave this choice up to you, I don't feel strongly on this, but
thought it might be worth mentioning.

For the second thought, see below...

> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/guile] Don't allow libguile to change libgmp mem fns
> 
> gdb/ChangeLog:
> 
> 2021-05-03  Tom de Vries  <tdevries@suse.de>
> 
> 	PR guile/27806
> 	* guile/guile.c (gdbscm_initialize): Save and restore libgmp memory
> 	functions.
> 
> ---
>  gdb/guile/guile.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
> index bdf15cd498b..6ee8b3f47ce 100644
> --- a/gdb/guile/guile.c
> +++ b/gdb/guile/guile.c
> @@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
>    {
>      gdb::block_signals blocker;
>  
> +    /* There are libguile versions (f.i. v3.0.5) that by default call
> +       mp_get_memory_functions during initialization to install custom
> +       libgmp memory functions.  This is considered a bug and should be
> +       fixed starting v3.0.6.
> +       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
> +       gdbscm_finish_initialization", that bug had no effect for gdb,
> +       because gdb subsequently called mp_get_memory_functions to install
> +       its own custom functions in _initialize_gmp_utils.  However, since
> +       aforementioned gdb commit the initialization order is reversed,
> +       allowing libguile to install a custom malloc that is incompatible
> +       with the custom free as used in gmp-utils.c, resulting in a
> +       "double free or corruption (out)" error.
> +       Work around the libguile bug by saving the libgmp memory functions
> +       before guile initialization, and restoring them afterwards.  */
> +    void *(*alloc_func) (size_t);
> +    void *(*realloc_func) (void *, size_t, size_t);
> +    void (*free_func) (void *, size_t);
> +    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);

I think any time we do SAVE-VALUE -> WORK -> RESTORE-VALUE, we should
be wrapping this up in an RAII class.  Right now I don't believe
scm_with_guile can throw an exception, but you never know how the code
will change in the future, and creating an RAII class now just makes
things future proof.

Thanks,
Andrew

> +
>      /* scm_with_guile is the most portable way to initialize Guile.  Plus
>         we need to initialize the Guile support while in Guile mode (e.g.,
>         called from within a call to scm_with_guile).  */
>      scm_with_guile (call_initialize_gdb_module, NULL);
> +
> +    /* Restore libgmp memory functions.  */
> +    mp_set_memory_functions (alloc_func, realloc_func, free_func);
>    }
>  
>    /* Set Guile's backtrace to match the "set guile print-stack" default.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns
  2021-05-03 11:18 ` Andrew Burgess
@ 2021-05-03 12:20   ` Tom de Vries
  0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2021-05-03 12:20 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Ludovic Courtès

[-- Attachment #1: Type: text/plain, Size: 5235 bytes --]

On 5/3/21 1:18 PM, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2021-05-03 10:54:29 +0200]:
> 
>> Hi,
>>
>> Since gdb commit 880ae75a2b7 "gdb delay guile initialization until
>> gdbscm_finish_initialization" I'm running into:
>> ...
>> (gdb) print My_Var > 10.0^M
>> free(): invalid pointer^M
>> ERROR: GDB process no longer exists
>> GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT
>> UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0
>> ...
>>
>> The problem is that both gdb and libguile try to set the libgmp memory functions,
>> and since the gdb commit the ones from libguile are effective, which results
>> in gdb freeing some memory in a way that is not compatible with the way that
>> memory was actually allocated.
>>
>> The fact that libguile tries to set the libgmp memory functions is a bug which
>> should be fixed starting version v3.0.6.
>>
>> Meanwhile, work around this in gdb by not allowing libguile to set the libgomp
>> memory functions.
> 
> Thanks for looking into this, and sorry for causing the breakage.
> 

Price of progress I'd say :)

> I had a read through the bug, and this solution seems to make sense,
> however, I had two thoughts.
> 
> First, I already had to solve a similar problem for Python when doing
> this work.  For Python the issue related to which signal handlers were
> installed.
> 
> Though the problem was Python specific, I figured that it was cleaner
> to make the solution generic, so I placed the fix in
> gdb/extension.c:ext_lang_initialization - look for the use of
> scoped_default_sigint.
> 
> I wonder if this fix should similarly be placed at this level?  I'll
> leave this choice up to you, I don't feel strongly on this, but
> thought it might be worth mentioning.
> 

Thanks for mentioning that.  I've thought about it for a bit, and I
haven't convinced myself that it's a better idea to do this one level
up.  That is: I'm now working around a known bug.  Increasing the scope
of the workaround means that it will workaround bugs in other
components, possible unknown ones, which we'd like to know about.

So for now I'm leaving the workaround where it is.

FWIW, perhaps what could be done at a larger scope regardless is
checking.  So, currently we have:
...
void
_initialize_gmp_utils ()
{
  /* Tell GMP to use GDB's memory management routines.  */
  mp_set_memory_functions (xmalloc, xrealloc_for_gmp, xfree_for_gmp);
}
...
and we could add some:
...
void
_verify_gmp_utils ()
{
  /* Tell GMP to use GDB's memory management routines.  */
  mp_get_memory_functions (&f1, &f2, &f3);
  gdb_assert (f1 == xmalloc &&  f2 == xrealloc_for_gmp
              && f3 == xfree_for_gmp);
}
...
and use _initialize_gmp_utils () to add _verify_gmp_utils to some verify
hook list that is checked at some appropiate time(s).

> For the second thought, see below...
> 
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/guile] Don't allow libguile to change libgmp mem fns
>>
>> gdb/ChangeLog:
>>
>> 2021-05-03  Tom de Vries  <tdevries@suse.de>
>>
>> 	PR guile/27806
>> 	* guile/guile.c (gdbscm_initialize): Save and restore libgmp memory
>> 	functions.
>>
>> ---
>>  gdb/guile/guile.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
>> index bdf15cd498b..6ee8b3f47ce 100644
>> --- a/gdb/guile/guile.c
>> +++ b/gdb/guile/guile.c
>> @@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
>>    {
>>      gdb::block_signals blocker;
>>  
>> +    /* There are libguile versions (f.i. v3.0.5) that by default call
>> +       mp_get_memory_functions during initialization to install custom
>> +       libgmp memory functions.  This is considered a bug and should be
>> +       fixed starting v3.0.6.
>> +       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
>> +       gdbscm_finish_initialization", that bug had no effect for gdb,
>> +       because gdb subsequently called mp_get_memory_functions to install
>> +       its own custom functions in _initialize_gmp_utils.  However, since
>> +       aforementioned gdb commit the initialization order is reversed,
>> +       allowing libguile to install a custom malloc that is incompatible
>> +       with the custom free as used in gmp-utils.c, resulting in a
>> +       "double free or corruption (out)" error.
>> +       Work around the libguile bug by saving the libgmp memory functions
>> +       before guile initialization, and restoring them afterwards.  */
>> +    void *(*alloc_func) (size_t);
>> +    void *(*realloc_func) (void *, size_t, size_t);
>> +    void (*free_func) (void *, size_t);
>> +    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);
> 
> I think any time we do SAVE-VALUE -> WORK -> RESTORE-VALUE, we should
> be wrapping this up in an RAII class.  Right now I don't believe
> scm_with_guile can throw an exception, but you never know how the code
> will change in the future, and creating an RAII class now just makes
> things future proof.
> 

Yes, that makes sense, I've updated the patch.

I'll commit like this unless you have further comments.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-guile-Don-t-allow-libguile-to-change-libgmp-mem-fns.patch --]
[-- Type: text/x-patch, Size: 3794 bytes --]

[gdb/guile] Don't allow libguile to change libgmp mem fns

Since gdb commit 880ae75a2b7 "gdb delay guile initialization until
gdbscm_finish_initialization" I'm running into:
...
(gdb) print My_Var > 10.0^M
free(): invalid pointer^M
ERROR: GDB process no longer exists
GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT
UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0
...

The problem is that both gdb and libguile try to set the libgmp memory functions,
and since the gdb commit the ones from libguile are effective, which results
in gdb freeing some memory in a way that is not compatible with the way that
memory was actually allocated.

The fact that libguile tries to set the libgmp memory functions is a bug which
should be fixed starting version v3.0.6.

Meanwhile, work around this in gdb by not allowing libguile to set the libgomp
memory functions.

Tested on x86_64-linux.

gdb/ChangeLog:

2021-05-03  Tom de Vries  <tdevries@suse.de>

	PR guile/27806
	* guile/guile.c (struct scoped_restore_libgmp_memory_functions): New
	struct.
	(gdbscm_initialize): Use scoped_restore_libgmp_memory_functions to
	save and restore libgmp memory functions.

---
 gdb/guile/guile.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index bdf15cd498b..2d121b19505 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -637,6 +637,29 @@ call_initialize_gdb_module (void *data)
   return NULL;
 }
 
+/* RAII class used to restore libgmp memory functions.  */
+
+struct scoped_restore_libgmp_memory_functions
+{
+  scoped_restore_libgmp_memory_functions ()
+  {
+    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);
+  }
+
+  ~scoped_restore_libgmp_memory_functions ()
+  {
+    mp_set_memory_functions (alloc_func, realloc_func, free_func);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (scoped_restore_libgmp_memory_functions);
+
+private:
+  /* The saved libgmp memory functions that need to be restored.  */
+    void *(*alloc_func) (size_t);
+    void *(*realloc_func) (void *, size_t, size_t);
+    void (*free_func) (void *, size_t);
+};
+
 /* A callback to initialize Guile after gdb has finished all its
    initialization.  This is the extension_language_ops.initialize "method".  */
 
@@ -662,6 +685,22 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
   {
     gdb::block_signals blocker;
 
+    /* There are libguile versions (f.i. v3.0.5) that by default call
+       mp_get_memory_functions during initialization to install custom
+       libgmp memory functions.  This is considered a bug and should be
+       fixed starting v3.0.6.
+       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
+       gdbscm_finish_initialization", that bug had no effect for gdb,
+       because gdb subsequently called mp_get_memory_functions to install
+       its own custom functions in _initialize_gmp_utils.  However, since
+       aforementioned gdb commit the initialization order is reversed,
+       allowing libguile to install a custom malloc that is incompatible
+       with the custom free as used in gmp-utils.c, resulting in a
+       "double free or corruption (out)" error.
+       Work around the libguile bug by saving the libgmp memory functions
+       before guile initialization, and restoring them afterwards.  */
+    scoped_restore_libgmp_memory_functions restore_libgmp_memory_functions;
+
     /* scm_with_guile is the most portable way to initialize Guile.  Plus
        we need to initialize the Guile support while in Guile mode (e.g.,
        called from within a call to scm_with_guile).  */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns
  2021-05-03  8:54 [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns Tom de Vries
  2021-05-03 11:18 ` Andrew Burgess
@ 2021-05-03 20:10 ` Ludovic Courtès
  2021-05-04  8:27   ` Tom de Vries
  1 sibling, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2021-05-03 20:10 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

Hi Tom,

Tom de Vries <tdevries@suse.de> skribis:

> The fact that libguile tries to set the libgmp memory functions is a bug which
> should be fixed starting version v3.0.6.

Yes.  Building Guile with mini-GMP is recommended in 3.0.6 and later.

> +++ b/gdb/guile/guile.c
> @@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
>    {
>      gdb::block_signals blocker;
>  
> +    /* There are libguile versions (f.i. v3.0.5) that by default call
> +       mp_get_memory_functions during initialization to install custom
> +       libgmp memory functions.  This is considered a bug and should be
> +       fixed starting v3.0.6.
> +       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
> +       gdbscm_finish_initialization", that bug had no effect for gdb,
> +       because gdb subsequently called mp_get_memory_functions to install
> +       its own custom functions in _initialize_gmp_utils.  However, since
> +       aforementioned gdb commit the initialization order is reversed,
> +       allowing libguile to install a custom malloc that is incompatible
> +       with the custom free as used in gmp-utils.c, resulting in a
> +       "double free or corruption (out)" error.
> +       Work around the libguile bug by saving the libgmp memory functions
> +       before guile initialization, and restoring them afterwards.  */
> +    void *(*alloc_func) (size_t);
> +    void *(*realloc_func) (void *, size_t, size_t);
> +    void (*free_func) (void *, size_t);
> +    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);
> +
>      /* scm_with_guile is the most portable way to initialize Guile.  Plus
>         we need to initialize the Guile support while in Guile mode (e.g.,
>         called from within a call to scm_with_guile).  */
>      scm_with_guile (call_initialize_gdb_module, NULL);
> +
> +    /* Restore libgmp memory functions.  */
> +    mp_set_memory_functions (alloc_func, realloc_func, free_func);

This code would lead to memory leaks because Guile would still think it
has its memory functions installed so it would never explicitly free GMP
memory.

Instead, you can do:

  scm_install_gmp_memory_functions = 0;

before the first call to ‘scm_with_guile’.  That works with 3.0, 2.2,
and 2.0.

HTH!

Ludo’.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns
  2021-05-03 20:10 ` Ludovic Courtès
@ 2021-05-04  8:27   ` Tom de Vries
  0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2021-05-04  8:27 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: gdb-patches, Andrew Burgess

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

On 5/3/21 10:10 PM, Ludovic Courtès wrote:
> Hi Tom,
> 
> Tom de Vries <tdevries@suse.de> skribis:
> 
>> The fact that libguile tries to set the libgmp memory functions is a bug which
>> should be fixed starting version v3.0.6.
> 
> Yes.  Building Guile with mini-GMP is recommended in 3.0.6 and later.
> 
>> +++ b/gdb/guile/guile.c
>> @@ -662,10 +662,32 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
>>    {
>>      gdb::block_signals blocker;
>>  
>> +    /* There are libguile versions (f.i. v3.0.5) that by default call
>> +       mp_get_memory_functions during initialization to install custom
>> +       libgmp memory functions.  This is considered a bug and should be
>> +       fixed starting v3.0.6.
>> +       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
>> +       gdbscm_finish_initialization", that bug had no effect for gdb,
>> +       because gdb subsequently called mp_get_memory_functions to install
>> +       its own custom functions in _initialize_gmp_utils.  However, since
>> +       aforementioned gdb commit the initialization order is reversed,
>> +       allowing libguile to install a custom malloc that is incompatible
>> +       with the custom free as used in gmp-utils.c, resulting in a
>> +       "double free or corruption (out)" error.
>> +       Work around the libguile bug by saving the libgmp memory functions
>> +       before guile initialization, and restoring them afterwards.  */
>> +    void *(*alloc_func) (size_t);
>> +    void *(*realloc_func) (void *, size_t, size_t);
>> +    void (*free_func) (void *, size_t);
>> +    mp_get_memory_functions (&alloc_func, &realloc_func, &free_func);
>> +
>>      /* scm_with_guile is the most portable way to initialize Guile.  Plus
>>         we need to initialize the Guile support while in Guile mode (e.g.,
>>         called from within a call to scm_with_guile).  */
>>      scm_with_guile (call_initialize_gdb_module, NULL);
>> +
>> +    /* Restore libgmp memory functions.  */
>> +    mp_set_memory_functions (alloc_func, realloc_func, free_func);
> 
> This code would lead to memory leaks because Guile would still think it
> has its memory functions installed so it would never explicitly free GMP
> memory.
> 
> Instead, you can do:
> 
>   scm_install_gmp_memory_functions = 0;
> 
> before the first call to ‘scm_with_guile’.  That works with 3.0, 2.2,
> and 2.0.
> 
> HTH!

It does, thanks for the review :)

Committed as below.

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-guile-Don-t-allow-libguile-to-change-libgmp-mem-fns.patch --]
[-- Type: text/x-patch, Size: 2643 bytes --]

[gdb/guile] Don't allow libguile to change libgmp mem fns

Since gdb commit 880ae75a2b7 "gdb delay guile initialization until
gdbscm_finish_initialization" I'm running into:
...
(gdb) print My_Var > 10.0^M
free(): invalid pointer^M
ERROR: GDB process no longer exists
GDB process exited with wait status 5995 exp9 0 0 CHILDKILLED SIGABRT SIGABRT
UNRESOLVED: gdb.ada/fixed_cmp.exp: gnat_encodings=all: print My_Var > 10.0
...

The problem is that both gdb and libguile try to set the libgmp memory functions,
and since the gdb commit the ones from libguile are effective, which results
in gdb freeing some memory in a way that is not compatible with the way that
memory was actually allocated.

The fact that libguile tries to set the libgmp memory functions is a bug which
should be fixed starting version v3.0.6.

Meanwhile, work around this in gdb by not allowing libguile to set the libgomp
memory functions.

Tested on x86_64-linux.

gdb/ChangeLog:

2021-05-03  Tom de Vries  <tdevries@suse.de>

	PR guile/27806
	* guile/guile.c	(gdbscm_initialize): Don't let guile change libgmp
	memory functions.

---
 gdb/guile/guile.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index bdf15cd498b..c6959f5b713 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -662,6 +662,22 @@ gdbscm_initialize (const struct extension_language_defn *extlang)
   {
     gdb::block_signals blocker;
 
+    /* There are libguile versions (f.i. v3.0.5) that by default call
+       mp_get_memory_functions during initialization to install custom
+       libgmp memory functions.  This is considered a bug and should be
+       fixed starting v3.0.6.
+       Before gdb commit 880ae75a2b7 "gdb delay guile initialization until
+       gdbscm_finish_initialization", that bug had no effect for gdb,
+       because gdb subsequently called mp_get_memory_functions to install
+       its own custom functions in _initialize_gmp_utils.  However, since
+       aforementioned gdb commit the initialization order is reversed,
+       allowing libguile to install a custom malloc that is incompatible
+       with the custom free as used in gmp-utils.c, resulting in a
+       "double free or corruption (out)" error.
+       Work around the libguile bug by disabling the installation of the
+       libgmp memory functions by guile initialization.  */
+    scm_install_gmp_memory_functions = 0;
+
     /* scm_with_guile is the most portable way to initialize Guile.  Plus
        we need to initialize the Guile support while in Guile mode (e.g.,
        called from within a call to scm_with_guile).  */

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-05-04  8:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03  8:54 [PATCH][gdb/guile] Don't allow libguile to change libgmp mem fns Tom de Vries
2021-05-03 11:18 ` Andrew Burgess
2021-05-03 12:20   ` Tom de Vries
2021-05-03 20:10 ` Ludovic Courtès
2021-05-04  8:27   ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).