From e7d3e7440d27aa203d649b361b2c6b2204426397 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen <lars@metafoo.de>
Date: Sun, 25 Sep 2022 13:08:10 +0200
Subject: [PATCH 1/2] Prevent non-blocking writes to fields of automatic
 structs

The `PEIdent::has_aa_term()` method still uses the old `symbol_search()`
and will fail to find the variable if part of the identifier path is a
member select of a variable.

As a result such writes to fields of automatic structs can be classified as
static and it is possible to do non-blocking assignments to them. E.g.

```
task automatic t;
  struct packed {
    logic x;
  } s;
  s <= ...; // This fails
  s.x <= ...; // This works, but should fail
endtask
```

Switch to the new symbol search to make sure this case is handled
correctly. The new symbol search will correctly handle identifier paths
that have a trailing item after the variable, while the old symbol search
will always return an error in that case.

Note that while it is not allowed to do a non-blocking write to a class
object automatic variable, it is allowed to do a non-blocking write to a
property of a class object that is stored in an automatic variable, as the
non-blocking write is supposed to capture a reference to the object and not
reference the variable. E.g.

```
class C;
  int x;
endclass

task automatic t;
  C c;
  c <= ...; // Not allowed
  c.x <= ...; // Allowed
endtask
```

Non-blocking access to class properties is not yet support in
Icarus in general, but the error handling for that needs to be done
somewhere else.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 PExpr.cc | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/PExpr.cc b/PExpr.cc
index 2bd2b78d6..ff67da5e0 100644
--- a/PExpr.cc
+++ b/PExpr.cc
@@ -446,19 +446,17 @@ void PEIdent::declare_implicit_nets(LexicalScope*scope, NetNet::Type type)
 
 bool PEIdent::has_aa_term(Design*des, NetScope*scope) const
 {
-      NetNet*       net = 0;
-      ivl_type_t    cls_val;
-      const NetExpr*par = 0;
-      ivl_type_t    par_type;
-      NetEvent*     eve = 0;
+      symbol_search_results sr;
+      if (!symbol_search(this, des, scope, path_, &sr))
+	    return false;
 
-      scope = symbol_search(this, des, scope, path_, net, par, eve,
-                            par_type, cls_val);
+      // Class properties are not considered automatic since a non-blocking
+      // assignment to an object stored in an automatic variable is supposed to
+      // capture a reference to the object, not the variable.
+      if (!sr.path_tail.empty() && sr.net && sr.net->class_type())
+	    return false;
 
-      if (scope)
-            return scope->is_auto();
-      else
-            return false;
+      return sr.scope->is_auto();
 }
 
 PENewArray::PENewArray(PExpr*size_expr, PExpr*init_expr)

From 070fc2aedcdb324c926dfe26f16e9fac8fcee8a7 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen <lars@metafoo.de>
Date: Sat, 1 Oct 2022 19:15:07 +0200
Subject: [PATCH 2/2] Add regression tests for invalid non-blocking writes to
 SV constructs

Current regression tests only cover checking for invalid non-blocking
writes to constructs that are valid in Verilog. Add two tests to
additionally cover some SystemVerilog constructs.

 * Non-blocking writes to members of a struct typed variable with automatic
   lifetime
 * Non-blocking writes to class typed variables with automatic lifetime

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 ivtest/ivltests/automatic_error14.v | 18 ++++++++++++++++++
 ivtest/ivltests/automatic_error15.v | 19 +++++++++++++++++++
 ivtest/regress-sv.list              |  2 ++
 3 files changed, 39 insertions(+)
 create mode 100644 ivtest/ivltests/automatic_error14.v
 create mode 100644 ivtest/ivltests/automatic_error15.v

diff --git a/ivtest/ivltests/automatic_error14.v b/ivtest/ivltests/automatic_error14.v
new file mode 100644
index 000000000..2c1de6000
--- /dev/null
+++ b/ivtest/ivltests/automatic_error14.v
@@ -0,0 +1,18 @@
+// Check that it is not possible to perform non-blocking assignments to fields
+// of structs with automatic lifetime.
+
+module test;
+
+  task automatic auto_task;
+    struct packed {
+      logic x;
+    } s;
+    s.x <= 10;
+    $display("FAILED");
+  endtask
+
+  initial begin
+    auto_task;
+  end
+
+endmodule
diff --git a/ivtest/ivltests/automatic_error15.v b/ivtest/ivltests/automatic_error15.v
new file mode 100644
index 000000000..b3235bb29
--- /dev/null
+++ b/ivtest/ivltests/automatic_error15.v
@@ -0,0 +1,19 @@
+// Check that it is not possible to perform non-blocking assignments to a class
+// object variable with automatic lifetime.
+
+module test;
+
+  class C;
+  endclass
+
+  task automatic auto_task;
+    C c1, c2;
+    c1 <= c2;
+    $display("FAILED");
+  endtask
+
+  initial begin
+    auto_task;
+  end
+
+endmodule
diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list
index 9e133585b..35b55dfd1 100644
--- a/ivtest/regress-sv.list
+++ b/ivtest/regress-sv.list
@@ -104,6 +104,8 @@ assign_op_oob		normal,-g2009		ivltests
 assign_op_real_array	normal,-g2009		ivltests
 assign_op_real_array_oob normal,-g2009		ivltests
 assign_op_type		normal,-g2009		ivltests
+automatic_error14	CE,-g2005-sv		ivltests
+automatic_error15	CE,-g2005-sv		ivltests
 bitp1			normal,-g2005-sv	ivltests
 bits			normal,-g2005-sv	ivltests
 bits2			normal,-g2005-sv	ivltests