diff --git a/Changes b/Changes index cfbe0328d..4a7355dae 100644 --- a/Changes +++ b/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 diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 62d812b64..2b1c78aad 100644 --- a/src/V3Width.cpp +++ b/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); } diff --git a/test_regress/t/t_display_type_bad.out b/test_regress/t/t_display_type_bad.out new file mode 100644 index 000000000..5d68880f5 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_display_type_bad.pl b/test_regress/t/t_display_type_bad.pl new file mode 100755 index 000000000..a5846c699 --- /dev/null +++ b/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; diff --git a/test_regress/t/t_display_type_bad.v b/test_regress/t/t_display_type_bad.v new file mode 100644 index 000000000..ff17cf8b3 --- /dev/null +++ b/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