From 19cdbc5a0ced3b02cddf6bba227c7901d9fbbf99 Mon Sep 17 00:00:00 2001 From: George Rennie Date: Wed, 4 Jun 2025 21:02:21 +0100 Subject: [PATCH 1/6] opt_dff: don't remove cells until all have been visited to prevent UAF --- passes/opt/opt_dff.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/passes/opt/opt_dff.cc b/passes/opt/opt_dff.cc index 8539432c0..726516fea 100644 --- a/passes/opt/opt_dff.cc +++ b/passes/opt/opt_dff.cc @@ -737,6 +737,7 @@ struct OptDffWorker bool run_constbits() { ModWalker modwalker(module->design, module); QuickConeSat qcsat(modwalker); + std::vector cells_to_remove; // Run as a separate sub-pass, so that we don't mutate (non-FF) cells under ModWalker. bool did_something = false; @@ -830,7 +831,7 @@ struct OptDffWorker if (!removed_sigbits.count(i)) keep_bits.push_back(i); if (keep_bits.empty()) { - module->remove(cell); + cells_to_remove.emplace_back(cell); did_something = true; continue; } @@ -840,6 +841,8 @@ struct OptDffWorker did_something = true; } } + for (auto* cell : cells_to_remove) + module->remove(cell); return did_something; } }; From 8c38e2081d3171ba6b374d6421b8a5a66f190697 Mon Sep 17 00:00:00 2001 From: George Rennie Date: Fri, 6 Jun 2025 23:46:07 +0100 Subject: [PATCH 2/6] opt_dff: don't emit cells until all have been visited to prevent UAF --- passes/opt/opt_dff.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/passes/opt/opt_dff.cc b/passes/opt/opt_dff.cc index 726516fea..4ed4b0cb6 100644 --- a/passes/opt/opt_dff.cc +++ b/passes/opt/opt_dff.cc @@ -737,9 +737,12 @@ struct OptDffWorker bool run_constbits() { ModWalker modwalker(module->design, module); QuickConeSat qcsat(modwalker); - std::vector cells_to_remove; - // Run as a separate sub-pass, so that we don't mutate (non-FF) cells under ModWalker. + // Defer mutating cells by removing them/emiting new flip flops so that + // cell references in modwalker are not invalidated + std::vector cells_to_remove; + std::vector ffs_to_emit; + bool did_something = false; for (auto cell : module->selected_cells()) { if (!RTLIL::builtin_ff_cell_types().count(cell->type)) @@ -837,12 +840,14 @@ struct OptDffWorker } ff = ff.slice(keep_bits); ff.cell = cell; - ff.emit(); + ffs_to_emit.emplace_back(ff); did_something = true; } } for (auto* cell : cells_to_remove) module->remove(cell); + for (auto& ff : ffs_to_emit) + ff.emit(); return did_something; } }; From 7160c9180051b49b5a5a9c0559c3d6cb469f089a Mon Sep 17 00:00:00 2001 From: George Rennie Date: Fri, 6 Jun 2025 23:46:23 +0100 Subject: [PATCH 3/6] tests: add test for #5164 opt_dff -sat UAF --- tests/opt/bug5164.ys | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/opt/bug5164.ys diff --git a/tests/opt/bug5164.ys b/tests/opt/bug5164.ys new file mode 100644 index 000000000..4ee71fe45 --- /dev/null +++ b/tests/opt/bug5164.ys @@ -0,0 +1,60 @@ +read_rtlil < Date: Thu, 19 Jun 2025 16:41:18 +0000 Subject: [PATCH 4/6] verilog: fix string literal regular expression (#5187) * verilog: fix string literal regular expression. A backslash was improperly quoted, causing string literal matching to fail when the final token before a closing quote was an escaped backslash. * verilog: add regression test for string literal regex bug. Test for bug triggered by escaped backslash immediately before closing quote (introduced in ca7d94af and fixed by 40aa7eaf). --- frontends/verilog/verilog_lexer.l | 2 +- tests/verilog/bug5160.v | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 tests/verilog/bug5160.v diff --git a/frontends/verilog/verilog_lexer.l b/frontends/verilog/verilog_lexer.l index 8148748d8..40162b8d3 100644 --- a/frontends/verilog/verilog_lexer.l +++ b/frontends/verilog/verilog_lexer.l @@ -336,7 +336,7 @@ TIME_SCALE_SUFFIX [munpf]?s } \" { BEGIN(STRING); } -([^\"]|\\.)+ { yymore(); real_location = old_location; } +([^\\"]|\\.)+ { yymore(); real_location = old_location; } \" { BEGIN(0); char *yystr = strdup(yytext); diff --git a/tests/verilog/bug5160.v b/tests/verilog/bug5160.v new file mode 100644 index 000000000..5b141a360 --- /dev/null +++ b/tests/verilog/bug5160.v @@ -0,0 +1,5 @@ +// Regression test for bug mentioned in #5160: +// https://github.com/YosysHQ/yosys/pull/5160#issuecomment-2983643084 +module top; + initial $display( "\\" ); +endmodule From 44aa313ba976b68476aaa3d548098d7f54e61094 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 20 Jun 2025 00:24:40 +0000 Subject: [PATCH 5/6] Bump version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b452105b1..e742928b3 100644 --- a/Makefile +++ b/Makefile @@ -160,7 +160,7 @@ ifeq ($(OS), Haiku) CXXFLAGS += -D_DEFAULT_SOURCE endif -YOSYS_VER := 0.54+15 +YOSYS_VER := 0.54+17 YOSYS_MAJOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f1) YOSYS_MINOR := $(shell echo $(YOSYS_VER) | cut -d'.' -f2 | cut -d'+' -f1) YOSYS_COMMIT := $(shell echo $(YOSYS_VER) | cut -d'+' -f2) From 34a2abeddb5054df29140baf1c6d5af02a9aac63 Mon Sep 17 00:00:00 2001 From: Gary Wong Date: Fri, 20 Jun 2025 17:26:20 -0600 Subject: [PATCH 6/6] verilog: fix parser "if" memory errors. Fix buggy memory allocation introduced in #5152: 1) clean up ast_stack to reflect AST node rearrangement when necessary, to avoid dangling pointer; 2) call free_attr() on unused attribute list when no new syntax node is created, to avoid leaking it. --- frontends/verilog/verilog_parser.y | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 7e53005f3..17edc357d 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -2874,6 +2874,7 @@ behavioral_stmt: } | if_attr TOK_IF '(' expr ')' { AstNode *node = 0; + AstNode *block = new AstNode(AST_BLOCK); AstNode *context = ast_stack.back(); if (context && context->type == AST_BLOCK && context->get_bool_attribute(ID::promoted_if)) { AstNode *outer = ast_stack[ast_stack.size() - 2]; @@ -2882,8 +2883,10 @@ behavioral_stmt: // parallel "else if": append condition to outer "if" node = outer; log_assert (node->children.size()); + ast_stack.pop_back(); delete node->children.back(); node->children.pop_back(); + ast_stack.push_back(block); } else if (outer->get_bool_attribute(ID::full_case)) (*$1)[ID::full_case] = AstNode::mkconst_int(1, false); } @@ -2894,8 +2897,8 @@ behavioral_stmt: append_attr(node, $1); ast_stack.back()->children.push_back(node); node->children.push_back(node->get_bool_attribute(ID::parallel_case) ? AstNode::mkconst_int(1, false, 1) : expr); - } - AstNode *block = new AstNode(AST_BLOCK); + } else + free_attr($1); AstNode *cond = new AstNode(AST_COND, node->get_bool_attribute(ID::parallel_case) ? expr : AstNode::mkconst_int(1, false, 1), block); SET_AST_NODE_LOC(cond, @4, @4); node->children.push_back(cond);