Re: [PATCH] modules: c++: Fix push_namespace ICE with modules

2020-08-27 Thread Nathan Sidwell

On 8/14/20 11:04 AM, Jeff Chapman wrote:

Hello!

Attached is a patch that fixes an ICE on the devel/c++-modules branch caused
by a slot invalidation edge case in push_namespace.


I just fell over this myself, reducing a testcase.  your fix wasn;t 
quite right -- we were creating an empty slot in the hash table and then 
doing another insertion, w/o initializing that slot.  We have to also 
not insert on the first looking.


Thanks for the patch! it was clueful when I hit the problem :)


It's been sitting around for a while and I wasn't sure if I should use the
original date or not. Feel free to adjust that to the commit date if that's
what it should be instead.


2020-05-15  Jeff Chapman II  

gcc/cp/
* name-lookup.c (push_namespace): Fix slot invalidation related
ICE when compiling with modules enabled.

gcc/testsuite/

* g++.dg/modules/string-view1.C: New test.
* g++.dg/modules/string-view2.C: Ditto.


Please let me know if there's anything more I can do,
Jeff Chapman II




--
Nathan Sidwell
diff --git c/ChangeLog.modules w/ChangeLog.modules
index 4d50352a698..27233f0228d 100644
--- c/ChangeLog.modules
+++ w/ChangeLog.modules
@@ -1,3 +1,14 @@
+2020-08-27  Nathan Sidwell  
+	Jeff Chapman II  
+
+	Fix hash table breakage.
+	gcc/cp/
+	* name-lookup.c (push_namespace): Do not create slot on first
+	lookup.
+	gcc/testsuite/
+	* g++.dg/modules/string-view1.C: New test.
+	* g++.dg/modules/string-view2.C: Ditto.
+
 2020-08-27  Nathan Sidwell  
 
 	Merge trunk 708b3600d04.
diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c
index d7ec9d214ed..3e5985d396c 100644
--- c/gcc/cp/name-lookup.c
+++ w/gcc/cp/name-lookup.c
@@ -8872,8 +8872,9 @@ push_namespace (tree name, bool make_inline)
 {
   /* Before making a new namespace, see if we already have one in
 	 the existing partitions of the current namespace.  */
-  tree *slot = find_namespace_slot (current_namespace, name, true);
-  ns = reuse_namespace (slot, current_namespace, name);
+  tree *slot = find_namespace_slot (current_namespace, name, false);
+  if (slot)
+	ns = reuse_namespace (slot, current_namespace, name);
   if (!ns)
 	ns = make_namespace (current_namespace, name,
 			 input_location, make_inline);
@@ -8884,6 +8885,12 @@ push_namespace (tree name, bool make_inline)
 	{
 	  /* finish up making the namespace.  */
 	  add_decl_to_level (NAMESPACE_LEVEL (current_namespace), ns);
+	  if (!slot)
+	{
+	  slot = find_namespace_slot (current_namespace, name);
+	  /* This should find the slot created by pushdecl.  */
+	  gcc_checking_assert (slot);
+	}
 	  make_namespace_finish (ns, slot);
 
 	  /* Add the anon using-directive here, we don't do it in
diff --git c/gcc/testsuite/g++.dg/modules/string-view1.C w/gcc/testsuite/g++.dg/modules/string-view1.C
new file mode 100644
index 000..f5391f39180
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/string-view1.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts" }
+module;
+#include 
+#include 
+export module foo;
+// { dg-module-cmi foo }
diff --git c/gcc/testsuite/g++.dg/modules/string-view2.C w/gcc/testsuite/g++.dg/modules/string-view2.C
new file mode 100644
index 000..dad30eabba6
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/string-view2.C
@@ -0,0 +1,56 @@
+// { dg-additional-options "-fmodules-ts" }
+// reduced from string-view1 through cvise. Broken under c++2a too.
+// we were creating a hash slot, and then doing another lookup without
+// initializing that first slot :(
+
+namespace std {
+typedef int a;
+int b;
+decltype(nullptr) c;
+namespace xyz {}
+__builtin_va_list d;
+int n;
+int e;
+int f;
+int g;
+int h;
+int i;
+int j;
+int k;
+typedef struct l m;
+typedef struct aa w;
+typedef struct x o;
+typedef x p;
+long q;
+long r;
+typedef l s;
+extern p ab;
+void t();
+void v();
+extern p ac;
+void ad();
+int ae;
+int af;
+extern p ag;
+extern p ah;
+void ai();
+void y();
+int aj;
+int ak;
+int al;
+char am;
+int an;
+a ao;
+int ap;
+int aq;
+void z();
+int ar;
+int as;
+void at();
+void au();
+void av();
+void aw();
+int u;
+namespace zz {
+}
+}


[PATCH] modules: c++: Fix push_namespace ICE with modules

2020-08-14 Thread Jeff Chapman via Gcc-patches
Hello!

Attached is a patch that fixes an ICE on the devel/c++-modules branch caused
by a slot invalidation edge case in push_namespace.

It's been sitting around for a while and I wasn't sure if I should use the
original date or not. Feel free to adjust that to the commit date if that's
what it should be instead.


2020-05-15  Jeff Chapman II  

gcc/cp/
* name-lookup.c (push_namespace): Fix slot invalidation related
ICE when compiling with modules enabled.

gcc/testsuite/

* g++.dg/modules/string-view1.C: New test.
* g++.dg/modules/string-view2.C: Ditto.


Please let me know if there's anything more I can do,
Jeff Chapman II
From d33a239c187cb6cef1c39953c5f014bd266492de Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Fri, 15 May 2020 06:37:41 -0400
Subject: [PATCH 1/1] c++: Fix push_namespace ICE with modules

push_namespace was reusing an earlier find_namespace_slot result after
it was invalidated by pushdecl in corner cases.

2020-05-15  Jeff Chapman II  

gcc/cp/
	* name-lookup.c (push_namespace): Fix slot invalidation related
	ICE when compiling with modules enabled.

gcc/testsuite/

	* g++.dg/modules/string-view1.C: New test.
	* g++.dg/modules/string-view2.C: Ditto.
---
 gcc/cp/name-lookup.c|  2 +
 gcc/testsuite/g++.dg/modules/string-view1.C |  6 +++
 gcc/testsuite/g++.dg/modules/string-view2.C | 53 +
 3 files changed, 61 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/string-view1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/string-view2.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e9495d2a282..462f028617c 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -8920,6 +8920,8 @@ push_namespace (tree name, bool make_inline)
 	{
 	  /* finish up making the namespace.  */
 	  add_decl_to_level (NAMESPACE_LEVEL (current_namespace), ns);
+	  /* pushdecl may invalidate slot, find name again.  */
+	  slot = find_namespace_slot (current_namespace, name, true);
 	  make_namespace_finish (ns, slot);
 
 	  /* Add the anon using-directive here, we don't do it in
diff --git a/gcc/testsuite/g++.dg/modules/string-view1.C b/gcc/testsuite/g++.dg/modules/string-view1.C
new file mode 100644
index 000..dabc16a8b01
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/string-view1.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts" }
+module;
+#include 
+#include 
+export module foo;
+
diff --git a/gcc/testsuite/g++.dg/modules/string-view2.C b/gcc/testsuite/g++.dg/modules/string-view2.C
new file mode 100644
index 000..2e389eacd8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/string-view2.C
@@ -0,0 +1,53 @@
+// { dg-additional-options "-fmodules-ts" }
+// reduced from string-view1 through cvise. Broken under c++2a too.
+namespace std {
+typedef int a;
+int b;
+decltype(nullptr) c;
+namespace xyz {}
+__builtin_va_list d;
+int n;
+int e;
+int f;
+int g;
+int h;
+int i;
+int j;
+int k;
+typedef struct l m;
+typedef struct aa w;
+typedef struct x o;
+typedef x p;
+long q;
+long r;
+typedef l s;
+extern p ab;
+void t();
+void v();
+extern p ac;
+void ad();
+int ae;
+int af;
+extern p ag;
+extern p ah;
+void ai();
+void y();
+int aj;
+int ak;
+int al;
+char am;
+int an;
+a ao;
+int ap;
+int aq;
+void z();
+int ar;
+int as;
+void at();
+void au();
+void av();
+void aw();
+int u;
+namespace zz {
+}
+}
-- 
2.27.0