public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] [gdb] Fix rethrow exception slicing
@ 2022-10-24  8:49 Tom de Vries
  2022-10-24  8:49 ` [PATCH 1/2] [gdb] Fix rethrow exception slicing in pretty_print_insn Tom de Vries
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Tom de Vries @ 2022-10-24  8:49 UTC (permalink / raw)
  To: gdb-patches

While investigating PR29712, I used my ignore-errors patch (
https://sourceware.org/pipermail/gdb-patches/2021-May/178990.html ) and
found that the thrown error was not caught by ignore-errors due to object
slicing of the exception by a rethrow.

In other words, a gdb_exception_error is thrown, caught and rethrown, but the
rethrow throws a gdb_exception instead, which will not be caught by an
encapsulating catch gdb_exception_error.

Fix this and one more occurrence of rethrow exception slicing that I found
using grepping.

Tom de Vries (2):
  [gdb] Fix rethrow exception slicing in pretty_print_insn
  [gdb] Fix rethrow exception slicing in insert_bp_location

 gdb/breakpoint.c | 28 +++++++++++++++++++++-------
 gdb/disasm.c     |  4 ++--
 2 files changed, 23 insertions(+), 9 deletions(-)


base-commit: b347f578952a29ff9b02090b0dafec563520c80b
-- 
2.35.3


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

* [PATCH 1/2] [gdb] Fix rethrow exception slicing in pretty_print_insn
  2022-10-24  8:49 [PATCH 0/2] [gdb] Fix rethrow exception slicing Tom de Vries
@ 2022-10-24  8:49 ` Tom de Vries
  2022-10-24  8:49 ` [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location Tom de Vries
  2022-10-24 11:20 ` [PATCH 0/2] [gdb] Fix rethrow exception slicing Andrew Burgess
  2 siblings, 0 replies; 11+ messages in thread
From: Tom de Vries @ 2022-10-24  8:49 UTC (permalink / raw)
  To: gdb-patches

The preferred way of rethrowing an exception is by using throw without
expression, because it avoids object slicing of the exception [1].

Fix this in gdb_pretty_print_disassembler::pretty_print_insn.

[1] https://en.cppreference.com/w/cpp/language/throw

Tested on x86_64-linux.
---
 gdb/disasm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index de44aac3263..60f95c398a9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -445,7 +445,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	size = m_di.print_insn (pc);
 	gdb_assert (size > 0);
       }
-    catch (const gdb_exception &ex)
+    catch (const gdb_exception &)
       {
 	/* An exception was thrown while disassembling the instruction.
 	   However, the disassembler might still have written something
@@ -454,7 +454,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	   object destructor as the write itself might throw an exception
 	   if the pager kicks in, and the user selects quit.  */
 	write_out_insn_buffer ();
-	throw ex;
+	throw;
       }
 
     if ((flags & (DISASSEMBLY_RAW_INSN | DISASSEMBLY_RAW_BYTES)) != 0)
-- 
2.35.3


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

* [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24  8:49 [PATCH 0/2] [gdb] Fix rethrow exception slicing Tom de Vries
  2022-10-24  8:49 ` [PATCH 1/2] [gdb] Fix rethrow exception slicing in pretty_print_insn Tom de Vries
@ 2022-10-24  8:49 ` Tom de Vries
  2022-10-24 16:36   ` Pedro Alves
  2022-10-24 11:20 ` [PATCH 0/2] [gdb] Fix rethrow exception slicing Andrew Burgess
  2 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2022-10-24  8:49 UTC (permalink / raw)
  To: gdb-patches

The preferred way of rethrowing an exception is by using throw without
expression, because it avoids object slicing of the exception [1].

Fix this in insert_bp_location.

[1] https://en.cppreference.com/w/cpp/language/throw
---
 gdb/breakpoint.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a0653f189b9..a001e78cfb4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2647,6 +2647,24 @@ breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr)
     return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr);
 }
 
+#define RETHROW_ON_TARGET_CLOSE_ERROR(E)				\
+  do									\
+    {									\
+      if ((E).reason != 0)						\
+	{								\
+	  /* Can't set the breakpoint.  */				\
+									\
+	  if ((E).error == TARGET_CLOSE_ERROR)				\
+	    /* If the target has closed then it will have deleted any	\
+	       breakpoints inserted within the target inferior, as a	\
+	       result any further attempts to interact with the		\
+	       breakpoint objects is not possible.  Just rethrow the	\
+	       error.  Don't use E to rethrow, to prevent object	\
+	       slicing of the exception.  */				\
+	    throw;							\
+	}								\
+    } while (0)
+
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
    DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems.
