diff mbox series

c++: Fix use-after-free of replaced friend instantiation [PR118807]

Message ID 67aa8494.170a0220.da597.54a5@mx.google.com
State New
Headers
Series c++: Fix use-after-free of replaced friend instantiation [PR118807] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Nathaniel Shead Feb. 10, 2025, 10:58 p.m. UTC 
  Bootstrapped and regtested on x86_64-pc-linux-gnu (and additionally
passed modules.exp with a checking=all build), OK for trunk?

-- >8 --

When instantiating a friend function, we call register_specialization
which adds it to the DECL_TEMPLATE_INSTANTIATIONS of the template.
However, in some circumstances we might immediately call pushdecl and
find an existing specialisation.  In this case, when reregistering the
specialisation we also need to update the DECL_TEMPLATE_INSTANTIATIONS
list so that we don't try to access the freed spec again later.

	PR c++/118807

gcc/cp/ChangeLog:

	* pt.cc (reregister_specialization): Remove spec from
	DECL_TEMPLATE_INSTANTIATIONS.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/pr118807.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/pt.cc                            | 11 +++++++++++
 gcc/testsuite/g++.dg/modules/pr118807.C | 11 +++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr118807.C

Comments

Jason Merrill Feb. 11, 2025, 9:34 a.m. UTC | #1 
On 2/10/25 11:58 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu (and additionally
> passed modules.exp with a checking=all build), OK for trunk?
> 
> -- >8 --
> 
> When instantiating a friend function, we call register_specialization
> which adds it to the DECL_TEMPLATE_INSTANTIATIONS of the template.
> However, in some circumstances we might immediately call pushdecl and
> find an existing specialisation.  In this case, when reregistering the
> specialisation we also need to update the DECL_TEMPLATE_INSTANTIATIONS
> list so that we don't try to access the freed spec again later.
> 
> 	PR c++/118807
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (reregister_specialization): Remove spec from
> 	DECL_TEMPLATE_INSTANTIATIONS.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/pr118807.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/pt.cc                            | 11 +++++++++++
>   gcc/testsuite/g++.dg/modules/pr118807.C | 11 +++++++++++
>   2 files changed, 22 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr118807.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 39232b5e67f..e1764743597 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1985,6 +1985,17 @@ reregister_specialization (tree spec, tree tinfo, tree new_spec)
>         gcc_assert (entry->spec == spec || entry->spec == new_spec);
>         gcc_assert (new_spec != NULL_TREE);
>         entry->spec = new_spec;
> +
> +      /* We need to also remove the old specialisation from

Let's say "spec" instead of "old specialisation", old and new are kind 
of confusing here since in duplicate_decls, new_spec is olddecl and spec 
is newdecl.

OK with that tweak.

> +	 DECL_TEMPLATE_INSTANTIATIONS if it was placed there.  */
> +      for (tree *inst = &DECL_TEMPLATE_INSTANTIATIONS (elt.tmpl);
> +	   *inst; inst = &TREE_CHAIN (*inst))
> +	if (TREE_VALUE (*inst) == spec)
> +	  {
> +	    *inst = TREE_CHAIN (*inst);
> +	    break;
> +	  }
> +
>         return 1;
>       }
>   
> diff --git a/gcc/testsuite/g++.dg/modules/pr118807.C b/gcc/testsuite/g++.dg/modules/pr118807.C
> new file mode 100644
> index 00000000000..a97afb92699
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr118807.C
> @@ -0,0 +1,11 @@
> +// PR c++/118807
> +// { dg-additional-options "-fmodules --param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -Wno-global-module" }
> +
> +module;
> +template <typename> class basic_streambuf;
> +template <typename> struct basic_streambuf {
> +  friend void __istream_extract();
> +};
> +template class basic_streambuf<char>;
> +template class basic_streambuf<wchar_t>;
> +export module M;
diff mbox series

Patch

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 39232b5e67f..e1764743597 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -1985,6 +1985,17 @@  reregister_specialization (tree spec, tree tinfo, tree new_spec)
       gcc_assert (entry->spec == spec || entry->spec == new_spec);
       gcc_assert (new_spec != NULL_TREE);
       entry->spec = new_spec;
+
+      /* We need to also remove the old specialisation from
+	 DECL_TEMPLATE_INSTANTIATIONS if it was placed there.  */
+      for (tree *inst = &DECL_TEMPLATE_INSTANTIATIONS (elt.tmpl);
+	   *inst; inst = &TREE_CHAIN (*inst))
+	if (TREE_VALUE (*inst) == spec)
+	  {
+	    *inst = TREE_CHAIN (*inst);
+	    break;
+	  }
+
       return 1;
     }
 
diff --git a/gcc/testsuite/g++.dg/modules/pr118807.C b/gcc/testsuite/g++.dg/modules/pr118807.C
new file mode 100644
index 00000000000..a97afb92699
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118807.C
@@ -0,0 +1,11 @@ 
+// PR c++/118807
+// { dg-additional-options "-fmodules --param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -Wno-global-module" }
+
+module;
+template <typename> class basic_streambuf;
+template <typename> struct basic_streambuf {
+  friend void __istream_extract();
+};
+template class basic_streambuf<char>;
+template class basic_streambuf<wchar_t>;
+export module M;