[RFC] gdbscm_wrap, eliminate GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS (Re: [RFA 2/4] Change gdbscm_exception_message_to_string to return a unique_xmalloc_ptr)

From: Pedro Alves <palves@redhat.com>

  On 07/17/2018 08:10 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> gdbscm_print_exception_message (port, SCM_BOOL_F, key, args);
>>> -  result = gdbscm_scm_to_c_string (scm_get_output_string (port));
>>> +  gdb::unique_xmalloc_ptr<char> result
>>> +    (gdbscm_scm_to_c_string (scm_get_output_string (port)));
>>> scm_close_port (port);
>>> -
>>> return result;
>>> }
> 
> Pedro> Did you try making gdbscm_scm_to_c_string return a unique_ptr too?
> 
> Yes, but this runs into the use of
> GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS in (at least) gdbscm_value_field.
> I haven't looked into what to do about this.

I gave this a try today.  How about this?

The main issue with the Guile code is that we have two types of exceptions
to consider.  GDB/C++ exceptions, and Guile/SJLJ exceptions.
Code that is facing the Guile interpreter must not throw GDB exceptions,
instead Guile exceptions must be thrown.  Also, because Guile exceptions
are SJLJ based, Guile-facing code must not use local objects with dtors,
unless wrapped in a scope with a TRY/CATCH, because the dtors won't
otherwise be run when a Guile exceptions is thrown.

This adds a new gdbscm_wrap wrapper function than encapsulates
a pattern I noticed in many of the functions using 
GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS.  The wrapper
is written such that you can pass either a lambda to it,
or a function plus a variable number of forwarded args.
I used a lambda when its body would be reasonably short,
and a separate function in the larger cases.

I converted a few functions that were using 
GDBSCM_HANDLE_GDB_EXCEPTION to use gdbscm_wrap too because
they followed a similar pattern.

You'll notice that a few cases of make_cleanup calls are replaced
with explicit xfree calls.  The make_cleanup/do_cleanups calls in
those cases are pointless, because do_cleanups won't be called
when a Scheme exception is thrown.  

We also have a couple cases of Guile-facing code using RAII-type
objects to manage memory, but those are incorrect, exactly because
their dtor won't be called if a Guile exception is thrown.  There's
one case like that that I did not fix here, both a cleanup
and a std::string in  gdbscm_execute_gdb_command.

There are only a handful of cleanups in gdb/guile/ after this:

 $ grep make_cleanup *
 guile.c:  cleanups = make_cleanup (xfree, command);
 scm-pretty-print.c:  cleanups = make_cleanup (null_cleanup, NULL);
 scm-pretty-print.c:      struct cleanup *inner_cleanup = make_cleanup (null_cleanup, NULL);
 scm-pretty-print.c:      make_cleanup (xfree, name);
 scm-pretty-print.c:  cleanups = make_cleanup (null_cleanup, NULL);
 scm-value.c:      struct cleanup *cleanups = make_cleanup (xfree, field);

WDYT?  Passes gdb.guile/*.exp here.

From 61aa83947adda43af7edf560b231884311841a84 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 18 Jul 2018 14:08:08 +0100
Subject: [PATCH] gdbscm_wrap, eliminate
 GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS

This eliminates most remaining cleanups under gdb/guile/.
---
 gdb/guile/guile-internal.h |  41 +++--
 gdb/guile/scm-frame.c      |   4 +-
 gdb/guile/scm-math.c       | 374 +++++++++++++++++--------------------
 gdb/guile/scm-symbol.c     |  17 +-
 gdb/guile/scm-type.c       |  16 +-
 gdb/guile/scm-value.c      | 445 +++++++++++++--------------------------------
 6 files changed, 340 insertions(+), 557 deletions(-)

[RFC] gdbscm_wrap, eliminate GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS (Re: [RFA 2/4] Change gdbscm_exception_message_to_string to return a unique_xmalloc_ptr)

Commit Message

Comments

Patch