c++: Modularise start_cleanup_fn [PR98893]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
'start_cleanup_fn' is not currently viable in modules, due to generating
functions relying on the 'start_cleanup_cnt' counter which is reset to 0
with each new TU. This means that cleanup functions declared in a TU
will conflict with any imported cleanup functions.
This patch mitigates the problem by using the mangled name of the decl
we're destroying as part of the name of the function. This should avoid
clashes unless the decls would have clashed anyway.
PR c++/98893
gcc/cp/ChangeLog:
* decl.cc (start_cleanup_fn): Make name from the mangled name of
the passed-in decl.
(register_dtor_fn): Pass decl to start_cleanup_fn.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr98893_a.H: New test.
* g++.dg/modules/pr98893_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/decl.cc | 18 +++++++++---------
gcc/testsuite/g++.dg/modules/pr98893_a.H | 9 +++++++++
gcc/testsuite/g++.dg/modules/pr98893_b.C | 10 ++++++++++
3 files changed, 28 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr98893_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/pr98893_b.C
Comments
On 2/1/25 5:29 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
OK.
> -- >8 --
>
> 'start_cleanup_fn' is not currently viable in modules, due to generating
> functions relying on the 'start_cleanup_cnt' counter which is reset to 0
> with each new TU. This means that cleanup functions declared in a TU
> will conflict with any imported cleanup functions.
>
> This patch mitigates the problem by using the mangled name of the decl
> we're destroying as part of the name of the function. This should avoid
> clashes unless the decls would have clashed anyway.
>
> PR c++/98893
>
> gcc/cp/ChangeLog:
>
> * decl.cc (start_cleanup_fn): Make name from the mangled name of
> the passed-in decl.
> (register_dtor_fn): Pass decl to start_cleanup_fn.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/pr98893_a.H: New test.
> * g++.dg/modules/pr98893_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/decl.cc | 18 +++++++++---------
> gcc/testsuite/g++.dg/modules/pr98893_a.H | 9 +++++++++
> gcc/testsuite/g++.dg/modules/pr98893_b.C | 10 ++++++++++
> 3 files changed, 28 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/pr98893_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/pr98893_b.C
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index cf5e055e146..7219543823b 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -96,7 +96,7 @@ static void record_key_method_defined (tree);
> static tree create_array_type_for_decl (tree, tree, tree, location_t);
> static tree get_atexit_node (void);
> static tree get_dso_handle_node (void);
> -static tree start_cleanup_fn (bool);
> +static tree start_cleanup_fn (tree, bool);
> static void end_cleanup_fn (void);
> static tree cp_make_fname_decl (location_t, tree, int);
> static void initialize_predefined_identifiers (void);
> @@ -10373,23 +10373,23 @@ get_dso_handle_node (void)
> }
>
> /* Begin a new function with internal linkage whose job will be simply
> - to destroy some particular variable. OB_PARM is true if object pointer
> + to destroy some particular DECL. OB_PARM is true if object pointer
> is passed to the cleanup function, otherwise no argument is passed. */
>
> -static GTY(()) int start_cleanup_cnt;
> -
> static tree
> -start_cleanup_fn (bool ob_parm)
> +start_cleanup_fn (tree decl, bool ob_parm)
> {
> - char name[32];
> -
> push_to_top_level ();
>
> /* No need to mangle this. */
> push_lang_context (lang_name_c);
>
> /* Build the name of the function. */
> - sprintf (name, "__tcf_%d", start_cleanup_cnt++);
> + gcc_checking_assert (HAS_DECL_ASSEMBLER_NAME_P (decl));
> + const char *dname = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> + dname = targetm.strip_name_encoding (dname);
> + char *name = ACONCAT (("__tcf", dname, NULL));
> +
> tree fntype = TREE_TYPE (ob_parm ? get_cxa_atexit_fn_ptr_type ()
> : get_atexit_fn_ptr_type ());
> /* Build the function declaration. */
> @@ -10482,7 +10482,7 @@ register_dtor_fn (tree decl)
> build_cleanup (decl);
>
> /* Now start the function. */
> - cleanup = start_cleanup_fn (ob_parm);
> + cleanup = start_cleanup_fn (decl, ob_parm);
>
> /* Now, recompute the cleanup. It may contain SAVE_EXPRs that refer
> to the original function, rather than the anonymous one. That
> diff --git a/gcc/testsuite/g++.dg/modules/pr98893_a.H b/gcc/testsuite/g++.dg/modules/pr98893_a.H
> new file mode 100644
> index 00000000000..062ab6d9ccc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr98893_a.H
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +struct S {
> + ~S() {}
> +};
> +inline void foo() {
> + static S a[1];
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/pr98893_b.C b/gcc/testsuite/g++.dg/modules/pr98893_b.C
> new file mode 100644
> index 00000000000..9065589bdfb
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr98893_b.C
> @@ -0,0 +1,10 @@
> +// { dg-additional-options "-fmodules" }
> +
> +import "pr98893_a.H";
> +static S b[1];
> +int main() {
> + foo();
> +}
> +
> +// { dg-final { scan-assembler {__tcf_ZZ3foovE1a:} } }
> +// { dg-final { scan-assembler {__tcf_ZL1b:} } }
@@ -96,7 +96,7 @@ static void record_key_method_defined (tree);
static tree create_array_type_for_decl (tree, tree, tree, location_t);
static tree get_atexit_node (void);
static tree get_dso_handle_node (void);
-static tree start_cleanup_fn (bool);
+static tree start_cleanup_fn (tree, bool);
static void end_cleanup_fn (void);
static tree cp_make_fname_decl (location_t, tree, int);
static void initialize_predefined_identifiers (void);
@@ -10373,23 +10373,23 @@ get_dso_handle_node (void)
}
/* Begin a new function with internal linkage whose job will be simply
- to destroy some particular variable. OB_PARM is true if object pointer
+ to destroy some particular DECL. OB_PARM is true if object pointer
is passed to the cleanup function, otherwise no argument is passed. */
-static GTY(()) int start_cleanup_cnt;
-
static tree
-start_cleanup_fn (bool ob_parm)
+start_cleanup_fn (tree decl, bool ob_parm)
{
- char name[32];
-
push_to_top_level ();
/* No need to mangle this. */
push_lang_context (lang_name_c);
/* Build the name of the function. */
- sprintf (name, "__tcf_%d", start_cleanup_cnt++);
+ gcc_checking_assert (HAS_DECL_ASSEMBLER_NAME_P (decl));
+ const char *dname = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
+ dname = targetm.strip_name_encoding (dname);
+ char *name = ACONCAT (("__tcf", dname, NULL));
+
tree fntype = TREE_TYPE (ob_parm ? get_cxa_atexit_fn_ptr_type ()
: get_atexit_fn_ptr_type ());
/* Build the function declaration. */
@@ -10482,7 +10482,7 @@ register_dtor_fn (tree decl)
build_cleanup (decl);
/* Now start the function. */
- cleanup = start_cleanup_fn (ob_parm);
+ cleanup = start_cleanup_fn (decl, ob_parm);
/* Now, recompute the cleanup. It may contain SAVE_EXPRs that refer
to the original function, rather than the anonymous one. That
new file mode 100644
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+struct S {
+ ~S() {}
+};
+inline void foo() {
+ static S a[1];
+}
new file mode 100644
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules" }
+
+import "pr98893_a.H";
+static S b[1];
+int main() {
+ foo();
+}
+
+// { dg-final { scan-assembler {__tcf_ZZ3foovE1a:} } }
+// { dg-final { scan-assembler {__tcf_ZL1b:} } }