@@ -2734,6 +2752,7 @@ insert_bp_location (struct bp_location *bl,
 	    }
 	  catch (gdb_exception &e)
 	    {
+	      RETHROW_ON_TARGET_CLOSE_ERROR (e);
 	      bp_excpt = std::move (e);
 	    }
 	}
@@ -2773,6 +2792,7 @@ insert_bp_location (struct bp_location *bl,
 		    }
 		  catch (gdb_exception &e)
 		    {
+		      RETHROW_ON_TARGET_CLOSE_ERROR (e);
 		      bp_excpt = std::move (e);
 		    }
 
@@ -2797,6 +2817,7 @@ insert_bp_location (struct bp_location *bl,
 		}
 	      catch (gdb_exception &e)
 		{
+		  RETHROW_ON_TARGET_CLOSE_ERROR (e);
 		  bp_excpt = std::move (e);
 		}
 	    }
@@ -2811,13 +2832,6 @@ insert_bp_location (struct bp_location *bl,
       if (bp_excpt.reason != 0)
 	{
 	  /* Can't set the breakpoint.  */
-
-	  /* If the target has closed then it will have deleted any
-	     breakpoints inserted within the target inferior, as a result
-	     any further attempts to interact with the breakpoint objects
-	     is not possible.  Just rethrow the error.  */
-	  if (bp_excpt.error == TARGET_CLOSE_ERROR)
-	    throw bp_excpt;
 	  gdb_assert (bl->owner != nullptr);
 
 	  /* In some cases, we might not be able to insert a
-- 
2.35.3


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

* Re: [PATCH 0/2] [gdb] Fix rethrow exception slicing
  2022-10-24  8:49 [PATCH 0/2] [gdb] Fix rethrow exception slicing Tom de Vries
  2022-10-24  8:49 ` [PATCH 1/2] [gdb] Fix rethrow exception slicing in pretty_print_insn Tom de Vries
  2022-10-24  8:49 ` [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location Tom de Vries
@ 2022-10-24 11:20 ` Andrew Burgess
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Burgess @ 2022-10-24 11:20 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

> While investigating PR29712, I used my ignore-errors patch (
> https://sourceware.org/pipermail/gdb-patches/2021-May/178990.html ) and
> found that the thrown error was not caught by ignore-errors due to object
> slicing of the exception by a rethrow.
>
> In other words, a gdb_exception_error is thrown, caught and rethrown, but the
> rethrow throws a gdb_exception instead, which will not be caught by an
> encapsulating catch gdb_exception_error.
>
> Fix this and one more occurrence of rethrow exception slicing that I found
> using grepping.

Both of these look good for me.

Thanks,
Andrew


>
> Tom de Vries (2):
>   [gdb] Fix rethrow exception slicing in pretty_print_insn
>   [gdb] Fix rethrow exception slicing in insert_bp_location
>
>  gdb/breakpoint.c | 28 +++++++++++++++++++++-------
>  gdb/disasm.c     |  4 ++--
>  2 files changed, 23 insertions(+), 9 deletions(-)
>
>
> base-commit: b347f578952a29ff9b02090b0dafec563520c80b
> -- 
> 2.35.3


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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24  8:49 ` [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location Tom de Vries
@ 2022-10-24 16:36   ` Pedro Alves
  2022-10-24 16:43     ` Tom de Vries
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2022-10-24 16:36 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:

> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)				\
> +  do									\
> +    {									\
> +      if ((E).reason != 0)						\
> +	{								\
> +	  /* Can't set the breakpoint.  */				\
> +									\
> +	  if ((E).error == TARGET_CLOSE_ERROR)				\
> +	    /* If the target has closed then it will have deleted any	\
> +	       breakpoints inserted within the target inferior, as a	\
> +	       result any further attempts to interact with the		\
> +	       breakpoint objects is not possible.  Just rethrow the	\
> +	       error.  Don't use E to rethrow, to prevent object	\
> +	       slicing of the exception.  */				\
> +	    throw;							\
> +	}								\
> +    } while (0)

Is there a reason this is a macro instead of a function?

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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24 16:36   ` Pedro Alves
@ 2022-10-24 16:43     ` Tom de Vries
  2022-10-24 16:48       ` Simon Marchi
  2022-10-24 16:51       ` Pedro Alves
  0 siblings, 2 replies; 11+ messages in thread
