verilog: rebuild user_type_stack from globals before parsing file
authorXiretza <xiretza@xiretza.xyz>
Tue, 16 Mar 2021 15:42:14 +0000 (16:42 +0100)
committerZachary Snow <zachary.j.snow@gmail.com>
Fri, 19 Mar 2021 00:52:36 +0000 (20:52 -0400)
This was actually a ticking UB bomb: after running the parser, the type
maps contain pointers to children of the current AST, which is
recursively deleted after the pass has executed. This leaves the
pointers in user_type_stack dangling, which just happened to never be a
problem due to another bug that causes typedefs from higher-level type
maps to never be considered.

Rebuilding the type stack from the design's globals ensures the AstNode
pointers are valid.

frontends/verilog/verilog_frontend.cc

index 5907707c8c7d972d3f3e42debb2125f43ee9e64b..84ac73e910ff86abf95754e60cbf402a5fb36760 100644 (file)
@@ -61,11 +61,6 @@ static void add_package_types(dict<std::string, AST::AstNode *> &user_types, std
                        }
                }
        }
-
-       // carry over typedefs from previous files, but allow them to be overridden
-       // note that these type maps are currently never reclaimed
-       if (user_type_stack.empty() || !user_type_stack.back()->empty())
-               user_type_stack.push_back(new UserTypeMap());
 }
 
 struct VerilogFrontend : public Frontend {
@@ -487,6 +482,19 @@ struct VerilogFrontend : public Frontend {
                // make package typedefs available to parser
                add_package_types(pkg_user_types, design->verilog_packages);
 
+               UserTypeMap *global_types_map = new UserTypeMap();
+               for (auto def : design->verilog_globals) {
+                       if (def->type == AST::AST_TYPEDEF) {
+                               (*global_types_map)[def->str] = def;
+                       }
+               }
+
+               log_assert(user_type_stack.empty());
+               // use previous global typedefs as bottom level of user type stack
+               user_type_stack.push_back(global_types_map);
+               // add a new empty type map to allow overriding existing global definitions
+               user_type_stack.push_back(new UserTypeMap());
+
                frontend_verilog_yyset_lineno(1);
                frontend_verilog_yyrestart(NULL);
                frontend_verilog_yyparse();
@@ -509,6 +517,14 @@ struct VerilogFrontend : public Frontend {
                if (!flag_nopp)
                        delete lexin;
 
+               // only the previous and new global type maps remain
+               log_assert(user_type_stack.size() == 2);
+               for (auto it : user_type_stack) {
+                       // the global typedefs have to remain valid for future invocations, so just drop the map without deleting values
+                       delete it;
+               }
+               user_type_stack.clear();
+
                delete current_ast;
                current_ast = NULL;