Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug][slang] Fix case selector huge bitwidth segmentation fault #1217

Closed

Conversation

likeamahoney
Copy link
Contributor

@likeamahoney likeamahoney commented Jan 20, 2025

Hi @MikePopoloski!

There are a two examples of casex which works incorrect with slang:

// This one is segfaults on release and debug builds

module test9(
  input reg [100:0] sel_i,
  input reg trg_i,
  output reg [1:0] reg_o
);
  always @(posedge trg_i) begin
    casex (sel_i)
      -12'b00zzzzz00001,
      12'b11: reg_o = 2'b01;
    endcase
  end
endmodule

and

// This one produce the strange error on debug build:
// internal compiler error: Assertion 'noteLocation' failed
// in file /home/yan/projects/slang/source/diagnostics/Diagnostics.cpp, line 30
//  function: slang::Diagnostic& slang::Diagnostic::addNote(slang::DiagCode, slang::SourceLocation)

module test9(
  input reg [100:0] sel_i,
  input reg trg_i,
  output reg [1:0] reg_o
);
  always @(posedge trg_i) begin
    casex (sel_i)
      12'sb00xxxxx00001,
      -12'b00zzzzz00001,
      12'b11: reg_o = 2'b01;
    endcase
  end
endmodule

I think the whole point is that it is dangerous to use unions (for example - here), especially since it is impossible to check which type was actually written to the tree node. This can also happen because of different bit lengths of labels (in one case - for the first label value.getBitWidth() returns 12 (leading zeros in front), and in the other 101).

@MikePopoloski
Copy link
Owner

Thanks for the report, but this is not the right fix. The constant value should all be sized the same. There is an invariant being broken where the constant value of the expression is not sized to the width of the expression's type, due to some of the other warning analysis stuff I've added recently. #1222 is related.

@MikePopoloski
Copy link
Owner

Superseded by e9107cf

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.

2 participants