diff mbox series

c++/modules: Propagate BINDING_VECTOR_*_DUPS_P on realloc [PR99242]

Message ID 668bdbd0.630a0220.b384a.4b89@mx.google.com
State New
Headers show
Series c++/modules: Propagate BINDING_VECTOR_*_DUPS_P on realloc [PR99242] | expand

Commit Message

Nathaniel Shead July 8, 2024, 12:30 p.m. UTC 
Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk/14 if full regtest succeeds?

-- >8 --

When importing modules, when a binding vector for a name runs out of
slots it gets reallocated with a larger size, and existing bindings are
copied across.  However, the flags to indicate whether deduping needs to
occur did not: this causes ICEs, as it allows a duplicate binding to be
added which then violates assumptions later on.

	PR c++/99242

gcc/cp/ChangeLog:

	* name-lookup.cc (append_imported_binding_slot): Propagate dups
	flags.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/pr99242_a.H: New test.
	* g++.dg/modules/pr99242_b.H: New test.
	* g++.dg/modules/pr99242_c.H: New test.
	* g++.dg/modules/pr99242_d.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/name-lookup.cc                    | 4 ++++
 gcc/testsuite/g++.dg/modules/pr99242_a.H | 3 +++
 gcc/testsuite/g++.dg/modules/pr99242_b.H | 3 +++
 gcc/testsuite/g++.dg/modules/pr99242_c.H | 3 +++
 gcc/testsuite/g++.dg/modules/pr99242_d.C | 7 +++++++
 5 files changed, 20 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_b.H
 create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_c.H
 create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_d.C

Comments

Patrick Palka July 12, 2024, 5:55 p.m. UTC | #1 
On Mon, 8 Jul 2024, Nathaniel Shead wrote:

> Bootstrapped and regtested (so far just modules.exp) on
> x86_64-pc-linux-gnu, OK for trunk/14 if full regtest succeeds?
> 
> -- >8 --
> 
> When importing modules, when a binding vector for a name runs out of
> slots it gets reallocated with a larger size, and existing bindings are
> copied across.  However, the flags to indicate whether deduping needs to
> occur did not: this causes ICEs, as it allows a duplicate binding to be
> added which then violates assumptions later on.

LGTM

> 
> 	PR c++/99242
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (append_imported_binding_slot): Propagate dups
> 	flags.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/pr99242_a.H: New test.
> 	* g++.dg/modules/pr99242_b.H: New test.
> 	* g++.dg/modules/pr99242_c.H: New test.
> 	* g++.dg/modules/pr99242_d.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/name-lookup.cc                    | 4 ++++
>  gcc/testsuite/g++.dg/modules/pr99242_a.H | 3 +++
>  gcc/testsuite/g++.dg/modules/pr99242_b.H | 3 +++
>  gcc/testsuite/g++.dg/modules/pr99242_c.H | 3 +++
>  gcc/testsuite/g++.dg/modules/pr99242_d.C | 7 +++++++
>  5 files changed, 20 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_a.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_b.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_c.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_d.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index b57893116eb..3f0b8a59ff4 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -352,6 +352,10 @@ append_imported_binding_slot (tree *slot, tree name, unsigned ix)
>  
>        tree new_vec = make_binding_vec (name, want);
>        BINDING_VECTOR_NUM_CLUSTERS (new_vec) = have + 1;
> +      BINDING_VECTOR_GLOBAL_DUPS_P (new_vec)
> +	= BINDING_VECTOR_GLOBAL_DUPS_P (*slot);
> +      BINDING_VECTOR_PARTITION_DUPS_P (new_vec)
> +	= BINDING_VECTOR_PARTITION_DUPS_P (*slot);
>        memcpy (BINDING_VECTOR_CLUSTER_BASE (new_vec),
>  	      BINDING_VECTOR_CLUSTER_BASE (*slot),
>  	      have * sizeof (binding_cluster));
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_a.H b/gcc/testsuite/g++.dg/modules/pr99242_a.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_a.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_b.H b/gcc/testsuite/g++.dg/modules/pr99242_b.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_b.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_c.H b/gcc/testsuite/g++.dg/modules/pr99242_c.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_c.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_d.C b/gcc/testsuite/g++.dg/modules/pr99242_d.C
> new file mode 100644
> index 00000000000..1046d1af984
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_d.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +bool __is_constant_evaluated();
> +import "pr99242_a.H";
> +void f() { __is_constant_evaluated(); }
> +import "pr99242_b.H";
> +import "pr99242_c.H";
> +void g() { __is_constant_evaluated(); }
> -- 
> 2.43.2
> 
>
Jason Merrill July 16, 2024, 4:07 p.m. UTC | #2 
On 7/8/24 8:30 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested (so far just modules.exp) on
> x86_64-pc-linux-gnu, OK for trunk/14 if full regtest succeeds?

OK.

