Makopo/lslint

Segfault when using a naked identifyer after decleration in the global scope

Closed this issue · 1 comments

This script will crash lslint:

integer a;
b;
default { state_entry() {} }

Tested with latest from git and with 1.0.9.

Backtrace from gdb:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004061cc in LLASTNode::get_next (this=0x0) at ast.hh:89
89          LLASTNode *get_next() { return next; }
(gdb) bt
#0  0x00000000004061cc in LLASTNode::get_next (this=0x0) at ast.hh:89
#1  0x0000000000401a8a in yyparse (scanner=0x7dbaf0) at lslmini.y:246
#2  0x00000000004131d9 in main (argc=2, argv=0x7fffffffdfb8) at lslmini.cc:1060

Thanks, I can reproduce that. You don't even need a complete script, just integer a; b; suffices, because it's crashing while parsing, in this line (as frame #1 in your backtrace shows):

        if ( $1 ) {
            DEBUG( LOG_DEBUG_SPAM, NULL, "** global [%p,%p] globals [%p,%p]\n", $1->get_prev(), $1->get_next(), $2->get_prev(), $2->get_next());

A git bisect shows that the crash is happening as a consequence of this commit: 6724501

It looks to me that since that commit, $2 can be NULL as a result of error recovery, and that's not checked. Based on that hypothesis, this patch indeed solves the crash:

--- a/lslmini.y
+++ b/lslmini.y
@@ -243,8 +243,10 @@ globals
 	| global globals
 	{
 		if ( $1 ) {
-			DEBUG( LOG_DEBUG_SPAM, NULL, "** global [%p,%p] globals [%p,%p]\n", $1->get_prev(), $1->get_next(), $2->get_prev(), $2->get_next());
-			$1->add_next_sibling($2);
+			if ( $2 ) {
+				DEBUG( LOG_DEBUG_SPAM, NULL, "** global [%p,%p] globals [%p,%p]\n", $1->get_prev(), $1->get_next(), $2->get_prev(), $2->get_next());
+				$1->add_next_sibling($2);
+			}
 			$$ = $1;
 		} else {
 			$$ = $2;

I'm not totally sure about that patch, though, because these two scripts produce different output:

integer a;
b;
default { state_entry() {} }

Output:

ERROR:: (  2,  2): syntax error, unexpected ';', expecting '('
 WARN:: (  1,  9): variable `a' declared but never used.
TOTAL:: Errors: 1  Warnings: 1

and

integer a;
b;
integer c;
default { state_entry() {} }

Output:

ERROR:: (  2,  2): syntax error, unexpected ';', expecting '('
TOTAL:: Errors: 1  Warnings: 0

That's a bit worrying and I need to look further into why the second script doesn't produce any warnings for unused identifiers.

I'll look more into it, thanks for the report.