On 4/20/21 8:26 PM, Jason Merrill wrote:
On Tue, Apr 20, 2021 at 8:31 PM Martin Sebor via Gcc <gcc@gcc.gnu.org> wrote:
On 4/20/21 5:03 PM, Martin Sebor wrote:
On 4/20/21 4:15 PM, Martin Sebor wrote:
On 4/20/21 2:36 PM, Martin Sebor wrote:
On 4/20/21 2:13 PM, Marek Polacek wrote:
On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
I have a static hash_map object that needs to persist across passes:
static GTY(()) hash_map<tree, bitmap> *map;
I initialize the map like so:
map = hash_map<tree, bitmap>::create_ggc (4);
But I see crashes when accessing the map after adding and removing
some number of entries in a few passes. The crashes are due to
the map having been garbage-collected and the memory allocated to
it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
in GDD being written into the map by poison_pages()).
I assumed marking the map pointer GTY(()) would keep the garbage
collector from touching the map. Is there something else I need
to do to keep it from doing that?
Thanks
Martin
PS Just in case it matters, the bitmap in the table is allocated
via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
the map.
My sense is that this is the problem. What happens if you use
BITMAP_GGC_ALLOC?
Same thing :(
I reduced the problem to the following patch/test case no involving
a bitmap or other pointers. With it applied, GCC reliably crashes.
An example of a stack trace is below the patch. What am I missing?
It seems that assigning the address of an object allocated by
ggc_alloc() (which presumably includes BITMAP_GGC_ALLOC()) to
a GTY-marked pointer "defeats" the purpose of the marker and
allows it to be garbage collected.
But this is not true. There are plenty of counterexamples, including
in tree.c which defines several GTY hash_tables (but no maps). I tried
moving the test case below from gimple.c to tree.c but there it doesn't
even compile. I get this error (and a couple of others):
In file included from /src/gcc/master/gcc/tree.c:16179:
./gt-tree.h: In function ‘void gt_ggc_mx(int_hash<int, 0, 2147483647>&)’:
./gt-tree.h:156:21: error: no matching function for call to
‘gt_ggc_mx(int_hash<int, 0, 2147483647>*)’
gt_ggc_mx (&((*x)));
^
So as another experiment I moved it builtins.c where it does compile
(go figure) but then it crashes when I call it the same way from
c_strlen().
This is because tree.c is in GTFILES, while gimple.c and tree.c are
not, so gengtype never sees the GTY decoration in the latter files.
I don't know why when it does see the GTY, it ends up trying to mark
an int_hash*.
Thanks for the hint. I didn't notice this in the manual before
so here are the steps to use GTY in file.c for reference:
1) add file.c to GTFILES in Makefile.in
2) add #include "gt-file.h" to the end of file.c
3) reconfigure + rebuild
I think I also finally figured out the compilation errors and ICEs,
after over two days of wrestling with it. It's seems like hash_map
support for GTY is incomplete, as is ggc.h. I suspect the same
problems as in hash_map are going to be in other hash-based containers.
(A gotcha that leads to cryptic errors that can add to the confusion
here is removing the GTY type from the source file as the gt-file.c
still refers to it. I dealt with it by reconfiguring again. Maybe
there's a better way.)
The reason for the error above (and others like it) is simply that
there's no gt_ggc_mx() defined for int_hash. The only gt_ function
documented in the manual that is defined (in the generated gt-foo.h)
is the three-argument overload of gt_pcx_nx().
But that's not the only missing piece or problem.
The gt_*() function templates in hash-map.h are defined static so
(IIRC from my early C++ days) they're not considered by ADL. That
leads to more compilation errors.
Also, ggc.h defines a few gt_*() overloads for a subset of integer
types but not nearly for all, so that leads to even more errors for
instantiations of int_hash on different integer types (e.g.,
uintptr_t).
Adding the full set of overloads (including, for instance, short)
leads to still more problems for int_hash specializations on types
like short.
/src/gcc/master/gcc/hash-map.h:107:14: error: no matching function for
call to ‘gt_pch_nx(short int*, void (*&)(void*, void*), void*&)’
gt_pch_nx (&x, op, cookie);
~~~~~~~~~~^~~~~~~~~~~~~~~~
Similar to ggc.h, there is a small subset of no-op overloads like
this in hash_map but not for types like short.
The attached POC patch fixes this (foo can be called from anywhere).
I haven't done a full bootstrap with it yet or looked at hash_set or
but please let me know if you see any issues with it. I'll submit
a complete patch once I'm done with it.
Martin
PS For the overloads I resisted introducing SFINAE to keep things
simple and avoid pulling in <type_traits> (among other things).
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8a5fb3fd99c..9ff8d1596f2 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1373,6 +1373,7 @@ OBJS = \
fixed-value.o \
fold-const.o \
fold-const-call.o \
+ foo.o \
function.o \
function-abi.o \
function-tests.o \
@@ -2662,6 +2663,7 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
$(srcdir)/dojump.c $(srcdir)/emit-rtl.h \
$(srcdir)/emit-rtl.c $(srcdir)/except.h $(srcdir)/explow.c $(srcdir)/expr.c \
$(srcdir)/expr.h \
+ $(srcdir)/foo.c \
$(srcdir)/function.c $(srcdir)/except.c \
$(srcdir)/ggc-tests.c \
$(srcdir)/gcse.c $(srcdir)/godump.c \
diff --git a/gcc/foo.c b/gcc/foo.c
new file mode 100644
index 00000000000..ea9cebfd9b2
--- /dev/null
+++ b/gcc/foo.c
@@ -0,0 +1,35 @@
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "hash-map.h"
+
+// typedef int_hash<char, 0, CHAR_MAX> UintPtrHash;
+// typedef int_hash<short, 0, SHRT_MAX> UintPtrHash;
+// typedef int_hash<int, 0, INT_MAX> UintPtrHash;
+// typedef int_hash<long, 0, LONG_MAX> UintPtrHash;
+// typedef int_hash<intptr_t, 0, INTPTR_MAX> UintPtrHash;
+typedef int_hash<uintptr_t, 0, UINTPTR_MAX> UintPtrHash;
+
+inline void
+gt_ggc_mx (UintPtrHash *) { }
+
+inline void
+gt_pch_nx (UintPtrHash *) { }
+
+typedef hash_map<UintPtrHash, tree> UintPtrHashMap;
+static GTY(()) UintPtrHashMap *tree_map;
+
+void
+foo (tree x)
+{
+ if (!tree_map)
+ tree_map = tree_map->create_ggc (1);
+
+ tree_map->put ((uintptr_t)x, x);
+ if (tree_map->elements () > 1024)
+ tree_map->empty ();
+}
+
+#include "gt-foo.h"
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 65f6cb4d19d..ad0e7488011 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -332,19 +332,24 @@ gt_pch_nx (const char *)
{
}
-inline void
-gt_ggc_mx (int)
-{
-}
-
-inline void
-gt_pch_nx (int)
-{
-}
-
-inline void
-gt_pch_nx (unsigned int)
-{
-}
+#define DEFINE_GT_HELPERS(T) \
+ inline void gt_ggc_mx (T) { } \
+ inline void gt_pch_nx (T) { } \
+ typedef void unused_ggc_type /* expect semicolon */
+
+DEFINE_GT_HELPERS (bool);
+DEFINE_GT_HELPERS (char);
+DEFINE_GT_HELPERS (signed char);
+DEFINE_GT_HELPERS (unsigned char);
+DEFINE_GT_HELPERS (short);
+DEFINE_GT_HELPERS (unsigned short);
+DEFINE_GT_HELPERS (int);
+DEFINE_GT_HELPERS (unsigned int);
+DEFINE_GT_HELPERS (long);
+DEFINE_GT_HELPERS (unsigned long);
+DEFINE_GT_HELPERS (long long);
+DEFINE_GT_HELPERS (unsigned long long);
+
+#undef DEFINE_GT_HELPERS
#endif
diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 0779c930f0a..60f6f50296a 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -107,27 +107,31 @@ class GTY((user)) hash_map
gt_pch_nx (&x, op, cookie);
}
- static void
- pch_nx_helper (int, gt_pointer_operator, void *)
- {
- }
-
- static void
- pch_nx_helper (unsigned int, gt_pointer_operator, void *)
- {
- }
-
- static void
- pch_nx_helper (bool, gt_pointer_operator, void *)
- {
- }
-
template<typename T>
static void
pch_nx_helper (T *&x, gt_pointer_operator op, void *cookie)
{
op (&x, cookie);
}
+
+ /* The overloads below should match those in ggc.h. */
+#define DEFINE_PCH_HELPER(T) \
+ static void pch_nx_helper (T, gt_pointer_operator, void *) { }
+
+ DEFINE_PCH_HELPER (bool);
+ DEFINE_PCH_HELPER (char);
+ DEFINE_PCH_HELPER (signed char);
+ DEFINE_PCH_HELPER (unsigned char);
+ DEFINE_PCH_HELPER (short);
+ DEFINE_PCH_HELPER (unsigned short);
+ DEFINE_PCH_HELPER (int);
+ DEFINE_PCH_HELPER (unsigned int);
+ DEFINE_PCH_HELPER (long);
+ DEFINE_PCH_HELPER (unsigned long);
+ DEFINE_PCH_HELPER (long long);
+ DEFINE_PCH_HELPER (unsigned long long);
+
+#undef DEFINE_PCH_HELPER
};
public:
@@ -301,21 +305,21 @@ private:
/* ggc marking routines. */
template<typename K, typename V, typename H>
-static inline void
+inline void
gt_ggc_mx (hash_map<K, V, H> *h)
{
gt_ggc_mx (&h->m_table);
}
template<typename K, typename V, typename H>
-static inline void
+inline void
gt_pch_nx (hash_map<K, V, H> *h)
{
gt_pch_nx (&h->m_table);
}
template<typename K, typename V, typename H>
-static inline void
+inline void
gt_cleare_cache (hash_map<K, V, H> *h)
{
if (h)
@@ -323,7 +327,7 @@ gt_cleare_cache (hash_map<K, V, H> *h)
}
template<typename K, typename V, typename H>
-static inline void
+inline void
gt_pch_nx (hash_map<K, V, H> *h, gt_pointer_operator op, void *cookie)
{
op (&h->m_table.m_entries, cookie);
