From 28944ed862aea49566950ca7aa3d7b741e71914f Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Fri, 5 May 2023 20:31:48 -0400
Subject: [PATCH] Fix crash on duplicate imported modules (#3231).

---
 Changes                              |  1 +
 src/V3LinkCells.cpp                  | 10 +++++++--
 test_regress/t/t_package_dup_bad.out | 21 +++++++++++++++++++
 test_regress/t/t_package_dup_bad.pl  | 19 +++++++++++++++++
 test_regress/t/t_package_dup_bad.v   | 31 ++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 test_regress/t/t_package_dup_bad.out
 create mode 100755 test_regress/t/t_package_dup_bad.pl
 create mode 100644 test_regress/t/t_package_dup_bad.v

diff --git a/Changes b/Changes
index bc034ee96..e88a895d6 100644
--- a/Changes
+++ b/Changes
@@ -24,6 +24,7 @@ Verilator 5.011 devel
 * Fix initialization order of initial static after function/task (#4159). [Kamil Rakoczy, Antmicro Ltd]
 * Fix linking AstRefDType if it has parameterized class ref (#4164) (#4170). [Ryszard Rozak, Antmicro Ltd]
 * Fix crash caused by $display() optimization (#4165) (#4166). [Tudor Timi]
+* Fix crash on duplicate imported modules (#3231). [Robert Balas]
 * Fix detection of wire/reg duplicates.
 
 
diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp
index 04639e6ad..d44ae8361 100644
--- a/src/V3LinkCells.cpp
+++ b/src/V3LinkCells.cpp
@@ -116,6 +116,7 @@ private:
     VSymGraph m_mods;  // Symbol table of all module names
     LinkCellsGraph m_graph;  // Linked graph of all cell interconnects
     LibraryVertex* m_libVertexp = nullptr;  // Vertex at root of all libraries
+    int m_dedupNum = 0;  // Package dedup number
     const V3GraphVertex* m_topVertexp = nullptr;  // Vertex of top module
     std::unordered_set<string> m_declfnWarned;  // Files we issued DECLFILENAME on
     string m_origTopModuleName;  // original name of the top module
@@ -504,8 +505,13 @@ private:
                                               << "... Location of original declaration\n"
                                               << foundp->warnContextSecondary());
                 }
-                nodep->unlinkFrBack();
-                VL_DO_DANGLING(pushDeletep(nodep), nodep);
+                if (VN_IS(nodep, Package)) {
+                    // Packages may be imported, we instead rename to be unique
+                    nodep->name(nodep->name() + "__Vdedup" + cvtToStr(m_dedupNum++));
+                } else {
+                    nodep->unlinkFrBack();
+                    VL_DO_DANGLING(pushDeletep(nodep), nodep);
+                }
             } else if (!foundp) {
                 m_mods.rootp()->insert(nodep->name(), new VSymEnt{&m_mods, nodep});
             }
diff --git a/test_regress/t/t_package_dup_bad.out b/test_regress/t/t_package_dup_bad.out
new file mode 100644
index 000000000..7cc93102e
--- /dev/null
+++ b/test_regress/t/t_package_dup_bad.out
@@ -0,0 +1,21 @@
+%Warning-MODDUP: t/t_package_dup_bad.v:11:9: Duplicate declaration of module: 'pkg'
+   11 | package pkg;
+      |         ^~~
+                 t/t_package_dup_bad.v:7:9: ... Location of original declaration
+    7 | package pkg;
+      |         ^~~
+                 ... For warning description see https://verilator.org/warn/MODDUP?v=latest
+                 ... Use "/* verilator lint_off MODDUP */" and lint_on around source to disable this message.
+%Warning-MODDUP: t/t_package_dup_bad.v:19:9: Duplicate declaration of module: 'pkg'
+   19 | package pkg;
+      |         ^~~
+                 t/t_package_dup_bad.v:7:9: ... Location of original declaration
+    7 | package pkg;
+      |         ^~~
+%Warning-MODDUP: t/t_package_dup_bad.v:22:9: Duplicate declaration of module: 'pkg'
+   22 | package pkg;
+      |         ^~~
+                 t/t_package_dup_bad.v:7:9: ... Location of original declaration
+    7 | package pkg;
+      |         ^~~
+%Error: Exiting due to
diff --git a/test_regress/t/t_package_dup_bad.pl b/test_regress/t/t_package_dup_bad.pl
new file mode 100755
index 000000000..c35c8bc93
--- /dev/null
+++ b/test_regress/t/t_package_dup_bad.pl
@@ -0,0 +1,19 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2022 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+lint(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_package_dup_bad.v b/test_regress/t/t_package_dup_bad.v
new file mode 100644
index 000000000..7a118fa59
--- /dev/null
+++ b/test_regress/t/t_package_dup_bad.v
@@ -0,0 +1,31 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2023 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+package pkg;
+   localparam PARAM = 10;
+endpackage
+
+package pkg;
+   localparam PARAM = 10;
+endpackage
+
+module sub import pkg::*;
+   #( ) ();
+endmodule
+
+package pkg;
+endpackage
+
+package pkg;
+endpackage
+
+module t (/*AUTOARG*/);
+   sub sub ();
+   initial begin
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule