Fix errors on using string in incorrect format (#5340).

Changes

@@ -1532,6 +1532,7 @@ Verilator 4.026 2020-01-11
 * Fix expand optimization slowing --lint-only. Closes #2091. [Thomas Watts]
 * Fix %{number}s with strings. #2093. [agrobman]
 * Fix shebang breaking some shells. Closes #2067. [zdave]
+* Fix errors on using string in incorrect format (#5340). [John Demme]
 
 
 Verilator 4.024 2019-12-08

src/V3Width.cpp

@@ -5033,6 +5033,13 @@ class WidthVisitor final : public VNVisitor {
         UASSERT_OBJ(nodep->lhsp()->dtypep()->widthSized(), nodep, "How can LValue be unsized?");
     }
 
+    void formatNoStringArg(AstNode* argp, char ch) {
+        if (argp && argp->isString()) {
+            argp->v3error("$display-line format of '%"s + ch + "' illegal with string argument\n"
+                          << argp->warnMore() << "... Suggest use '%s'");
+        }
+    }
+
     void visit(AstSFormatF* nodep) override {
         // Excludes NodeDisplay, see below
         if (m_vup && !m_vup->prelim()) return;  // Can be called as statement or function
@@ -5056,6 +5063,7 @@ class WidthVisitor final : public VNVisitor {
                 bool added = false;
                 const AstNodeDType* const dtypep = argp ? argp->dtypep()->skipRefp() : nullptr;
                 const AstBasicDType* const basicp = dtypep ? dtypep->basicp() : nullptr;
+                ch = std::tolower(ch);
                 if (ch == '?') {  // Unspecified by user, guess
                     if (argp && argp->isDouble()) {
                         ch = 'g';
@@ -5069,13 +5077,14 @@ class WidthVisitor final : public VNVisitor {
                         ch = 'p';
                     }
                 }
-                switch (std::tolower(ch)) {
+                switch (ch) {
                 case '%': break;  // %% - just output a %
                 case 'm': break;  // %m - auto insert "name"
                 case 'l': break;  // %m - auto insert "library"
                 case 'd': {  // Convert decimal to either 'd' or '#'
                     if (argp) {
                         AstNodeExpr* const nextp = VN_AS(argp->nextp(), NodeExpr);
+                        formatNoStringArg(argp, ch);
                         if (argp->isDouble()) {
                             spliceCvtS(argp, true, 64);
                             ch = '~';
@@ -5091,6 +5100,7 @@ class WidthVisitor final : public VNVisitor {
                 case 'x': {
                     if (argp) {
                         AstNodeExpr* const nextp = VN_AS(argp->nextp(), NodeExpr);
+                        formatNoStringArg(argp, ch);
                         if (argp->isDouble()) spliceCvtS(argp, true, 64);
                         argp = nextp;
                     }
@@ -5148,6 +5158,7 @@ class WidthVisitor final : public VNVisitor {
                 case 't': {  // Convert decimal time to realtime
                     if (argp) {
                         AstNodeExpr* const nextp = VN_AS(argp->nextp(), NodeExpr);
+                        formatNoStringArg(argp, ch);
                         if (argp->isDouble()) ch = '^';  // Convert it
                         UASSERT_OBJ(!nodep->timeunit().isNone(), nodep,
                                     "display %t has no time units");
@@ -5160,6 +5171,7 @@ class WidthVisitor final : public VNVisitor {
                 case 'g': {
                     if (argp) {
                         AstNodeExpr* const nextp = VN_AS(argp->nextp(), NodeExpr);
+                        formatNoStringArg(argp, ch);
                         if (!argp->isDouble()) {
                             iterateCheckReal(nodep, "Display argument", argp, BOTH);
                         }

test_regress/t/t_display_type_bad.out

@@ -0,0 +1,21 @@
+%Error: t/t_display_type_bad.v:10:31: $display-line format of '%d' illegal with string argument
+                                    : ... note: In instance 't'
+                                    : ... Suggest use '%s'
+   10 |       $display("%d %x %f %t", s, s, s, s);
+      |                               ^
+%Error: t/t_display_type_bad.v:10:34: $display-line format of '%x' illegal with string argument
+                                    : ... note: In instance 't'
+                                    : ... Suggest use '%s'
+   10 |       $display("%d %x %f %t", s, s, s, s);
+      |                                  ^
+%Error: t/t_display_type_bad.v:10:37: $display-line format of '%f' illegal with string argument
+                                    : ... note: In instance 't'
+                                    : ... Suggest use '%s'
+   10 |       $display("%d %x %f %t", s, s, s, s);
+      |                                     ^
+%Error: t/t_display_type_bad.v:10:40: $display-line format of '%t' illegal with string argument
+                                    : ... note: In instance 't'
+                                    : ... Suggest use '%s'
+   10 |       $display("%d %x %f %t", s, s, s, s);
+      |                                        ^
+%Error: Exiting due to

test_regress/t/t_display_type_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 2003 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(vlt => 1);
+
+lint(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_display_type_bad.v

@@ -0,0 +1,14 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2003 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module t;
+   string s = "a string";
+   initial begin
+      $display("%d %x %f %t", s, s, s, s);
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule