libcpp: Improve the diagnostic for poisoned identifiers [PR36887]

The PR requests an enhancement to the diagnostic issued for the use of a
poisoned identifier. Currently, we show the location of the usage, but not
the location which requested the poisoning, which would be helpful for the
user if the decision to poison an identifier was made externally, such as
in a library header.

In order to output this information, we need to remember a location_t for
each identifier that has been poisoned, and that data needs to be preserved
as well in a PCH. One option would be to add a field to struct cpp_hashnode,
but there is no convenient place to add it without increasing the size of
the struct for all identifiers. Given this facility will be needed rarely,
it seemed better to add a second hash map, which is handled PCH-wise the
same as the current one in gcc/stringpool.cc. This hash map associates a new
struct cpp_hashnode_extra with each identifier that needs one. Currently
that struct only contains the new location_t, but it could be extended in
the future if there is other ancillary data that may be convenient to put
there for other purposes.

libcpp/ChangeLog:

	PR preprocessor/36887
	* directives.cc (do_pragma_poison): Store in the extra hash map the
	location from which an identifier has been poisoned.
	* lex.cc (identifier_diagnostics_on_lex): When issuing a diagnostic
	for the use of a poisoned identifier, also add a note indicating the
	location from which it was poisoned.
	* identifiers.cc (alloc_node): Convert to template function.
	(_cpp_init_hashtable): Handle the new extra hash map.
	(_cpp_destroy_hashtable): Likewise.
	* include/cpplib.h (struct cpp_hashnode_extra): New struct.
	(cpp_create_reader): Update prototype to...
	* init.cc (cpp_create_reader): ...accept an argument for the extra
	hash table and pass it to _cpp_init_hashtable.
	* include/symtab.h (ht_lookup): New overload for convenience.
	* internal.h (struct cpp_reader): Add EXTRA_HASH_TABLE member.
	(_cpp_init_hashtable): Adjust prototype.

gcc/c-family/ChangeLog:

	PR preprocessor/36887
	* c-opts.cc (c_common_init_options): Pass new extra hash map
	argument to cpp_create_reader().

gcc/ChangeLog:

	PR preprocessor/36887
	* toplev.h (ident_hash_extra): Declare...
	* stringpool.cc (ident_hash_extra): ...this new global variable.
	(init_stringpool): Handle ident_hash_extra as well as ident_hash.
	(ggc_mark_stringpool): Likewise.
	(ggc_purge_stringpool): Likewise.
	(struct string_pool_data_extra): New struct.
	(spd2): New GC root variable.
	(gt_pch_save_stringpool): Use spd2 to handle ident_hash_extra,
	analogous to how spd is used to handle ident_hash.
	(gt_pch_restore_stringpool): Likewise.

gcc/testsuite/ChangeLog:

	PR preprocessor/36887
	* c-c++-common/cpp/diagnostic-poison.c: New test.
	* g++.dg/pch/pr36887.C: New test.
	* g++.dg/pch/pr36887.Hs: New test.

6 files changed, 65 insertions, 2 deletions

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index ce2e021..e9f7e6d 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -238,7 +238,7 @@ c_common_init_options (unsigned int decoded_options_count,
     = new (ggc_alloc <string_concat_db> ()) string_concat_db ();
 
   parse_in = cpp_create_reader (c_dialect_cxx () ? CLK_GNUCXX: CLK_GNUC89,
-				ident_hash, line_table);
+				ident_hash, line_table, ident_hash_extra);
   cb = cpp_get_callbacks (parse_in);
   cb->diagnostic = c_cpp_diagnostic;
 
diff --git a/gcc/stringpool.cc b/gcc/stringpool.cc
index 8658e6a..3848967 100644
--- a/gcc/stringpool.cc
+++ b/gcc/stringpool.cc
@@ -29,8 +29,10 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
+#include "cpplib.h"
 
 struct ht *ident_hash;
+struct ht *ident_hash_extra;
 
 static hashnode alloc_node (cpp_hash_table *);
 static int mark_ident (struct cpp_reader *, hashnode, const void *);
@@ -49,11 +51,21 @@ init_stringpool (void)
      (We can't make this idempotent since identifiers contain state) */
   if (ident_hash)
     ht_destroy (ident_hash);
+  if (ident_hash_extra)
+    ht_destroy (ident_hash_extra);
 
   /* Create with 16K (2^14) entries.  */
   ident_hash = ht_create (14);
   ident_hash->alloc_node = alloc_node;
   ident_hash->alloc_subobject = stringpool_ggc_alloc;
+
+  /* Create with 64 (2^6) entries.  */
+  ident_hash_extra = ht_create (6);
+  ident_hash_extra->alloc_node = [] (cpp_hash_table *)
+  {
+    return HT_NODE (ggc_cleared_alloc<cpp_hashnode_extra> ());
+  };
+  ident_hash_extra->alloc_subobject = stringpool_ggc_alloc;
 }
 
 /* Allocate a hash node.  */
