public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node
@ 2022-06-03  1:20 Antoni Boucher
  2022-06-03  1:23 ` Antoni Boucher
  2022-06-03  1:23 ` Andrew Pinski
  0 siblings, 2 replies; 5+ messages in thread
From: Antoni Boucher @ 2022-06-03  1:20 UTC (permalink / raw)
  To: gcc-patches, jit

Hi.
The attached patch fix bug 105827:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105827

I'm not sure how to test this, so please share ideas.

Thanks for the review.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node
  2022-06-03  1:20 [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node Antoni Boucher
@ 2022-06-03  1:23 ` Antoni Boucher
  2022-06-06 23:01   ` David Malcolm
  2022-06-03  1:23 ` Andrew Pinski
  1 sibling, 1 reply; 5+ messages in thread
From: Antoni Boucher @ 2022-06-03  1:23 UTC (permalink / raw)
  To: gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

Sorry, forgot to attach the patch.

Here it is.

On Thu, 2022-06-02 at 21:20 -0400, Antoni Boucher via Jit wrote:
> Hi.
> The attached patch fix bug 105827:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105827
> 
> I'm not sure how to test this, so please share ideas.
> 
> Thanks for the review.


[-- Attachment #2: 0001-libgccjit-Fix-infinite-recursion-in-gt_ggc_mx_lang_t.patch --]
[-- Type: text/x-patch, Size: 2423 bytes --]

From 609153a39921b8e9aa1934da131575bb64881d67 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Thu, 2 Jun 2022 21:14:06 -0400
Subject: [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node

2022-06-02  Antoni Boucher  <bouanto@zoho.com>

gcc/jit/
	PR target/105827
	* dummy-frontend.cc: Fix lang_tree_node.
	* jit-common.h: New function (jit_tree_chain_next) used by
	lang_tree_node.
---
 gcc/jit/dummy-frontend.cc | 13 +++++++------
 gcc/jit/jit-common.h      | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
index 84ff359bfe3..8bb5d03d630 100644
--- a/gcc/jit/dummy-frontend.cc
+++ b/gcc/jit/dummy-frontend.cc
@@ -506,13 +506,14 @@ struct GTY(()) lang_identifier
 
 /* The resulting tree type.  */
 
+/* See lang_tree_node in gcc/c/c-decl.cc.  */
 union GTY((desc ("TREE_CODE (&%h.generic) == IDENTIFIER_NODE"),
-	   chain_next ("CODE_CONTAINS_STRUCT (TREE_CODE (&%h.generic), TS_COMMON) ? ((union lang_tree_node *) TREE_CHAIN (&%h.generic)) : NULL")))
-lang_tree_node
-{
-  union tree_node GTY((tag ("0"),
-		       desc ("tree_node_structure (&%h)"))) generic;
-  struct lang_identifier GTY((tag ("1"))) identifier;
+       chain_next ("(union lang_tree_node *) jit_tree_chain_next (&%h.generic)"))) lang_tree_node
+ {
+  union tree_node GTY ((tag ("0"),
+			desc ("tree_node_structure (&%h)")))
+    generic;
+  struct lang_identifier GTY ((tag ("1"))) identifier;
 };
 
 /* We don't use language_function.  */
diff --git a/gcc/jit/jit-common.h b/gcc/jit/jit-common.h
index 3ff7447fbf3..50580358a1f 100644
--- a/gcc/jit/jit-common.h
+++ b/gcc/jit/jit-common.h
@@ -93,6 +93,21 @@ const int NUM_GCC_JIT_TYPES = GCC_JIT_TYPE_INT128_T + 1;
 
    End of comment for inclusion in the docs.  */
 
+/* See c_tree_chain_next in gcc/c-family/c-common.h.  */
+static inline tree
+jit_tree_chain_next (tree t)
+{
+  /* TREE_CHAIN of a type is TYPE_STUB_DECL, which is different
+     kind of object, never a long chain of nodes.  Prefer
+     TYPE_NEXT_VARIANT for types.  */
+  if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPE_COMMON))
+    return TYPE_NEXT_VARIANT (t);
+  /* Otherwise, if there is TREE_CHAIN, return it.  */
+  if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_COMMON))
+    return TREE_CHAIN (t);
+  return NULL;
+}
+
 namespace gcc {
 
 namespace jit {
-- 
2.26.2.7.g19db9cfb68.dirty


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node
  2022-06-03  1:20 [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node Antoni Boucher
  2022-06-03  1:23 ` Antoni Boucher
@ 2022-06-03  1:23 ` Andrew Pinski
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Pinski @ 2022-06-03  1:23 UTC (permalink / raw)
  To: Antoni Boucher; +Cc: GCC Patches, jit

On Thu, Jun 2, 2022 at 6:21 PM Antoni Boucher via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi.
> The attached patch fix bug 105827:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105827
>
> I'm not sure how to test this, so please share ideas.

Looks like the attachment was removed ...

>
> Thanks for the review.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node
  2022-06-03  1:23 ` Antoni Boucher
@ 2022-06-06 23:01   ` David Malcolm
  2024-01-10 23:24     ` Antoni Boucher
  0 siblings, 1 reply; 5+ messages in thread
From: David Malcolm @ 2022-06-06 23:01 UTC (permalink / raw)
  To: Antoni Boucher, gcc-patches, jit

On Thu, 2022-06-02 at 21:23 -0400, Antoni Boucher via Gcc-patches
wrote:
> Sorry, forgot to attach the patch.
> 
> Here it is.
> 
> On Thu, 2022-06-02 at 21:20 -0400, Antoni Boucher via Jit wrote:
> > Hi.
> > The attached patch fix bug 105827:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105827
> > 
> > I'm not sure how to test this, so please share ideas.

Do you have a reproducer for this?

With garbage-collections issues in libgccjit, you can set:

  gcc_jit_context_set_bool_option (ctxt,
                                   GCC_JIT_BOOL_OPTION_SELFCHECK_GC,
                                   1);

which will force a full garbage collection at every opportunity that
the collector considers doing one (rather than following heuristics)

This really slows things down, but makes reproducing crashes much more
deterministic, often turning "it crashes every now and then" to "it
crashes every time" (and the test suite runs with this enabled).

> > 
> > Thanks for the review.
> 

> diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
> index 84ff359bfe3..8bb5d03d630 100644
> --- a/gcc/jit/dummy-frontend.cc
> +++ b/gcc/jit/dummy-frontend.cc
> @@ -506,13 +506,14 @@ struct GTY(()) lang_identifier
>  
>  /* The resulting tree type.  */
>  
> +/* See lang_tree_node in gcc/c/c-decl.cc.  */
>  union GTY((desc ("TREE_CODE (&%h.generic) == IDENTIFIER_NODE"),
> -	   chain_next ("CODE_CONTAINS_STRUCT (TREE_CODE (&%h.generic), TS_COMMON) ? ((union lang_tree_node *) TREE_CHAIN (&%h.generic)) : NULL")))
> -lang_tree_node
> -{
> -  union tree_node GTY((tag ("0"),
> -		       desc ("tree_node_structure (&%h)"))) generic;
> -  struct lang_identifier GTY((tag ("1"))) identifier;
> +       chain_next ("(union lang_tree_node *) jit_tree_chain_next (&%h.generic)"))) lang_tree_node
> + {
> +  union tree_node GTY ((tag ("0"),
> +			desc ("tree_node_structure (&%h)")))
> +    generic;
> +  struct lang_identifier GTY ((tag ("1"))) identifier;
>  };