From: Tom de Vries @ 2022-10-24 16:43 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 10/24/22 18:36, Pedro Alves wrote:
> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
> 
>> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)				\
>> +  do									\
>> +    {									\
>> +      if ((E).reason != 0)						\
>> +	{								\
>> +	  /* Can't set the breakpoint.  */				\
>> +									\
>> +	  if ((E).error == TARGET_CLOSE_ERROR)				\
>> +	    /* If the target has closed then it will have deleted any	\
>> +	       breakpoints inserted within the target inferior, as a	\
>> +	       result any further attempts to interact with the		\
>> +	       breakpoint objects is not possible.  Just rethrow the	\
>> +	       error.  Don't use E to rethrow, to prevent object	\
>> +	       slicing of the exception.  */				\
>> +	    throw;							\
>> +	}								\
>> +    } while (0)
> 
> Is there a reason this is a macro instead of a function?

yes, the throw without expression.

Do you know of a way to do this using a function?

Thanks,
- Tom

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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24 16:43     ` Tom de Vries
@ 2022-10-24 16:48       ` Simon Marchi
  2022-10-25  7:08         ` Tom de Vries
  2022-10-24 16:51       ` Pedro Alves
  1 sibling, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2022-10-24 16:48 UTC (permalink / raw)
  To: Tom de Vries, Pedro Alves, gdb-patches

On 10/24/22 12:43, Tom de Vries via Gdb-patches wrote:
> On 10/24/22 18:36, Pedro Alves wrote:
>> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
>>
>>> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)                \
>>> +  do                                    \
>>> +    {                                    \
>>> +      if ((E).reason != 0)                        \
>>> +    {                                \
>>> +      /* Can't set the breakpoint.  */                \
>>> +                                    \
>>> +      if ((E).error == TARGET_CLOSE_ERROR)                \
>>> +        /* If the target has closed then it will have deleted any    \
>>> +           breakpoints inserted within the target inferior, as a    \
>>> +           result any further attempts to interact with the        \
>>> +           breakpoint objects is not possible.  Just rethrow the    \
>>> +           error.  Don't use E to rethrow, to prevent object    \
>>> +           slicing of the exception.  */                \
>>> +        throw;                            \
>>> +    }                                \
>>> +    } while (0)
>>
>> Is there a reason this is a macro instead of a function?
> 
> yes, the throw without expression.
> 
> Do you know of a way to do this using a function?

Maybe by passing the exception object as a parameter and using:

https://en.cppreference.com/w/cpp/error/rethrow_exception

?

Simon


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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24 16:43     ` Tom de Vries
  2022-10-24 16:48       ` Simon Marchi
@ 2022-10-24 16:51       ` Pedro Alves
  2022-10-25  7:14         ` Tom de Vries
  1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2022-10-24 16:51 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches



On 2022-10-24 5:43 p.m., Tom de Vries wrote:
> On 10/24/22 18:36, Pedro Alves wrote:
>> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
>>
>>> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)                \
>>> +  do                                    \
>>> +    {                                    \
>>> +      if ((E).reason != 0)                        \
>>> +    {                                \
>>> +      /* Can't set the breakpoint.  */                \
>>> +                                    \
>>> +      if ((E).error == TARGET_CLOSE_ERROR)                \
>>> +        /* If the target has closed then it will have deleted any    \
>>> +           breakpoints inserted within the target inferior, as a    \
>>> +           result any further attempts to interact with the        \
>>> +           breakpoint objects is not possible.  Just rethrow the    \
>>> +           error.  Don't use E to rethrow, to prevent object    \
>>> +           slicing of the exception.  */                \
>>> +        throw;                            \
>>> +    }                                \
>>> +    } while (0)
>>
>> Is there a reason this is a macro instead of a function?
> 
> yes, the throw without expression.
> 
> Do you know of a way to do this using a function?

WDYM?  I believe it should Just Work.  E.g.:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#include <stdio.h>

struct excpt
{
};

struct excpt2 : excpt
{
};

void
rethrow ()
{
  throw;
}

int main ()
{
  try
    {
      try
	{
	  throw excpt2{};
	}
      catch (const excpt &)
	{
	  rethrow ();
	}
    }
  catch (const excpt2 &)
    {
      printf ("caught excpt2\n");
    }
  return 0;
}

$ g++ test_rethrow.cc -o test_rethrow
$ ./test_rethrow
caught excpt2
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24 16:48       ` Simon Marchi
@ 2022-10-25  7:08         ` Tom de Vries
  0 siblings, 0 replies; 11+ messages in thread
From: Tom de Vries @ 2022-10-25  7:08 UTC (permalink / raw)
  To: Simon Marchi, Pedro Alves, gdb-patches

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

On 10/24/22 18:48, Simon Marchi wrote:
> On 10/24/22 12:43, Tom de Vries via Gdb-patches wrote:
>> On 10/24/22 18:36, Pedro Alves wrote:
>>> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
>>>
>>>> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)                \
>>>> +  do                                    \
>>>> +    {                                    \
>>>> +      if ((E).reason != 0)                        \
>>>> +    {                                \
>>>> +      /* Can't set the breakpoint.  */                \
>>>> +                                    \
>>>> +      if ((E).error == TARGET_CLOSE_ERROR)                \
>>>> +        /* If the target has closed then it will have deleted any    \
>>>> +           breakpoints inserted within the target inferior, as a    \
>>>> +           result any further attempts to interact with the        \
>>>> +           breakpoint objects is not possible.  Just rethrow the    \
>>>> +           error.  Don't use E to rethrow, to prevent object    \
>>>> +           slicing of the exception.  */                \
>>>> +        throw;                            \
>>>> +    }                                \
>>>> +    } while (0)
>>>
>>> Is there a reason this is a macro instead of a function?
>>
>> yes, the throw without expression.
>>
>> Do you know of a way to do this using a function?
> 
> Maybe by passing the exception object as a parameter and using:
> 
> https://en.cppreference.com/w/cpp/error/rethrow_exception
> 
> ?

Thanks for pointing out this possibility, I've tried this out, as attached.

I like the fact that we no longer throw something implicitly defined by 
a catch outside the function, which looks a bit awkward to me.

But sofar, I didn't manage to understand from reading the specifications 
of std::current_exception and std::rethrow_exception whether there will 
be copying of the exception or not, and AFAICT only the throw without 
expression guarantees to reuse the existing exception object, so I'm 
going with that instead (which I'll post in reply to Pedro).

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Rewrite-RETHROW_ON_TARGET_CLOSE_ERROR-into-funct.patch --]
[-- Type: text/x-patch, Size: 3579 bytes --]

From 48ae602c38c8b879baa76a7378176bc9b80f8662 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 25 Oct 2022 08:17:46 +0200
Subject: [PATCH] [gdb] Rewrite RETHROW_ON_TARGET_CLOSE_ERROR into function

Recent commit b2829fcf9b5 ("[gdb] Fix rethrow exception slicing in
insert_bp_location") introduced macro RETHROW_ON_TARGET_CLOSE_ERROR.

I wrote this as a macro in order to have the rethrowing throw be part of the
same function as the catch, but as it turns out that's not necessary.

Rewrite into a function.

Rebuild on x86_64-linux and tested gdb.server/server-kill.exp, which
triggers the rethrow.
---
 gdb/breakpoint.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a001e78cfb4..ca055aa5398 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2647,23 +2647,28 @@ breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr)
     return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr);
 }
 
-#define RETHROW_ON_TARGET_CLOSE_ERROR(E)				\
-  do									\
-    {									\
-      if ((E).reason != 0)						\
-	{								\
-	  /* Can't set the breakpoint.  */				\
-									\
-	  if ((E).error == TARGET_CLOSE_ERROR)				\
-	    /* If the target has closed then it will have deleted any	\
-	       breakpoints inserted within the target inferior, as a	\
-	       result any further attempts to interact with the		\
-	       breakpoint objects is not possible.  Just rethrow the	\
-	       error.  Don't use E to rethrow, to prevent object	\
-	       slicing of the exception.  */				\
-	    throw;							\
-	}								\
-    } while (0)
+/* Rethrow the currently handled exception, if it's a TARGET_CLOSE_ERROR.
+   E is either the currently handled exception, or a copy, or a sliced copy,
+   so we can't rethrow that one, but we can use it to inspect the properties
+   of the currently handled exception.  Instead, rethrow EPTR.  */
+
+static void
+rethrow_on_target_close_error (const gdb_exception &e, std::exception_ptr eptr)
+{
+  if (e.reason == 0)
+    return;
+  /* Can't set the breakpoint.  */
+
+  if (e.error != TARGET_CLOSE_ERROR)
+    return;
+
+  /* If the target has closed then it will have deleted any breakpoints
+     inserted within the target inferior, as a result any further attempts
+     to interact with the breakpoint objects is not possible.  Just rethrow
+     the error.  Don't use e to rethrow, to prevent object slicing of the
+     exception.  */
+  std::rethrow_exception (eptr);
+}
 
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
@@ -2752,7 +2757,7 @@ insert_bp_location (struct bp_location *bl,
 	    }
 	  catch (gdb_exception &e)
 	    {
-	      RETHROW_ON_TARGET_CLOSE_ERROR (e);
+	      rethrow_on_target_close_error (e, std::current_exception());
 	      bp_excpt = std::move (e);
 	    }
 	}
