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.

Sign up for GitHub

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

Jump to bottom

Memory corruption when parsing modules with triangle dependancy #550

Closed
veripoolbot opened this issue Aug 15, 2012 · 4 comments
Closed

Memory corruption when parsing modules with triangle dependancy #550

veripoolbot opened this issue Aug 15, 2012 · 4 comments
Labels
area: parser Issue involves SystemVerilog parsing resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor

Author Name: Ted Campbell
Original Redmine Issue: 550 from https://www.veripool.org
Original Date: 2012-08-15

There is memory corruption when modules are instantiated with a triangle pattern. The error only occurs if C is instantiated before B. Disabling the destructor of VSymGraph resolves the issue, but of course introduces a memory leak. It isn't obvious to me where the correct fix would go.

Fails against 3.840 and master branch. Passes on 3.833.

A
|\
| B
|/
C


// A.v
module A;
     C c();
     B b();
endmodule

// B.v
module B;
     C c();
endmodule

// C.v
module C;
     initial
         $display( "Hello World" );
endmodule


> valgrind verilator_bin --lint-only A.v

==2854== Invalid read of size 8
==2854==    at 0x40667D: VSymEnt::nodep() const (V3SymTable.h:96)
==2854==    by 0x44EA27: V3ParseImp::lexToken() (verilog.l:1130)
==2854==    by 0x44EB5B: V3ParseImp::lexToBison() (verilog.l:1150)
==2854==    by 0x40EBF3: yyparse() (verilog.c:7757)
==2854==    by 0x429D18: V3ParseImp::bisonParse() (verilog.y:3283)
==2854==    by 0x404BE6: V3ParseImp::lexFile(std::string const&) (V3ParseImp.cpp:156)
==2854==    by 0x40539E: V3ParseImp::parseFile(FileLine*, std::string const&, bool, std::string const&) (V3ParseImp.cpp:143)
==2854==    by 0x405417: V3Parse::parseFile(FileLine*, std::string const&, bool, std::string const&) (V3ParseImp.cpp:170)
==2854==    by 0x5905F8: LinkCellsVisitor::visit(AstCell*, AstNUser*) (V3LinkCells.cpp:206)
==2854==    by 0x4974AB: AstCell::accept(AstNVisitor&, AstNUser*) (V3AstNodes.h:1283)
==2854==    by 0x48ECB4: AstNode::iterateAndNext(AstNVisitor&, AstNUser*) (V3Ast.cpp:768)
==2854==    by 0x48EE0E: AstNode::iterateChildren(AstNVisitor&, AstNUser*) (V3Ast.cpp:748)
==2854==  Address 0x5982300 is 48 bytes inside a block of size 88 free'd
==2854==    at 0x4C26DCF: operator delete(void*) (vg_replace_malloc.c:387)
==2854==    by 0x40A43C: VSymGraph::~VSymGraph() (V3SymTable.h:221)
==2854==    by 0x40A4D1: V3ParseSym::~V3ParseSym() (V3ParseImp.h:200)
==2854==    by 0x405A12: V3ParseImp::~V3ParseImp() (V3ParseImp.cpp:67)
==2854==    by 0x405B12: V3Parse::~V3Parse() (V3ParseImp.cpp:166)
==2854==    by 0x5907B9: LinkCellsVisitor::visit(AstCell*, AstNUser*) (V3LinkCells.cpp:214)
==2854==    by 0x4974AB: AstCell::accept(AstNVisitor&, AstNUser*) (V3AstNodes.h:1283)
==2854==    by 0x48ECB4: AstNode::iterateAndNext(AstNVisitor&, AstNUser*) (V3Ast.cpp:768)
==2854==    by 0x48EE0E: AstNode::iterateChildren(AstNVisitor&, AstNUser*) (V3Ast.cpp:748)
==2854==    by 0x58FA5E: LinkCellsVisitor::visit(AstNodeModule*, AstNUser*) (V3LinkCells.cpp:182)
==2854==    by 0x47D189: AstNVisitor::visit(AstModule*, AstNUser*) (V3Ast__gen_visitor.h:119)
==2854==    by 0x42BDD9: AstModule::accept(AstNVisitor&, AstNUser*) (V3AstNodes.h:1222)
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-08-15T21:35:03Z

Unsurprisingly on my system valgrind is content. I'll walk through the changes and see if something around there jumps out at me.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-08-16T01:29:31Z

I'm not quite sure why this didn't fail in prior versions since the fundamental lifetime of the structure in question was wrong. The fix was to have the parse symbol table persist across all parse runs. This is probably more correct than before, but may result in some fallout if people relied on data types not being persistent across separately parsed cells.

Anyhow believe fixed in git towards 3.841.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Ted Campbell
Original Date: 2012-08-16T01:41:42Z

Thanks. My design now passes with latest git.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-09-04T00:15:11Z

In 3.841.

@veripoolbot veripoolbot added area: parser Issue involves SystemVerilog parsing resolution: fixed Closed; fixed labels Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: parser Issue involves SystemVerilog parsing resolution: fixed Closed; fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@veripoolbot