Skip to content

Fix internal error on integer/char formatting of a null handle - #1944

Open
EylonKrause wants to merge 1 commit into
MikePopoloski:masterfrom
EylonKrause:fix-sformatf-null-int-specifiers
Open

Fix internal error on integer/char formatting of a null handle#1944
EylonKrause wants to merge 1 commit into
MikePopoloski:masterfrom
EylonKrause:fix-sformatf-null-int-specifiers

Conversation

@EylonKrause

Copy link
Copy Markdown
Contributor

The integer and char format specifiers (%h %x %d %o %b %c) accept a class handle, a chandle, or the null literal — checkArgType() explicitly permits those types. In a constant context, though, formatArg formatted them via arg.convertToInt().integer(), and convertToInt() returns an invalid (bad) ConstantValue for a null handle, so integer() does std::get<SVInt> on the wrong variant and aborts with internal compiler error: std::get: wrong index for variant.

Repro

module m; localparam string S = $sformatf("%d", null); endmodule

Also crashes with %x / %o / %b / %h / %c, and with a chandle in a constant function:

module m;
  function automatic string f(); chandle c = null; return $sformatf("%x", c); endfunction
  localparam string S = f();
endmodule

Fix

Guard the conversion in a small helper: when convertToInt() produces no usable integer, format the null handle as its numeric value of zero (SVInt::Zero) instead of dereferencing the missing value. This mirrors the variant-aware fix in #1916 for the %u/%z specifiers and matches how a null handle formats numerically. (A class handle is already rejected earlier in a constant context, so the null literal and a chandle are the values that actually reach formatArg.)

Added a regression test in EvalTests.cpp; the full unit test suite passes (22972 assertions in 2487 test cases).

Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission.

The integer and char format specifiers (%h %x %d %o %b %c) accept a class
handle, a chandle, or the null literal: checkArgType() explicitly permits
those types. But formatArg formatted them via arg.convertToInt().integer(),
and convertToInt() yields an invalid (bad) ConstantValue for a null handle,
so integer() does std::get<SVInt> on the wrong variant and aborts with
"internal compiler error: std::get: wrong index for variant".

Guard the conversion in a small helper: when convertToInt() produces no
usable integer, format the null handle as its numeric value of zero
(SVInt::Zero) instead of dereferencing the missing value. This mirrors the
variant-aware fix in MikePopoloski#1916 for the %u/%z specifiers and matches how a null
handle formats numerically. A class handle is already rejected earlier in a
constant context, but the null literal and a chandle reach formatArg.

Signed-off-by: Eylon Krause <eylon1909@gmail.com>
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.03%. Comparing base (a5ab852) to head (1b0a64b).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1944   +/-   ##
=======================================
  Coverage   96.03%   96.03%           
=======================================
  Files         247      247           
  Lines       56855    56860    +5     
=======================================
+ Hits        54599    54605    +6     
+ Misses       2256     2255    -1     
Files with missing lines Coverage Δ
source/ast/SFormat.cpp 97.57% <100.00%> (+0.03%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5ab852...1b0a64b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant