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

Segfault with certain typedef names #2647

Open
s-holst opened this issue Nov 12, 2024 · 4 comments
Open

Segfault with certain typedef names #2647

s-holst opened this issue Nov 12, 2024 · 4 comments

Comments

@s-holst
Copy link

s-holst commented Nov 12, 2024

Consider this verilog module:

module m;
    typedef enum { FALSE, TRUE } u;
    //typedef u [0:1] yeah;  // "yeah" works
    typedef u [0:1] crash;  // "crash" segfaults
endmodule

This crashes synlig in systemverilog_plugin::synlig_simplify and this crash seems to depend on the name of the second typedef.

Here is the crash in GDB:

$ gdb /cad/synlig/bin/synlig
GNU gdb (Ubuntu 12.1-0ubuntu1~22.04.2) 12.1
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /cad/synlig/bin/synlig...
(No debugging symbols found in /cad/synlig/bin/synlig)
(gdb) r
Starting program: /cad/synlig/bin/synlig 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

synlig> read_systemverilog -debug check.sv


1. Executing SystemVerilog frontend.
[INF:CM0023] Creating log file "/.../slpp_all/surelog.log".
[INF:CP0300] Compilation...
[INF:CP0303] /.../check.sv:1:1: Compile module "work@m".
[INF:CP0302] Compile class "work@mailbox".
[INF:CP0302] Compile class "work@process".
[INF:CP0302] Compile class "work@semaphore".
[INF:EL0526] Design Elaboration...
[NTE:EL0503] /.../check.sv:1:1: Top level module "work@m".
[NTE:EL0508] Nb Top level modules: 1.
[NTE:EL0509] Max instance depth: 1.
[NTE:EL0510] Nb instances: 1.
[NTE:EL0511] Nb leaf instances: 1.
[INF:UH0706] Creating UHDM Model...
[INF:UH0707] Elaborating UHDM...
[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 0
[WARNING] : 0
[   NOTE] : 5
Object 'work@m' of type 'design'
  Object 'builtin' of type 'package'
  Object 'work@m' of type 'module_inst'
    Object 'crash' of type 'packed_array_typespec'
      Object '' of type 'range'
        Object '' of type 'constant'
        Object '' of type 'constant'
      Object 'u' of type 'enum_typespec'
        Object 'FALSE' of type 'enum_const'
        Object 'TRUE' of type 'enum_const'
    Object 'u' of type 'enum_typespec'
      Object 'FALSE' of type 'enum_const'
      Object 'TRUE' of type 'enum_const'

Program received signal SIGSEGV, Segmentation fault.
0x00005555577b4200 in systemverilog_plugin::synlig_simplify(Yosys::AST::AstNode*, bool, bool, bool, int, int, bool, bool) ()
(gdb) 

I didn't manage to build synlig with debug symbols yet.
If you need more info, please let me know.

@kamilrakoczy
Copy link
Collaborator

kamilrakoczy commented Nov 14, 2024

Thanks for the report.

We are always building synlig with debug symbols, but we are stripping them on installation.

It looks like the problem here is related to order in which typedefs are parsed.
This order is based on name of the typedef.
So, when you have typedef with name u that is used in typedef named yeah, it works ok, as first we are parsing u and then yeah.
In second case, first we are parsing crash and then u. In this case, we recursively executing synlig_simplify until we are out of stack.

Are you willing to investigate it further to find out the root cause for it?

@s-holst
Copy link
Author

s-holst commented Nov 14, 2024

Thanks. I got debug symbols now and can confirm a infinite recursion here:

while (synlig_simplify(template_node, const_fold, at_zero, in_lvalue, stage, width_hint, sign_hint, in_param)) {

I can try to look into that, but I'm new to this code and don't know how long it will take (also busy with lots of other things). So, no promises ;)

@s-holst
Copy link
Author

s-holst commented Nov 14, 2024

Some observations from the debugger:
First crash gets resolved, then the u as dependency.
This operation involves convert_packed_unpacked_range (some sort of multi range workaround) that in turn calls resolve_wiretype that calls synlig_simplify. Everything works until this point.
Then, u gets processed again. Now convert_packed_unpacked_range -> resolve_wiretype -> synlig_simplify, and synlig_simplify is calling itself indefinitely.

I'm too unfamiliar with this code to get any deeper insight or come up with a fix with reasonable effort by myself.

Any help is very welcome

@kamilrakoczy
Copy link
Collaborator

kamilrakoczy commented Nov 22, 2024

Hi, @s-holst thanks for looking into it.

It turned out, that we were always overriding typedef name when processing packed enums (but we should do it only in case of anonymous enums). We fixed this in: #2670

While doing it, we noticed different issue: accessing packed enums doesn't work as expected, e.g.:

typedef enum { FALSE, TRUE } u;
typedef u [0:1] yeah;
yeah x;
assign x[0] = TRUE;
assign x[1] = FALSE;  

will be translated to:

logic [63:0] x;
assign x[1:0] = TRUE;
assign x[3:2] = FALSE;

but it should be changed to:

logic [63:0] x;
assign x[31:0] = TRUE;
assign x[63:32] = FALSE;

as a temporary workaround, you can wrap typedef definition into struct, such access works ok:

typedef enum { FALSE, TRUE } u;
typedef struct packed {
	u [1:0] e;
} yeah;

yeah x;
assign x.e[0] = TRUE;
assign x.e[1] = FALSE;

tgorochowik added a commit that referenced this issue Nov 22, 2024
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

No branches or pull requests

2 participants