@@ -2792,7 +2797,7 @@ insert_bp_location (struct bp_location *bl,
 		    }
 		  catch (gdb_exception &e)
 		    {
-		      RETHROW_ON_TARGET_CLOSE_ERROR (e);
+		      rethrow_on_target_close_error (e, std::current_exception());
 		      bp_excpt = std::move (e);
 		    }
 
@@ -2817,7 +2822,7 @@ insert_bp_location (struct bp_location *bl,
 		}
 	      catch (gdb_exception &e)
 		{
-		  RETHROW_ON_TARGET_CLOSE_ERROR (e);
+		  rethrow_on_target_close_error (e, std::current_exception());
 		  bp_excpt = std::move (e);
 		}
 	    }

base-commit: c6d20401a20148b032cb7fb9dba079466a9383cc
-- 
2.35.3


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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-24 16:51       ` Pedro Alves
@ 2022-10-25  7:14         ` Tom de Vries
  2022-10-25  9:26           ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2022-10-25  7:14 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

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

On 10/24/22 18:51, Pedro Alves wrote:
> 
> 
> On 2022-10-24 5:43 p.m., Tom de Vries wrote:
>> On 10/24/22 18:36, Pedro Alves wrote:
>>> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
>>>
>>>> +#define RETHROW_ON_TARGET_CLOSE_ERROR(E)                \
>>>> +  do                                    \
>>>> +    {                                    \
>>>> +      if ((E).reason != 0)                        \
>>>> +    {                                \
>>>> +      /* Can't set the breakpoint.  */                \
>>>> +                                    \
>>>> +      if ((E).error == TARGET_CLOSE_ERROR)                \
>>>> +        /* If the target has closed then it will have deleted any    \
>>>> +           breakpoints inserted within the target inferior, as a    \
>>>> +           result any further attempts to interact with the        \
>>>> +           breakpoint objects is not possible.  Just rethrow the    \
>>>> +           error.  Don't use E to rethrow, to prevent object    \
>>>> +           slicing of the exception.  */                \
>>>> +        throw;                            \
>>>> +    }                                \
>>>> +    } while (0)
>>>
>>> Is there a reason this is a macro instead of a function?
>>
>> yes, the throw without expression.
>>
>> Do you know of a way to do this using a function?
> 
> WDYM?  I believe it should Just Work.  E.g.:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> #include <stdio.h>
> 
> struct excpt
> {
> };
> 
> struct excpt2 : excpt
> {
> };
> 
> void
> rethrow ()
> {
>    throw;
> }
> 
> int main ()
> {
>    try
>      {
>        try
> 	{
> 	  throw excpt2{};
> 	}
>        catch (const excpt &)
> 	{
> 	  rethrow ();
> 	}
>      }
>    catch (const excpt2 &)
>      {
>        printf ("caught excpt2\n");
>      }
>    return 0;
> }
> 
> $ g++ test_rethrow.cc -o test_rethrow
> $ ./test_rethrow
> caught excpt2
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Hmm, it does indeed, I didn't realize this was possible.

Fixed in attached patch.

Any further comments?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Rewrite-RETHROW_ON_TARGET_CLOSE_ERROR-into-funct.patch --]
[-- Type: text/x-patch, Size: 3359 bytes --]

From 633dfb9645a281bd1d69b63d046d2ed8408cb0d3 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 25 Oct 2022 08:17:46 +0200
Subject: [PATCH] [gdb] Rewrite RETHROW_ON_TARGET_CLOSE_ERROR into function

Recent commit b2829fcf9b5 ("[gdb] Fix rethrow exception slicing in
insert_bp_location") introduced macro RETHROW_ON_TARGET_CLOSE_ERROR.

I wrote this as a macro in order to have the rethrowing throw be part of the
same function as the catch, but as it turns out that not necessary.

Rewrite into a function.

Tested on x86_64-linux.
---
 gdb/breakpoint.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a001e78cfb4..30826032360 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2647,23 +2647,28 @@ breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr)
     return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr);
 }
 
