handle unwind tables that are embedded within unwinding code, [PR111731]
Checks
Commit Message
Original bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111731
Given that this is a regression, is this okay for gcc 13 and mainline?
The unwinding mechanism registers both the code range and the unwind
table itself within a b-tree lookup structure. That data structure
assumes that is consists of non-overlappping intervals. This
becomes a problem if the unwinding table is embedded within the
code itself, as now the intervals do overlap.
To fix this problem we now keep the unwind tables in a separate
b-tree, which prevents the overlap.
libgcc/ChangeLog:
PR libgcc/111731
* unwind-dw2-fde.c: Split unwind ranges if they contain the
unwind table.
---
libgcc/unwind-dw2-fde.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
Comments
On Fri, Mar 15, 2024 at 11:31 AM Thomas Neumann
<thomas.neumann@in.tum.de> wrote:
>
> Original bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111731
> Given that this is a regression, is this okay for gcc 13 and mainline?
It does look straightforward but I hope Jason or Florian can provide the ACK.
Thanks,
Richard.
> The unwinding mechanism registers both the code range and the unwind
> table itself within a b-tree lookup structure. That data structure
> assumes that is consists of non-overlappping intervals. This
> becomes a problem if the unwinding table is embedded within the
> code itself, as now the intervals do overlap.
>
> To fix this problem we now keep the unwind tables in a separate
> b-tree, which prevents the overlap.
>
> libgcc/ChangeLog:
> PR libgcc/111731
> * unwind-dw2-fde.c: Split unwind ranges if they contain the
> unwind table.
> ---
> libgcc/unwind-dw2-fde.c | 37 +++++++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c
> index 61a578d097e..9d503545677 100644
> --- a/libgcc/unwind-dw2-fde.c
> +++ b/libgcc/unwind-dw2-fde.c
> @@ -48,6 +48,7 @@ typedef __UINTPTR_TYPE__ uintptr_type;
> #include "unwind-dw2-btree.h"
>
> static struct btree registered_frames;
> +static struct btree registered_objects;
> static bool in_shutdown;
>
> static void
> @@ -58,6 +59,7 @@ release_registered_frames (void)
> /* Release the b-tree and all frames. Frame releases that happen later are
> * silently ignored */
> btree_destroy (®istered_frames);
> + btree_destroy (®istered_objects);
> in_shutdown = true;
> }
>
> @@ -103,6 +105,21 @@ static __gthread_mutex_t object_mutex;
> #endif
> #endif
>
> +#ifdef ATOMIC_FDE_FAST_PATH
> +// Register the pc range for a given object in the lookup structure.
> +static void
> +register_pc_range_for_object (uintptr_type begin, struct object *ob)
> +{
> + // Register the object itself to know the base pointer on deregistration.
> + btree_insert (®istered_objects, begin, 1, ob);
> +
> + // Register the frame in the b-tree
> + uintptr_type range[2];
> + get_pc_range (ob, range);
> + btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
> +}
> +#endif
> +
> /* Called from crtbegin.o to register the unwind info for an object. */
>
> void
> @@ -124,13 +141,7 @@ __register_frame_info_bases (const void *begin, struct object *ob,
> #endif
>
> #ifdef ATOMIC_FDE_FAST_PATH
> - // Register the object itself to know the base pointer on deregistration.
> - btree_insert (®istered_frames, (uintptr_type) begin, 1, ob);
> -
> - // Register the frame in the b-tree
> - uintptr_type range[2];
> - get_pc_range (ob, range);
> - btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
> + register_pc_range_for_object ((uintptr_type) begin, ob);
> #else
> init_object_mutex_once ();
> __gthread_mutex_lock (&object_mutex);
> @@ -178,13 +189,7 @@ __register_frame_info_table_bases (void *begin, struct object *ob,
> ob->s.b.encoding = DW_EH_PE_omit;
>
> #ifdef ATOMIC_FDE_FAST_PATH
> - // Register the object itself to know the base pointer on deregistration.
> - btree_insert (®istered_frames, (uintptr_type) begin, 1, ob);
> -
> - // Register the frame in the b-tree
> - uintptr_type range[2];
> - get_pc_range (ob, range);
> - btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
> + register_pc_range_for_object ((uintptr_type) begin, ob);
> #else
> init_object_mutex_once ();
> __gthread_mutex_lock (&object_mutex);
> @@ -232,7 +237,7 @@ __deregister_frame_info_bases (const void *begin)
>
> #ifdef ATOMIC_FDE_FAST_PATH
> // Find the originally registered object to get the base pointer.
> - ob = btree_remove (®istered_frames, (uintptr_type) begin);
> + ob = btree_remove (®istered_objects, (uintptr_type) begin);
>
> // Remove the corresponding PC range.
> if (ob)
> @@ -240,7 +245,7 @@ __deregister_frame_info_bases (const void *begin)
> uintptr_type range[2];
> get_pc_range (ob, range);
> if (range[0] != range[1])
> - btree_remove (®istered_frames, range[0]);
> + btree_remove (®istered_frames, range[0]);
> }
>
> // Deallocate the sort array if any.
> --
> 2.43.0
>
On 3/15/24 4:29 AM, Thomas Neumann wrote:
> Original bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111731
> Given that this is a regression, is this okay for gcc 13 and mainline?
>
> The unwinding mechanism registers both the code range and the unwind
> table itself within a b-tree lookup structure. That data structure
> assumes that is consists of non-overlappping intervals. This
> becomes a problem if the unwinding table is embedded within the
> code itself, as now the intervals do overlap.
>
> To fix this problem we now keep the unwind tables in a separate
> b-tree, which prevents the overlap.
>
> libgcc/ChangeLog:
> PR libgcc/111731
> * unwind-dw2-fde.c: Split unwind ranges if they contain the
> unwind table.
I'll go ahead and give the final OK here :-)
Jeff
On 3/15/24 4:29 AM, Thomas Neumann wrote:
> Original bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111731
> Given that this is a regression, is this okay for gcc 13 and mainline?
>
> The unwinding mechanism registers both the code range and the unwind
> table itself within a b-tree lookup structure. That data structure
> assumes that is consists of non-overlappping intervals. This
> becomes a problem if the unwinding table is embedded within the
> code itself, as now the intervals do overlap.
>
> To fix this problem we now keep the unwind tables in a separate
> b-tree, which prevents the overlap.
>
> libgcc/ChangeLog:
> PR libgcc/111731
> * unwind-dw2-fde.c: Split unwind ranges if they contain the
> unwind table.
And what I'd suggest is committing to the trunk now, then waiting a week
or two before backporting to gcc-13.
jeff
>> libgcc/ChangeLog:
>> PR libgcc/111731
>> * unwind-dw2-fde.c: Split unwind ranges if they contain the
>> unwind table.
> And what I'd suggest is committing to the trunk now, then waiting a week
> or two before backporting to gcc-13.
I will do that, thanks for looking at the patch.
Best
Thomas
@@ -48,6 +48,7 @@ typedef __UINTPTR_TYPE__ uintptr_type;
#include "unwind-dw2-btree.h"
static struct btree registered_frames;
+static struct btree registered_objects;
static bool in_shutdown;
static void
@@ -58,6 +59,7 @@ release_registered_frames (void)
/* Release the b-tree and all frames. Frame releases that happen later are
* silently ignored */
btree_destroy (®istered_frames);
+ btree_destroy (®istered_objects);
in_shutdown = true;
}
@@ -103,6 +105,21 @@ static __gthread_mutex_t object_mutex;
#endif
#endif
+#ifdef ATOMIC_FDE_FAST_PATH
+// Register the pc range for a given object in the lookup structure.
+static void
+register_pc_range_for_object (uintptr_type begin, struct object *ob)
+{
+ // Register the object itself to know the base pointer on deregistration.
+ btree_insert (®istered_objects, begin, 1, ob);
+
+ // Register the frame in the b-tree
+ uintptr_type range[2];
+ get_pc_range (ob, range);
+ btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
+}
+#endif
+
/* Called from crtbegin.o to register the unwind info for an object. */
void
@@ -124,13 +141,7 @@ __register_frame_info_bases (const void *begin, struct object *ob,
#endif
#ifdef ATOMIC_FDE_FAST_PATH
- // Register the object itself to know the base pointer on deregistration.
- btree_insert (®istered_frames, (uintptr_type) begin, 1, ob);
-
- // Register the frame in the b-tree
- uintptr_type range[2];
- get_pc_range (ob, range);
- btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
+ register_pc_range_for_object ((uintptr_type) begin, ob);
#else
init_object_mutex_once ();
__gthread_mutex_lock (&object_mutex);
@@ -178,13 +189,7 @@ __register_frame_info_table_bases (void *begin, struct object *ob,
ob->s.b.encoding = DW_EH_PE_omit;
#ifdef ATOMIC_FDE_FAST_PATH
- // Register the object itself to know the base pointer on deregistration.
- btree_insert (®istered_frames, (uintptr_type) begin, 1, ob);
-
- // Register the frame in the b-tree
- uintptr_type range[2];
- get_pc_range (ob, range);
- btree_insert (®istered_frames, range[0], range[1] - range[0], ob);
+ register_pc_range_for_object ((uintptr_type) begin, ob);
#else
init_object_mutex_once ();
__gthread_mutex_lock (&object_mutex);
@@ -232,7 +237,7 @@ __deregister_frame_info_bases (const void *begin)
#ifdef ATOMIC_FDE_FAST_PATH
// Find the originally registered object to get the base pointer.
- ob = btree_remove (®istered_frames, (uintptr_type) begin);
+ ob = btree_remove (®istered_objects, (uintptr_type) begin);
// Remove the corresponding PC range.
if (ob)
@@ -240,7 +245,7 @@ __deregister_frame_info_bases (const void *begin)
uintptr_type range[2];
get_pc_range (ob, range);
if (range[0] != range[1])
- btree_remove (®istered_frames, range[0]);
+ btree_remove (®istered_frames, range[0]);
}
// Deallocate the sort array if any.