> -- >8 --
> 
> When importing modules, when a binding vector for a name runs out of
> slots it gets reallocated with a larger size, and existing bindings are
> copied across.  However, the flags to indicate whether deduping needs to
> occur did not: this causes ICEs, as it allows a duplicate binding to be
> added which then violates assumptions later on.
> 
> 	PR c++/99242
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (append_imported_binding_slot): Propagate dups
> 	flags.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/pr99242_a.H: New test.
> 	* g++.dg/modules/pr99242_b.H: New test.
> 	* g++.dg/modules/pr99242_c.H: New test.
> 	* g++.dg/modules/pr99242_d.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/name-lookup.cc                    | 4 ++++
>   gcc/testsuite/g++.dg/modules/pr99242_a.H | 3 +++
>   gcc/testsuite/g++.dg/modules/pr99242_b.H | 3 +++
>   gcc/testsuite/g++.dg/modules/pr99242_c.H | 3 +++
>   gcc/testsuite/g++.dg/modules/pr99242_d.C | 7 +++++++
>   5 files changed, 20 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_b.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_c.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr99242_d.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index b57893116eb..3f0b8a59ff4 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -352,6 +352,10 @@ append_imported_binding_slot (tree *slot, tree name, unsigned ix)
>   
>         tree new_vec = make_binding_vec (name, want);
>         BINDING_VECTOR_NUM_CLUSTERS (new_vec) = have + 1;
> +      BINDING_VECTOR_GLOBAL_DUPS_P (new_vec)
> +	= BINDING_VECTOR_GLOBAL_DUPS_P (*slot);
> +      BINDING_VECTOR_PARTITION_DUPS_P (new_vec)
> +	= BINDING_VECTOR_PARTITION_DUPS_P (*slot);
>         memcpy (BINDING_VECTOR_CLUSTER_BASE (new_vec),
>   	      BINDING_VECTOR_CLUSTER_BASE (*slot),
>   	      have * sizeof (binding_cluster));
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_a.H b/gcc/testsuite/g++.dg/modules/pr99242_a.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_a.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_b.H b/gcc/testsuite/g++.dg/modules/pr99242_b.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_b.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_c.H b/gcc/testsuite/g++.dg/modules/pr99242_c.H
> new file mode 100644
> index 00000000000..2df0b4184ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_c.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +bool __is_constant_evaluated();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99242_d.C b/gcc/testsuite/g++.dg/modules/pr99242_d.C
> new file mode 100644
> index 00000000000..1046d1af984
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99242_d.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +bool __is_constant_evaluated();
> +import "pr99242_a.H";
> +void f() { __is_constant_evaluated(); }
> +import "pr99242_b.H";
> +import "pr99242_c.H";
> +void g() { __is_constant_evaluated(); }
diff mbox series

Patch

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index b57893116eb..3f0b8a59ff4 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -352,6 +352,10 @@  append_imported_binding_slot (tree *slot, tree name, unsigned ix)
 
       tree new_vec = make_binding_vec (name, want);
       BINDING_VECTOR_NUM_CLUSTERS (new_vec) = have + 1;
+      BINDING_VECTOR_GLOBAL_DUPS_P (new_vec)
+	= BINDING_VECTOR_GLOBAL_DUPS_P (*slot);
+      BINDING_VECTOR_PARTITION_DUPS_P (new_vec)
+	= BINDING_VECTOR_PARTITION_DUPS_P (*slot);
       memcpy (BINDING_VECTOR_CLUSTER_BASE (new_vec),
 	      BINDING_VECTOR_CLUSTER_BASE (*slot),
 	      have * sizeof (binding_cluster));
diff --git a/gcc/testsuite/g++.dg/modules/pr99242_a.H b/gcc/testsuite/g++.dg/modules/pr99242_a.H
new file mode 100644
index 00000000000..2df0b4184ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99242_a.H
@@ -0,0 +1,3 @@ 
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+bool __is_constant_evaluated();
diff --git a/gcc/testsuite/g++.dg/modules/pr99242_b.H b/gcc/testsuite/g++.dg/modules/pr99242_b.H
new file mode 100644
index 00000000000..2df0b4184ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99242_b.H
@@ -0,0 +1,3 @@ 
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+bool __is_constant_evaluated();
diff --git a/gcc/testsuite/g++.dg/modules/pr99242_c.H b/gcc/testsuite/g++.dg/modules/pr99242_c.H
new file mode 100644
index 00000000000..2df0b4184ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99242_c.H
@@ -0,0 +1,3 @@ 
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+bool __is_constant_evaluated();
diff --git a/gcc/testsuite/g++.dg/modules/pr99242_d.C b/gcc/testsuite/g++.dg/modules/pr99242_d.C
new file mode 100644
index 00000000000..1046d1af984
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99242_d.C
@@ -0,0 +1,7 @@ 
+// { dg-additional-options "-fmodules-ts" }
+bool __is_constant_evaluated();
+import "pr99242_a.H";
+void f() { __is_constant_evaluated(); }
+import "pr99242_b.H";
+import "pr99242_c.H";
+void g() { __is_constant_evaluated(); }