On Thu, Dec 4, 2025 at 12:41 AM Richard Biener <[email protected]> wrote: > > On Thu, Dec 4, 2025 at 6:02 AM Andrew Pinski > <[email protected]> wrote: > > > > Currently the GC marker functions don't support chain_next on non-toplevel > > tag structures (and does not error out either). So since > > r16-4747-g529c25ed6e0a06 > > if there are a lot of chained symtab_nodes (happens most likely with LTO), > > the GC > > marker function could cause a stack overflow because of the recusive nature > > of the marker. > > > > This fixes the problem by moving next/previous to toplevel_node. I had > > originally > > thought about doing chain_next/chain_prev and using is_a<symtab_node *> to > > get if > > it was symtab_node and then used the next/previous from there. But it was > > noticed that > > asm_node had a next too (though not using chain_next) so adding a previous > > is not going > > to much more space anyways; there will not be many toplevel inline-asm > > anyways. > > > > Bootstraped and tested on x86_64-linux-gnu. > > LGTM. Please give IPA maintainers the chance to chime in.
Ok, I have not seen any comments after a week so I am pushing it now as approved. > I do wonder what's > the C++ "pattern" to have the base class next/previous pointers statically > typed > to the derived class type? I don't think there is one. Though I was thinking about adding a new/previous member function which returns the statically casted type. I decided against that and just add the casts in the places of the uses. > I understand we actually do not chain asm_nodes and > symtab_nodes in the same chain? Otherwise the walkers would not be safe after > unifying the chain pointers as in principle a symtab_node could be followed by > an asm_node, breaking iterations like that in > symtab_node::next_defined_symbol? Correct, we don't chain asm_nodes and symtab_nodes together. Thanks, Andrew > > Richard. > > > > > PR ipa/122955 > > gcc/ChangeLog: > > > > * cgraph.h (toplevel_node): Add next and previous fields. > > Add chain_next and chain_prev to GTY. > > (symtab_node): Remove next and previous field. Remove chain_next > > and chain_prev > > from the GTY. > > (asm_node): Remove next field. > > (symtab_node::next_defined_symbol): Use save_as_a<symtab_node*> > > around next. > > (symbol_table::unregister): Likewise > > (FOR_EACH_SYMBOL): Likewise > > (symbol_table::first_defined_symbol): Likewise > > (symbol_table::first_variable): Likewise > > (symbol_table::next_variable): Likewise > > (symbol_table::first_static_initializer): Likewise > > (symbol_table::next_static_initializer): Likewise > > (symbol_table::first_defined_variable): Likewise > > (symbol_table::next_defined_variable): Likewise > > (symbol_table::first_defined_function): Likewise > > (symbol_table::next_defined_function): Likewise > > (symbol_table::first_function): Likewise > > (symbol_table::next_function): Likewise > > (symbol_table::first_function_with_gimple_body): Likewise > > (symbol_table::next_function_with_gimple_body): Likewise > > * cgraphunit.cc (analyze_functions): Likewise > > (output_in_order): Likewise > > * lto-streamer-out.cc (lto_output): Use save_as_a<asm_node*> around > > next. > > * symtab.cc (symtab_node::verify_symtab_nodes): Likewise. > > > > gcc/lto/ChangeLog: > > > > * lto-partition.cc (lto_1_to_1_map): Use save_as_a<asm_node*> > > around next. > > (create_asm_partition): Likewise. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > gcc/cgraph.h | 67 ++++++++++++++++++++-------------------- > > gcc/cgraphunit.cc | 11 ++++--- > > gcc/lto-streamer-out.cc | 4 ++- > > gcc/lto/lto-partition.cc | 4 +-- > > gcc/symtab.cc | 4 ++- > > 5 files changed, 48 insertions(+), 42 deletions(-) > > > > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > > index 313610fbe2c..6d589f59442 100644 > > --- a/gcc/cgraph.h > > +++ b/gcc/cgraph.h > > @@ -107,7 +107,9 @@ enum symbol_partitioning_class > > > > /* Base of all toplevel entries. > > Inherited by symtab_node and asm_node. */ > > -struct GTY ((desc ("%h.type"), tag ("TOPLEVEL_BASE"))) toplevel_node { > > +struct GTY ((desc ("%h.type"), tag ("TOPLEVEL_BASE"), > > + chain_next("%h.next"), > > + chain_prev("%h.previous"))) toplevel_node { > > /* Constructor. */ > > explicit toplevel_node (toplevel_type t) > > : lto_file_data (NULL), order (-1), type (t) > > @@ -116,6 +118,10 @@ struct GTY ((desc ("%h.type"), tag ("TOPLEVEL_BASE"))) > > toplevel_node { > > /* File stream where this node is being written to. */ > > struct lto_file_decl_data * lto_file_data; > > > > + /* Linked list of toplevel entries. */ > > + toplevel_node *next = nullptr; > > + toplevel_node *previous = nullptr; > > + > > /* Ordering of all cgraph nodes. */ > > int order; > > > > @@ -125,8 +131,7 @@ struct GTY ((desc ("%h.type"), tag ("TOPLEVEL_BASE"))) > > toplevel_node { > > > > /* Base of all entries in the symbol table. > > The symtab_node is inherited by cgraph and varpol nodes. */ > > -struct GTY ((tag ("SYMTAB_SYMBOL"), > > - chain_next ("%h.next"), chain_prev ("%h.previous"))) > > +struct GTY ((tag ("SYMTAB_SYMBOL"))) > > symtab_node: public toplevel_node > > { > > public: > > @@ -633,10 +638,6 @@ public: > > /* Declaration representing the symbol. */ > > tree decl; > > > > - /* Linked list of symbol table entries starting with symtab_nodes. */ > > - symtab_node *next; > > - symtab_node *previous; > > - > > /* Linked list of symbols with the same asm name. There may be multiple > > entries for single symbol name during LTO, because symbols are renamed > > only after partitioning. > > @@ -2243,10 +2244,8 @@ private: > > > > struct GTY ((tag ("TOPLEVEL_ASM"))) asm_node: public toplevel_node { > > explicit asm_node (tree asm_str) > > - : toplevel_node (TOPLEVEL_ASM), next (NULL), asm_str (asm_str) > > + : toplevel_node (TOPLEVEL_ASM), asm_str (asm_str) > > {} > > - /* Next asm node. */ > > - asm_node *next; > > /* String for this asm node. */ > > tree asm_str; > > }; > > @@ -2867,9 +2866,9 @@ symtab_node::get_alias_target_tree () > > inline symtab_node * > > symtab_node::next_defined_symbol (void) > > { > > - symtab_node *node1 = next; > > + symtab_node *node1 = safe_as_a<symtab_node *>(next); > > > > - for (; node1; node1 = node1->next) > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > if (node1->definition) > > return node1; > > > > @@ -2997,7 +2996,7 @@ symbol_table::unregister (symtab_node *node) > > if (node->previous) > > node->previous->next = node->next; > > else > > - nodes = node->next; > > + nodes = safe_as_a<symtab_node *>(node->next); > > > > if (node->next) > > node->next->previous = node->previous; > > @@ -3026,7 +3025,8 @@ symbol_table::first_symbol (void) > > > > /* Walk all symbols. */ > > #define FOR_EACH_SYMBOL(node) \ > > - for ((node) = symtab->first_symbol (); (node); (node) = (node)->next) > > + for ((node) = symtab->first_symbol (); (node); \ > > + (node) = safe_as_a<symtab_node *>((node)->next)) > > > > /* Return first static symbol with definition. */ > > inline symtab_node * > > @@ -3034,7 +3034,8 @@ symbol_table::first_defined_symbol (void) > > { > > symtab_node *node; > > > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; > > + node = safe_as_a<symtab_node *>(node->next)) > > if (node->definition) > > return node; > > > > @@ -3051,7 +3052,7 @@ inline varpool_node * > > symbol_table::first_variable (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > if (varpool_node *vnode = dyn_cast <varpool_node *> (node)) > > return vnode; > > return NULL; > > @@ -3061,8 +3062,8 @@ symbol_table::first_variable (void) > > inline varpool_node * > > symbol_table::next_variable (varpool_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > if (varpool_node *vnode1 = dyn_cast <varpool_node *> (node1)) > > return vnode1; > > return NULL; > > @@ -3078,7 +3079,7 @@ inline varpool_node * > > symbol_table::first_static_initializer (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > { > > varpool_node *vnode = dyn_cast <varpool_node *> (node); > > if (vnode && DECL_INITIAL (node->decl)) > > @@ -3091,8 +3092,8 @@ symbol_table::first_static_initializer (void) > > inline varpool_node * > > symbol_table::next_static_initializer (varpool_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > { > > varpool_node *vnode1 = dyn_cast <varpool_node *> (node1); > > if (vnode1 && DECL_INITIAL (node1->decl)) > > @@ -3111,7 +3112,7 @@ inline varpool_node * > > symbol_table::first_defined_variable (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > { > > varpool_node *vnode = dyn_cast <varpool_node *> (node); > > if (vnode && vnode->definition) > > @@ -3124,8 +3125,8 @@ symbol_table::first_defined_variable (void) > > inline varpool_node * > > symbol_table::next_defined_variable (varpool_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > { > > varpool_node *vnode1 = dyn_cast <varpool_node *> (node1); > > if (vnode1 && vnode1->definition) > > @@ -3143,7 +3144,7 @@ inline cgraph_node * > > symbol_table::first_defined_function (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > { > > cgraph_node *cn = dyn_cast <cgraph_node *> (node); > > if (cn && cn->definition) > > @@ -3156,8 +3157,8 @@ symbol_table::first_defined_function (void) > > inline cgraph_node * > > symbol_table::next_defined_function (cgraph_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > { > > cgraph_node *cn1 = dyn_cast <cgraph_node *> (node1); > > if (cn1 && cn1->definition) > > @@ -3176,7 +3177,7 @@ inline cgraph_node * > > symbol_table::first_function (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > if (cgraph_node *cn = dyn_cast <cgraph_node *> (node)) > > return cn; > > return NULL; > > @@ -3186,8 +3187,8 @@ symbol_table::first_function (void) > > inline cgraph_node * > > symbol_table::next_function (cgraph_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > if (cgraph_node *cn1 = dyn_cast <cgraph_node *> (node1)) > > return cn1; > > return NULL; > > @@ -3198,7 +3199,7 @@ inline cgraph_node * > > symbol_table::first_function_with_gimple_body (void) > > { > > symtab_node *node; > > - for (node = nodes; node; node = node->next) > > + for (node = nodes; node; node = safe_as_a<symtab_node *>(node->next)) > > { > > cgraph_node *cn = dyn_cast <cgraph_node *> (node); > > if (cn && cn->has_gimple_body_p ()) > > @@ -3211,8 +3212,8 @@ symbol_table::first_function_with_gimple_body (void) > > inline cgraph_node * > > symbol_table::next_function_with_gimple_body (cgraph_node *node) > > { > > - symtab_node *node1 = node->next; > > - for (; node1; node1 = node1->next) > > + symtab_node *node1 = safe_as_a<symtab_node *>(node->next); > > + for (; node1; node1 = safe_as_a<symtab_node *>(node1->next)) > > { > > cgraph_node *cn1 = dyn_cast <cgraph_node *> (node1); > > if (cn1 && cn1->has_gimple_body_p ()) > > diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc > > index a81f685654f..017c05750bb 100644 > > --- a/gcc/cgraphunit.cc > > +++ b/gcc/cgraphunit.cc > > @@ -1210,8 +1210,8 @@ analyze_functions (bool first_time) > > > > /* First identify the trivially needed symbols. */ > > for (node = symtab->first_symbol (); > > - node != first_analyzed > > - && node != first_analyzed_var; node = node->next) > > + node != first_analyzed && node != first_analyzed_var; > > + node = safe_as_a<symtab_node *>(node->next)) > > { > > /* Convert COMDAT group designators to IDENTIFIER_NODEs. */ > > node->get_comdat_group_id (); > > @@ -1373,7 +1373,7 @@ analyze_functions (bool first_time) > > node != first_handled > > && node != first_handled_var; node = next) > > { > > - next = node->next; > > + next = safe_as_a<symtab_node *>(node->next); > > /* For symbols declared locally we clear TREE_READONLY when emitting > > the constructor (if one is needed). For external declarations we > > can > > not safely assume that the type is readonly because we may be > > called > > @@ -1428,7 +1428,7 @@ analyze_functions (bool first_time) > > } > > node->aux = NULL; > > } > > - for (;node; node = node->next) > > + for (;node; node = safe_as_a<symtab_node *>(node->next)) > > node->aux = NULL; > > first_analyzed = symtab->first_function (); > > first_analyzed_var = symtab->first_variable (); > > @@ -2203,7 +2203,8 @@ output_in_order (void) > > && !DECL_HAS_VALUE_EXPR_P (vnode->decl)) > > nodes.safe_push (cgraph_order_sort (vnode)); > > > > - for (anode = symtab->first_asm_symbol (); anode; anode = anode->next) > > + for (anode = symtab->first_asm_symbol (); anode; > > + anode = safe_as_a<asm_node*>(anode->next)) > > nodes.safe_push (cgraph_order_sort (anode)); > > > > /* Sort nodes by order. */ > > diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc > > index d03c41f38e4..54f6110c933 100644 > > --- a/gcc/lto-streamer-out.cc > > +++ b/gcc/lto-streamer-out.cc > > @@ -2828,7 +2828,9 @@ lto_output (void) > > if (!flag_wpa) > > { > > asm_node *anode; > > - for (anode = symtab->first_asm_symbol (); anode; anode = anode->next) > > + for (anode = symtab->first_asm_symbol (); > > + anode; > > + anode = safe_as_a<asm_node*>(anode->next)) > > lto_set_symtab_encoder_in_partition (encoder, anode); > > } > > > > diff --git a/gcc/lto/lto-partition.cc b/gcc/lto/lto-partition.cc > > index 8c6ec6f8338..435a0d98a7e 100644 > > --- a/gcc/lto/lto-partition.cc > > +++ b/gcc/lto/lto-partition.cc > > @@ -383,7 +383,7 @@ lto_1_to_1_map (void) > > } > > > > struct asm_node *anode; > > - for (anode = symtab->first_asm_symbol (); anode; anode = anode->next) > > + for (anode = symtab->first_asm_symbol (); anode; anode = > > safe_as_a<asm_node*>(anode->next)) > > node_into_file_partition (anode, pmap); > > > > create_partition_if_empty (); > > @@ -406,7 +406,7 @@ create_asm_partition (void) > > if (anode) > > { > > ltrans_partition partition = new_partition ("asm_nodes"); > > - for (; anode; anode = anode->next) > > + for (; anode; anode = safe_as_a<asm_node*>(anode->next)) > > add_symbol_to_partition (partition, anode); > > } > > } > > diff --git a/gcc/symtab.cc b/gcc/symtab.cc > > index 3dbfad33ea2..a8b6091848b 100644 > > --- a/gcc/symtab.cc > > +++ b/gcc/symtab.cc > > @@ -1485,7 +1485,9 @@ symtab_node::verify_symtab_nodes (void) > > hash_map<tree, symtab_node *> comdat_head_map (251); > > asm_node *anode; > > > > - for (anode = symtab->first_asm_symbol (); anode; anode = anode->next) > > + for (anode = symtab->first_asm_symbol (); > > + anode; > > + anode = safe_as_a<asm_node*>(anode->next)) > > if (anode->order < 0 || anode->order >= symtab->order) > > { > > error ("invalid order in asm node %i", anode->order); > > -- > > 2.43.0 > >
