On Fri, 2023-11-17 at 17:48 -0500, Antoni Boucher wrote:
> Hi.
> This adds functions to set the personality function (bug 112603).
>
> I'm not sure I can make a test for this: it seems the personality
> function will not be set if there are no try/catch inside the
> functions.
> Do you know a way to keep the personality function that is set in
> this
> case?
>
> Or should we wait until I send the patch for try/catch?
Thanks for the patch
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 556bcf7ef59..25d50289b24 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -13559,6 +13559,14 @@ build_personality_function (const char *lang)
>
> name = ACONCAT (("__", lang, "_personality", unwind_and_version, NULL));
>
> + return build_personality_function_with_name (name);
> +}
> +
> +tree
> +build_personality_function_with_name (const char *name)
> +{
> + tree decl, type;
> +
> type = build_function_type_list (unsigned_type_node,
> integer_type_node, integer_type_node,
> long_long_unsigned_type_node,
I confess I'm not very familiar with personalities, sorry; hopefully a
reviewer who is can take a look at the non-jit parts of the patch,
though FWIW they look fairly trivial.
> diff --git a/gcc/jit/docs/topics/compatibility.rst
b/gcc/jit/docs/topics/compatibility.rst
> index ebede440ee4..31c3ef6401a 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -378,3 +378,13 @@ alignment of a variable:
> --------------------
> ``LIBGCCJIT_ABI_25`` covers the addition of
> :func:`gcc_jit_type_get_restrict`
> +
> +.. _LIBGCCJIT_ABI_26:
> +
> +``LIBGCCJIT_ABI_26``
> +--------------------
> +``LIBGCCJIT_ABI_26`` covers the addition of functions to set the personality
> +function:
> +
> + * :func:`gcc_jit_function_set_personality_function`
> + * :func:`gcc_jit_set_global_personality_function_name`
Obviously you're going to need to update the number if the other patch
goes in first.
> diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
> index cf5cb716daf..e59885c3549 100644
> --- a/gcc/jit/docs/topics/functions.rst
> +++ b/gcc/jit/docs/topics/functions.rst
> @@ -197,6 +197,34 @@ Functions
>
> .. type:: gcc_jit_case
>
> +.. function:: void
> + gcc_jit_function_set_personality_function (gcc_jit_function *fn,
> + gcc_jit_function *personality_func)
> +
> + Set the personality function of ``fn`` to ``personality_func``.
> +
> + were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presence
> + using
Likewise here.
Is there a URL in the main GCC docs we can link to that describes what
this is for?
Are there restrictions on what a personality_func is? Sorry for my
ignorance here.
> +
> + .. code-block:: c
> +
> + #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION
> +
> +.. function:: void
> + gcc_jit_set_global_personality_function_name (char* name)
> +
> + Set the global personality function.
> +
> + This is mainly useful to prevent the optimizer from unsetting the personality
> + function set on other functions.
> +
> + were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presence
> + using
Likewise here: ABI numbering, and a URL for more info on what this is.
> +
> + .. code-block:: c
> +
> + #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION
> +
> Blocks
> ------
> .. type:: gcc_jit_block
> diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
> index a729086bafb..c9dedd59b24 100644
> --- a/gcc/jit/dummy-frontend.cc
> +++ b/gcc/jit/dummy-frontend.cc
> @@ -146,6 +146,20 @@ const struct attribute_spec jit_format_attribute_table[] =
> { NULL, 0, 0, false, false, false, false, NULL, NULL }
> };
>
> +char* jit_personality_func_name = NULL;
> +static tree personality_decl;
> +
> +/* FIXME: This is a hack to preserve trees that we create from the
> + garbage collector. */
> +
> +static GTY (()) tree jit_gc_root;
> +
> +void
> +jit_preserve_from_gc (tree t)
> +{
> + jit_gc_root = tree_cons (NULL_TREE, t, jit_gc_root);
> +}
If I'm reading the patch correctly, this is only used by
jit_langhook_eh_personality, to preserve personality_decl.
Can't you just add a GTY (()) marker to personality_decl's decl
instead, as:
static GTY (()) tree personality_decl;
[...]
> diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> index 0451b4df7f9..67d9036249e 100644
> --- a/gcc/jit/libgccjit.cc
> +++ b/gcc/jit/libgccjit.cc
> @@ -3590,6 +3590,28 @@ gcc_jit_context_add_command_line_option (gcc_jit_context *ctxt,
> ctxt->add_command_line_option (optname);
> }
>
> +/* Public entrypoint. See description in libgccjit.h.
> + After error-checking, the real work is done by the
> + gcc::jit::recording::function::set_personality_function method, in
> + jit-recording.c. */
> +
> +void
> +gcc_jit_function_set_personality_function (gcc_jit_function *fn,
> + gcc_jit_function *personality_func)
> +{
> + RETURN_IF_FAIL (fn, NULL, NULL, "NULL function");
I see various unconditional dereferences of m_personality_function, so
am I right in assuming that personality_function must be non-NULL? If
so we should document that, and reject NULL here.
> +
> + fn->set_personality_function (personality_func);
> +}
> +
> +extern char* jit_personality_func_name;
> +
> +void
> +gcc_jit_set_global_personality_function_name (char* name)
> +{
> + jit_personality_func_name = name;
This probably should be per-context state, rather than a global
variable. So I think this needs a gcc_jit_context * param, which must
be non-null, and we'd have a new field m_personality_func_name of the
recording::context, rather than the global.
Also everywhere else in the API we take a copy of the string, rather
than via a pointer. Is there a reason for doing this here? Otherwise,
this param should be a const char *, and we should xstrdup it (and free
any existing value).
[...]
> diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> index 8b90a0e2ff3..2a0eb819a52 100644
> --- a/gcc/jit/libgccjit.map
> +++ b/gcc/jit/libgccjit.map
> @@ -276,3 +276,9 @@ LIBGCCJIT_ABI_25 {
> global:
> gcc_jit_type_get_restrict;
> } LIBGCCJIT_ABI_24;
> +
> +LIBGCCJIT_ABI_26 {
> + global:
> + gcc_jit_set_global_personality_function_name;
> + gcc_jit_function_set_personality_function;
> +} LIBGCCJIT_ABI_25;
Same note as above about ABI numbering.
> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> index e762563f9bd..3785a788ffa 100644
> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> @@ -322,6 +322,9 @@
> /* test-setting-alignment.c: This can't be in the testcases array as it
> is target-specific. */
>
> +/* test-personality-function.c: This can't be in the testcases array as it
> + is target-specific. */
...and because it modifies per-context state.
[...]
Hope the above makes sense; sorry again about my ignorance of the
underlying personality stuff.
Dave
From 6beb6452c7bac9ecbdaea750d61d6e6c6bd3ed8f Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sun, 16 Apr 2023 13:19:20 -0400
Subject: [PATCH] libgccjit: Add ways to set the personality function
gcc/ChangeLog:
PR jit/112603
* expr.cc (build_personality_function_with_name): New function.
* tree.cc (tree_cc_finalize): Cleanup gcc_eh_personality_decl.
* tree.h (build_personality_function_with_name): New decl.
gcc/jit/ChangeLog:
PR jit/112603
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_26): New ABI tag.
* docs/topics/functions.rst: Document the functions
gcc_jit_set_global_personality_function_name and
gcc_jit_function_set_personality_function.
* dummy-frontend.cc (jit_gc_root): New variable.
(jit_preserve_from_gc): New function.
(jit_langhook_init): Initialize new variables.
(jit_langhook_eh_personality): New hook.
(LANG_HOOKS_EH_PERSONALITY): New hook.
* jit-playback.cc (set_personality_function): New function.
* jit-playback.h: New decl.
* jit-recording.cc
(memento_of_set_personality_function::make_debug_string,
recording::memento_of_set_personality_function::write_reproducer,
recording::function::set_personality_function,
recording::memento_of_set_personality_function::replay_into):
New functions
* jit-recording.h (class memento_of_set_personality_function):
New class.
(recording::function::set_personality_function): New function.
* libgccjit.cc (gcc_jit_function_set_personality_function,
gcc_jit_set_global_personality_function_name): New functions.
* libgccjit.h (gcc_jit_set_global_personality_function_name,
gcc_jit_function_set_personality_function): New functions.
* libgccjit.map: New functions.
gcc/testsuite/ChangeLog:
* jit.dg/test-personality-function.c: New test.
* jit.dg/all-non-failing-tests.h: Mention
test-personality-function.c.
---
gcc/expr.cc | 8 +++
gcc/jit/docs/topics/compatibility.rst | 10 ++++
gcc/jit/docs/topics/functions.rst | 28 ++++++++++
gcc/jit/dummy-frontend.cc | 36 ++++++++++++
gcc/jit/jit-playback.cc | 8 +++
gcc/jit/jit-playback.h | 3 +
gcc/jit/jit-recording.cc | 44 +++++++++++++++
gcc/jit/jit-recording.h | 23 ++++++++
gcc/jit/libgccjit.cc | 22 ++++++++
gcc/jit/libgccjit.h | 8 +++
gcc/jit/libgccjit.map | 6 ++
gcc/testsuite/jit.dg/all-non-failing-tests.h | 3 +
.../jit.dg/test-personality-function.c | 55 +++++++++++++++++++
gcc/tree.cc | 1 +
gcc/tree.h | 1 +
15 files changed, 256 insertions(+)
create mode 100644 gcc/testsuite/jit.dg/test-personality-function.c
@@ -13559,6 +13559,14 @@ build_personality_function (const char *lang)
name = ACONCAT (("__", lang, "_personality", unwind_and_version, NULL));
+ return build_personality_function_with_name (name);
+}
+
+tree
+build_personality_function_with_name (const char *name)
+{
+ tree decl, type;
+
type = build_function_type_list (unsigned_type_node,
integer_type_node, integer_type_node,
long_long_unsigned_type_node,
@@ -378,3 +378,13 @@ alignment of a variable:
--------------------
``LIBGCCJIT_ABI_25`` covers the addition of
:func:`gcc_jit_type_get_restrict`
+
+.. _LIBGCCJIT_ABI_26:
+
+``LIBGCCJIT_ABI_26``
+--------------------
+``LIBGCCJIT_ABI_26`` covers the addition of functions to set the personality
+function:
+
+ * :func:`gcc_jit_function_set_personality_function`
+ * :func:`gcc_jit_set_global_personality_function_name`
@@ -197,6 +197,34 @@ Functions
.. type:: gcc_jit_case
+.. function:: void
+ gcc_jit_function_set_personality_function (gcc_jit_function *fn,
+ gcc_jit_function *personality_func)
+
+ Set the personality function of ``fn`` to ``personality_func``.
+
+ were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presence
+ using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION
+
+.. function:: void
+ gcc_jit_set_global_personality_function_name (char* name)
+
+ Set the global personality function.
+
+ This is mainly useful to prevent the optimizer from unsetting the personality
+ function set on other functions.
+
+ were added in :ref:`LIBGCCJIT_ABI_26`; you can test for their presence
+ using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_PERSONALITY_FUNCTION
+
Blocks
------
.. type:: gcc_jit_block
@@ -146,6 +146,20 @@ const struct attribute_spec jit_format_attribute_table[] =
{ NULL, 0, 0, false, false, false, false, NULL, NULL }
};
+char* jit_personality_func_name = NULL;
+static tree personality_decl;
+
+/* FIXME: This is a hack to preserve trees that we create from the
+ garbage collector. */
+
+static GTY (()) tree jit_gc_root;
+
+void
+jit_preserve_from_gc (tree t)
+{
+ jit_gc_root = tree_cons (NULL_TREE, t, jit_gc_root);
+}
+
/* Attribute handlers. */
/* Handle a "noreturn" attribute; arguments as in
@@ -578,6 +592,8 @@ jit_end_diagnostic (diagnostic_context *context,
static bool
jit_langhook_init (void)
{
+ jit_gc_root = NULL_TREE;
+ personality_decl = NULL_TREE;
gcc_assert (gcc::jit::active_playback_ctxt);
JIT_LOG_SCOPE (gcc::jit::active_playback_ctxt->get_logger ());
@@ -694,6 +710,26 @@ jit_langhook_getdecls (void)
return NULL;
}
+static tree
+jit_langhook_eh_personality (void)
+{
+ if (personality_decl == NULL_TREE)
+ {
+ if (jit_personality_func_name != NULL)
+ {
+ personality_decl =
+ build_personality_function_with_name (jit_personality_func_name);
+ jit_preserve_from_gc (personality_decl);
+ }
+ else
+ return lhd_gcc_personality ();
+ }
+ return personality_decl;
+}
+
+#undef LANG_HOOKS_EH_PERSONALITY
+#define LANG_HOOKS_EH_PERSONALITY jit_langhook_eh_personality
+
#undef LANG_HOOKS_NAME
#define LANG_HOOKS_NAME "libgccjit"
@@ -1857,6 +1857,14 @@ playback::function::get_address (location *loc)
return new rvalue (m_ctxt, t_fnptr);
}
+void
+playback::function::
+set_personality_function (function *personality_function)
+{
+ DECL_FUNCTION_PERSONALITY (m_inner_fndecl) =
+ personality_function->as_fndecl ();
+}
+
/* Build a statement list for the function as a whole out of the
lists of statements for the individual blocks, building labels
for each block. */
@@ -508,6 +508,9 @@ public:
rvalue *
get_address (location *loc);
+ void
+ set_personality_function (function *personality_function);
+
void
build_stmt_list ();
@@ -4164,6 +4164,50 @@ recording::function::replay_into (replayer *r)
m_builtin_id));
}
+/* Implementation of recording::memento::make_debug_string for
+ setting a personality function. */
+
+recording::string *
+recording::memento_of_set_personality_function::make_debug_string ()
+{
+ return string::from_printf (m_ctxt,
+ "%s",
+ m_personality_function->get_debug_string ());
+}
+
+/* Implementation of recording::memento::write_reproducer for setting the
+ personality function. */
+
+void
+recording::memento_of_set_personality_function::write_reproducer (reproducer &r)
+{
+ r.write (" gcc_jit_function_set_personality_function (%s,\n"
+ " %s);\n",
+ r.get_identifier (m_function),
+ r.get_identifier (m_personality_function));
+}
+
+void
+recording::function::set_personality_function (function *function)
+{
+ recording::memento_of_set_personality_function *result =
+ new memento_of_set_personality_function (m_ctxt, this, function);
+ m_ctxt->record (result);
+}
+
+/* The implementation of class
+ gcc::jit::recording::memento_of_set_personality_function. */
+
+/* Implementation of pure virtual hook recording::memento::replay_into
+ for recording::memento_of_set_personality_function. */
+
+void
+recording::memento_of_set_personality_function::replay_into (replayer *)
+{
+ m_function->playback_function ()->set_personality_function (
+ m_personality_function->playback_function ());
+}
+
/* Create a recording::local instance and add it to
the functions's context's list of mementos, and to the function's
list of locals.
@@ -1342,6 +1342,8 @@ public:
rvalue *get_address (location *loc);
+ void set_personality_function (function *function);
+
private:
string * make_debug_string () final override;
void write_reproducer (reproducer &r) final override;
@@ -1359,6 +1361,27 @@ private:
type *m_fn_ptr_type;
};
+class memento_of_set_personality_function : public memento
+{
+public:
+ memento_of_set_personality_function (context *ctx,
+ function *func,
+ function *personality_function)
+ : memento (ctx),
+ m_function (func),
+ m_personality_function (personality_function) {}
+
+ void replay_into (replayer *r) final override;
+
+private:
+ string * make_debug_string () final override;
+ void write_reproducer (reproducer &r) final override;
+
+private:
+ function *m_function;
+ function *m_personality_function;
+};
+
class block : public memento
{
public:
@@ -3590,6 +3590,28 @@ gcc_jit_context_add_command_line_option (gcc_jit_context *ctxt,
ctxt->add_command_line_option (optname);
}
+/* Public entrypoint. See description in libgccjit.h.
+ After error-checking, the real work is done by the
+ gcc::jit::recording::function::set_personality_function method, in
+ jit-recording.c. */
+
+void
+gcc_jit_function_set_personality_function (gcc_jit_function *fn,
+ gcc_jit_function *personality_func)
+{
+ RETURN_IF_FAIL (fn, NULL, NULL, "NULL function");
+
+ fn->set_personality_function (personality_func);
+}
+
+extern char* jit_personality_func_name;
+
+void
+gcc_jit_set_global_personality_function_name (char* name)
+{
+ jit_personality_func_name = name;
+}
+
/* Public entrypoint. See description in libgccjit.h.
The real work is done by the
@@ -1797,6 +1797,14 @@ extern gcc_jit_rvalue *
gcc_jit_function_get_address (gcc_jit_function *fn,
gcc_jit_location *loc);
+#define LIBGCCJIT_HAVE_PERSONALITY_FUNCTION
+
+extern void
+gcc_jit_set_global_personality_function_name (char* name);
+
+void
+gcc_jit_function_set_personality_function (gcc_jit_function *fn,
+ gcc_jit_function *personality_func);
#define LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_vector
@@ -276,3 +276,9 @@ LIBGCCJIT_ABI_25 {
global:
gcc_jit_type_get_restrict;
} LIBGCCJIT_ABI_24;
+
+LIBGCCJIT_ABI_26 {
+ global:
+ gcc_jit_set_global_personality_function_name;
+ gcc_jit_function_set_personality_function;
+} LIBGCCJIT_ABI_25;
@@ -322,6 +322,9 @@
/* test-setting-alignment.c: This can't be in the testcases array as it
is target-specific. */
+/* test-personality-function.c: This can't be in the testcases array as it
+ is target-specific. */
+
/* test-string-literal.c */
#define create_code create_code_string_literal
#define verify_code verify_code_string_literal
new file mode 100644
@@ -0,0 +1,55 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+/* We don't want set_options() in harness.h to set -O3 so our little local
+ is optimized away. */
+#define TEST_ESCHEWS_SET_OPTIONS
+static void set_options (gcc_jit_context *ctxt, const char *argv0)
+{
+}
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME "output-of-test-personality-function.c.s"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ gcc_jit_context_add_driver_option (ctxt, "-fexceptions");
+ gcc_jit_context_add_command_line_option (ctxt, "-fexceptions");
+ gcc_jit_set_global_personality_function_name ("my_personality_function");
+
+ gcc_jit_type *void_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+
+ gcc_jit_function *personality_func =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ void_type,
+ "my_personality_function",
+ 0, NULL,
+ 0);
+ gcc_jit_block *block_personality = gcc_jit_function_new_block (personality_func, NULL);
+ gcc_jit_block_end_with_void_return (block_personality, NULL);
+
+ gcc_jit_function *func_a =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ void_type,
+ "a",
+ 0, NULL,
+ 0);
+
+
+ gcc_jit_block *block_a = gcc_jit_function_new_block (func_a, NULL);
+ gcc_jit_block_end_with_void_return (block_a, NULL);
+ gcc_jit_function_set_personality_function (func_a, personality_func);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output ".cfi_personality TODO" } } */
@@ -15115,6 +15115,7 @@ void
tree_cc_finalize (void)
{
clear_nonstandard_integer_type_cache ();
+ gcc_eh_personality_decl = NULL;
vec_free (bitint_type_cache);
}
@@ -6672,6 +6672,7 @@ extern tree get_inner_reference (tree, poly_int64 *, poly_int64 *,
tree *, machine_mode *, int *, int *, int *);
extern tree build_personality_function (const char *);
+extern tree build_personality_function_with_name (const char *);
struct GTY(()) int_n_trees_t {
/* These parts are initialized at runtime */
--
2.42.1