Those GTY markings on gcc/jit/dummy-frontend.cc's lang_tree_node union
have been like that since my initial proof-of-concept patch back in
2013:
  https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg00228.html
so presumably I simply copied and pasted that from another frontend
when I was initially prototyping libgccjit.  There was an element of
"cargo cult programming" as I was getting the project started.

Jakub had changed the C and C++ frontends in June 2011 with
563007852e8d19b66ec8c1e42e431efaaa967dc6, which introduced the
c_tree_chain_next that you're now copying in your patch, so presumably
I copied from a different frontend.  My notes say I created the first
prototype in July 2013, so that's when I would have copied&pasted the
code.

Dave







^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node
  2022-06-06 23:01   ` David Malcolm
@ 2024-01-10 23:24     ` Antoni Boucher
  0 siblings, 0 replies; 5+ messages in thread
From: Antoni Boucher @ 2024-01-10 23:24 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

It seems I cannot reproduce the issue.
Should we drop this patch, then?
Or do you think there's value in keeping it?

On Mon, 2022-06-06 at 19:01 -0400, David Malcolm wrote:
> On Thu, 2022-06-02 at 21:23 -0400, Antoni Boucher via Gcc-patches
> wrote:
> > Sorry, forgot to attach the patch.
> > 
> > Here it is.
> > 
> > On Thu, 2022-06-02 at 21:20 -0400, Antoni Boucher via Jit wrote:
> > > Hi.
> > > The attached patch fix bug 105827:
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105827
> > > 
> > > I'm not sure how to test this, so please share ideas.
> 
> Do you have a reproducer for this?
> 
> With garbage-collections issues in libgccjit, you can set:
> 
>   gcc_jit_context_set_bool_option (ctxt,
>                                    GCC_JIT_BOOL_OPTION_SELFCHECK_GC,
>                                    1);
> 
> which will force a full garbage collection at every opportunity that
> the collector considers doing one (rather than following heuristics)
> 
> This really slows things down, but makes reproducing crashes much
> more
> deterministic, often turning "it crashes every now and then" to "it
> crashes every time" (and the test suite runs with this enabled).
> 
> > > 
> > > Thanks for the review.
> > 
> 
> > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
> > index 84ff359bfe3..8bb5d03d630 100644
> > --- a/gcc/jit/dummy-frontend.cc
> > +++ b/gcc/jit/dummy-frontend.cc
> > @@ -506,13 +506,14 @@ struct GTY(()) lang_identifier
> >  
> >  /* The resulting tree type.  */
> >  
> > +/* See lang_tree_node in gcc/c/c-decl.cc.  */
> >  union GTY((desc ("TREE_CODE (&%h.generic) == IDENTIFIER_NODE"),
> > -	   chain_next ("CODE_CONTAINS_STRUCT (TREE_CODE
> > (&%h.generic), TS_COMMON) ? ((union lang_tree_node *) TREE_CHAIN
> > (&%h.generic)) : NULL")))
> > -lang_tree_node
> > -{
> > -  union tree_node GTY((tag ("0"),
> > -		       desc ("tree_node_structure (&%h)")))
> > generic;
> > -  struct lang_identifier GTY((tag ("1"))) identifier;
> > +       chain_next ("(union lang_tree_node *) jit_tree_chain_next
> > (&%h.generic)"))) lang_tree_node
> > + {
> > +  union tree_node GTY ((tag ("0"),
> > +			desc ("tree_node_structure (&%h)")))
> > +    generic;
> > +  struct lang_identifier GTY ((tag ("1"))) identifier;
> >  };
> 
> Those GTY markings on gcc/jit/dummy-frontend.cc's lang_tree_node
> union
> have been like that since my initial proof-of-concept patch back in
> 2013:
>   https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg00228.html
> so presumably I simply copied and pasted that from another frontend
> when I was initially prototyping libgccjit.  There was an element of
> "cargo cult programming" as I was getting the project started.
> 
> Jakub had changed the C and C++ frontends in June 2011 with
> 563007852e8d19b66ec8c1e42e431efaaa967dc6, which introduced the
> c_tree_chain_next that you're now copying in your patch, so
> presumably
> I copied from a different frontend.  My notes say I created the first
> prototype in July 2013, so that's when I would have copied&pasted the
> code.
> 
> Dave
> 
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-01-10 23:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03  1:20 [PATCH] libgccjit: Fix infinite recursion in gt_ggc_mx_lang_tree_node Antoni Boucher
2022-06-03  1:23 ` Antoni Boucher
2022-06-06 23:01   ` David Malcolm
2024-01-10 23:24     ` Antoni Boucher
2022-06-03  1:23 ` Andrew Pinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).