@@ -166,6 +178,12 @@ void
 ggc_mark_stringpool (void)
 {
   ht_forall (ident_hash, mark_ident, NULL);
+  ht_forall (ident_hash_extra,
+	     [] (cpp_reader *, hashnode h, const void *)
+	     {
+	       gt_ggc_m_18cpp_hashnode_extra (h);
+	       return 1;
+	     }, nullptr);
 }
 
 /* Purge the identifier hash of identifiers which are no longer
@@ -175,6 +193,11 @@ void
 ggc_purge_stringpool (void)
 {
   ht_purge (ident_hash, maybe_delete_ident, NULL);
+  ht_purge (ident_hash_extra,
+	    [] (cpp_reader *, hashnode h, const void *) -> int
+	    {
+	      return !ggc_marked_p (h);
+	    }, nullptr);
 }
 
 /* Pointer-walking routine for strings (not very interesting, since
@@ -251,7 +274,19 @@ struct GTY(()) string_pool_data {
   unsigned int nelements;
 };
 
+struct GTY (()) string_pool_data_extra
+{
+  ht_identifier_ptr *
+    GTY((length ("%h.nslots"),
+	 nested_ptr (cpp_hashnode_extra, "%h ? HT_NODE (%h) : nullptr",
+		     "(cpp_hashnode_extra *)%h")))
+    entries;
+  unsigned int nslots;
+  unsigned int nelements;
+};
+
 static GTY(()) struct string_pool_data * spd;
+static GTY(()) struct string_pool_data_extra *spd2;
 
 /* Save the stringpool data in SPD.  */
 
@@ -264,6 +299,13 @@ gt_pch_save_stringpool (void)
   spd->entries = ggc_vec_alloc<ht_identifier_ptr> (spd->nslots);
   memcpy (spd->entries, ident_hash->entries,
 	  spd->nslots * sizeof (spd->entries[0]));
+
+  spd2 = ggc_alloc<string_pool_data_extra> ();
+  spd2->nslots = ident_hash_extra->nslots;
+  spd2->nelements = ident_hash_extra->nelements;
+  spd2->entries = ggc_vec_alloc<ht_identifier_ptr> (spd2->nslots);
+  memcpy (spd2->entries, ident_hash_extra->entries,
+	  spd2->nslots * sizeof (spd2->entries[0]));
 }
 
 /* Return the stringpool to its state before gt_pch_save_stringpool
@@ -281,7 +323,10 @@ void
 gt_pch_restore_stringpool (void)
 {
   ht_load (ident_hash, spd->entries, spd->nslots, spd->nelements, false);
+  ht_load (ident_hash_extra, spd2->entries, spd2->nslots, spd2->nelements,
+	   false);
   spd = NULL;
+  spd2 = NULL;
 }
 
 #include "gt-stringpool.h"
diff --git a/gcc/testsuite/c-c++-common/cpp/diagnostic-poison.c b/gcc/testsuite/c-c++-common/cpp/diagnostic-poison.c
new file mode 100644
index 0000000..294f77e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/diagnostic-poison.c
@@ -0,0 +1,13 @@
+/* PR preprocessor/36887 */
+/* { dg-do preprocess } */
+
+#ifdef LEVEL2
+/* Test that we get the include traced location as well.  */
+#pragma GCC poison p1 /* { dg-note "poisoned here" } */
+#else
+#define LEVEL2
+#include "diagnostic-poison.c"
+int p1; /* { dg-error "attempt to use poisoned" } */
+_Pragma("GCC poison p2") /* { dg-note "poisoned here" } */
+int p2; /* { dg-error "attempt to use poisoned" } */
+#endif
diff --git a/gcc/testsuite/g++.dg/pch/pr36887.C b/gcc/testsuite/g++.dg/pch/pr36887.C
new file mode 100644
index 0000000..620ccc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pch/pr36887.C
@@ -0,0 +1,3 @@
+#include "pr36887.H"
+int p1; /* { dg-error "attempt to use poisoned" } */
+/* { dg-note "poisoned here" "" { target *-*-* } 1 } */
diff --git a/gcc/testsuite/g++.dg/pch/pr36887.Hs b/gcc/testsuite/g++.dg/pch/pr36887.Hs
new file mode 100644
index 0000000..e5f4caf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pch/pr36887.Hs
@@ -0,0 +1 @@
+#pragma GCC poison p1
diff --git a/gcc/toplev.h b/gcc/toplev.h
index 981112d..7150665 100644
--- a/gcc/toplev.h
+++ b/gcc/toplev.h
@@ -81,8 +81,9 @@ extern int flag_rerun_cse_after_global_opts;
 
 extern void print_version (FILE *, const char *, bool);
 
-/* The hashtable, so that the C front ends can pass it to cpplib.  */
+/* The hashtables, so that the C front ends can pass them to cpplib.  */
 extern struct ht *ident_hash;
+extern struct ht *ident_hash_extra;
 
 /* Functions used to get and set GCC's notion of in what directory
    compilation was started.  */