Fix jumping over object initialization (#4411)

src/V3AstNodeOther.h

@@ -699,6 +699,16 @@ public:
                && finalsp() == nullptr;
     }
 };
+class AstCLocalScope final : public AstNode {
+    // Pack statements into an unnamed scope when generating C++
+    // @astgen op1 := stmtsp : List[AstNode]
+public:
+    AstCLocalScope(FileLine* fl, AstNode* stmtsp)
+        : ASTGEN_SUPER_CLocalScope(fl) {
+        this->addStmtsp(stmtsp);
+    }
+    ASTGEN_MEMBERS_AstCLocalScope;
+};
 class AstCUse final : public AstNode {
     // C++ use of a class or #include; indicates need of forward declaration
     // Parents:  NODEMODULE

src/V3Const.cpp

@@ -2288,6 +2288,13 @@ private:
             iterateChildren(nodep);
         }
     }
+    void visit(AstCLocalScope* nodep) override {
+        iterateChildren(nodep);
+        if (!nodep->stmtsp()) {
+            nodep->unlinkFrBack();
+            VL_DO_DANGLING(nodep->deleteTree(), nodep);
+        }
+    }
     void visit(AstScope* nodep) override {
         // No ASSIGNW removals under scope, we've long eliminated INITIALs
         VL_RESTORER(m_wremove);

src/V3EmitCFunc.h

@@ -883,6 +883,11 @@ public:
         iterateAndNextConstNull(nodep->endStmtsp());
         puts("}\n");
     }
+    void visit(AstCLocalScope* nodep) override {
+        puts("{\n");
+        iterateAndNextConstNull(nodep->stmtsp());
+        puts("}\n");
+    }
     void visit(AstJumpGo* nodep) override {
         puts("goto __Vlabel" + cvtToStr(nodep->labelp()->blockp()->labelNum()) + ";\n");
     }

src/V3Timing.cpp

@@ -52,6 +52,7 @@
 #include "V3Ast.h"
 #include "V3Const.h"
 #include "V3EmitV.h"
+#include "V3Global.h"
 #include "V3Graph.h"
 #include "V3MemberMap.h"
 #include "V3SenExprBuilder.h"
@@ -388,6 +389,7 @@ private:
     AstNode* m_procp = nullptr;  // NodeProcedure/CFunc/Begin we're under
     double m_timescaleFactor = 1.0;  // Factor to scale delays by
     int m_forkCnt = 0;  // Number of forks inside a module
+    bool m_underJumpBlock = false;  // True if we are inside of a jump-block
 
     // Unique names
     V3UniqueNames m_contAssignVarNames{"__VassignWtmp"};  // Names for temp AssignW vars
@@ -594,6 +596,17 @@ private:
         m_netlistp->typeTablep()->addTypesp(m_forkDtp);
         return m_forkDtp;
     }
+    // Move `insertBeforep` into `AstCLocalScope` if necessary to avoid jumping over
+    // a variable initialization that whould be inserted before `insertBeforep`. All
+    // access to this variable shoule be contained within returned `AstCLocalScope`.
+    AstCLocalScope* addCLocalScope(FileLine* const flp, AstNode* const insertBeforep) const {
+        if (!insertBeforep || !m_underJumpBlock) return nullptr;
+        VNRelinker handle;
+        insertBeforep->unlinkFrBack(&handle);
+        AstCLocalScope* const cscopep = new AstCLocalScope{flp, insertBeforep};
+        handle.relink(cscopep);
+        return cscopep;
+    }
     // Create a temp variable and optionally put it before the specified node (mark local if so)
     AstVarScope* createTemp(FileLine* const flp, const std::string& name,
                             AstNodeDType* const dtypep, AstNode* const insertBeforep = nullptr) {
@@ -625,6 +638,7 @@ private:
         FileLine* const flp = forkp->fileline();
         // If we're in a function, insert the sync var directly before the fork
         AstNode* const insertBeforep = m_classp ? forkp : nullptr;
+        addCLocalScope(flp, insertBeforep);
         AstVarScope* forkVscp
             = createTemp(flp, forkp->name() + "__sync", getCreateForkSyncDTypep(), insertBeforep);
         unsigned joinCount = 0;  // Needed for join counter
@@ -691,6 +705,11 @@ private:
                 new AstCStmt{nodep->fileline(), "vlProcess->state(VlProcess::FINISHED);\n"});
         }
     }
+    void visit(AstJumpBlock* nodep) override {
+        VL_RESTORER(m_underJumpBlock);
+        m_underJumpBlock = true;
+        visit(static_cast<AstNodeStmt*>(nodep));
+    }
     void visit(AstAlways* nodep) override {
         if (nodep->user1SetOnce()) return;
         VL_RESTORER(m_procp);
@@ -908,6 +927,7 @@ private:
         // Insert new vars before the timing control if we're in a function; in a process we can't
         // do that. These intra-assignment vars will later be passed to forked processes by value.
         AstNode* const insertBeforep = m_classp ? controlp : nullptr;
+        addCLocalScope(flp, insertBeforep);
         // Function for replacing values with intermediate variables
         const auto replaceWithIntermediate = [&](AstNodeExpr* const valuep,
                                                  const std::string& name) {

test_regress/t/t_jumps_uninit_destructor_call.pl

@@ -0,0 +1,23 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 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);
+
+compile(
+    verilator_flags2 => ["--exe --main --timing"],
+    make_main => 0,
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_jumps_uninit_destructor_call.v

@@ -0,0 +1,45 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2023 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+class Foo;
+  string arra[2];
+  string arrb[string];
+
+  function new();
+    arra = '{"baz", "boo"};
+    arrb = '{"baz": "inga!", "boo": "..."};
+  endfunction
+
+  task automatic return_before_fork(bit b);
+    if (b) begin
+      // This is going to translate to a `goto` statement.
+      return;
+    end
+    // This will instantiate a `VlForkSync` object (local to the call, as we are under a class)
+    fork
+      #10 $display("forked process");
+    join
+    // This is where we jump to from the aforementioned goto. If we don't wrap the `VlForkSync`
+    // object in a scope that ends before that point, we will end up calling its destructor
+    // without having it initialized first.
+  endtask
+  task automatic return_before_select(bit b, int idx);
+    if (b) return; // goto
+    // This will create two temporary strings used to select from `arrb` and assign to it.
+    arrb[arra[idx]] = #10 "yah!";
+    // jump here
+  endtask
+endclass
+
+module t();
+  initial begin
+    Foo foo;
+    foo = new;
+    foo.return_before_fork(1'b1);
+    foo.return_before_select(1'b1, 1);
+    $write("*-* All Finished *-*\n");
+    $finish;
+  end
+endmodule