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. I do wonder what's the C++ "pattern" to have the base class next/previous pointers statically typed to the derived class type? 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? 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 >