-#define RETHROW_ON_TARGET_CLOSE_ERROR(E)				\
-  do									\
-    {									\
-      if ((E).reason != 0)						\
-	{								\
-	  /* Can't set the breakpoint.  */				\
-									\
-	  if ((E).error == TARGET_CLOSE_ERROR)				\
-	    /* If the target has closed then it will have deleted any	\
-	       breakpoints inserted within the target inferior, as a	\
-	       result any further attempts to interact with the		\
-	       breakpoint objects is not possible.  Just rethrow the	\
-	       error.  Don't use E to rethrow, to prevent object	\
-	       slicing of the exception.  */				\
-	    throw;							\
-	}								\
-    } while (0)
+/* Rethrow the currently handled exception, if it's a TARGET_CLOSE_ERROR.
+   E is either the currently handled exception, or a copy, or a sliced copy,
+   so we can't rethrow that one, but we can use it to inspect the properties
+   of the currently handled exception.  */
+
+static void
+rethrow_on_target_close_error (const gdb_exception &e)
+{
+  if (e.reason == 0)
+    return;
+  /* Can't set the breakpoint.  */
+
+  if (e.error != TARGET_CLOSE_ERROR)
+    return;
+
+  /* If the target has closed then it will have deleted any breakpoints
+     inserted within the target inferior, as a result any further attempts
+     to interact with the breakpoint objects is not possible.  Just rethrow
+     the error.  Don't use e to rethrow, to prevent object slicing of the
+     exception.  */
+  throw;
+}
 
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
@@ -2752,7 +2757,7 @@ insert_bp_location (struct bp_location *bl,
 	    }
 	  catch (gdb_exception &e)
 	    {
-	      RETHROW_ON_TARGET_CLOSE_ERROR (e);
+	      rethrow_on_target_close_error (e);
 	      bp_excpt = std::move (e);
 	    }
 	}
@@ -2792,7 +2797,7 @@ insert_bp_location (struct bp_location *bl,
 		    }
 		  catch (gdb_exception &e)
 		    {
-		      RETHROW_ON_TARGET_CLOSE_ERROR (e);
+		      rethrow_on_target_close_error (e);
 		      bp_excpt = std::move (e);
 		    }
 
@@ -2817,7 +2822,7 @@ insert_bp_location (struct bp_location *bl,
 		}
 	      catch (gdb_exception &e)
 		{
-		  RETHROW_ON_TARGET_CLOSE_ERROR (e);
+		  rethrow_on_target_close_error (e);
 		  bp_excpt = std::move (e);
 		}
 	    }

base-commit: c6d20401a20148b032cb7fb9dba079466a9383cc
-- 
2.35.3


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

* Re: [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location
  2022-10-25  7:14         ` Tom de Vries
@ 2022-10-25  9:26           ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2022-10-25  9:26 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2022-10-25 8:14 a.m., Tom de Vries wrote:
> On 10/24/22 18:51, Pedro Alves wrote:
>> On 2022-10-24 5:43 p.m., Tom de Vries wrote:
>>> On 10/24/22 18:36, Pedro Alves wrote:
>>>> On 2022-10-24 9:49 a.m., Tom de Vries via Gdb-patches wrote:
>>>> Is there a reason this is a macro instead of a function?
>>>
>>> yes, the throw without expression.
>>>
>>> Do you know of a way to do this using a function?
>>
>> WDYM?  I believe it should Just Work.  E.g.:

...

> Hmm, it does indeed, I didn't realize this was possible.
> 
> Fixed in attached patch.
> 
> Any further comments?

Nope, looks fine.

Thanks,
Pedro Alves

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

end of thread, other threads:[~2022-10-25  9:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-24  8:49 [PATCH 0/2] [gdb] Fix rethrow exception slicing Tom de Vries
2022-10-24  8:49 ` [PATCH 1/2] [gdb] Fix rethrow exception slicing in pretty_print_insn Tom de Vries
2022-10-24  8:49 ` [PATCH 2/2] [gdb] Fix rethrow exception slicing in insert_bp_location Tom de Vries
2022-10-24 16:36   ` Pedro Alves
2022-10-24 16:43     ` Tom de Vries
2022-10-24 16:48       ` Simon Marchi
2022-10-25  7:08         ` Tom de Vries
2022-10-24 16:51       ` Pedro Alves
2022-10-25  7:14         ` Tom de Vries
2022-10-25  9:26           ` Pedro Alves
2022-10-24 11:20 ` [PATCH 0/2] [gdb] Fix rethrow exception slicing Andrew Burgess

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).