From ebd5dc81cc91aa45ca6e15820471f0909d42a08e Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 15 Oct 2020 16:58:35 +0100
Subject: [PATCH] Fix some more cases of malformed syntax crashing the
 compiler.

Fixes https://gitlab.com/kicad/code/kicad/issues/6016
---
 common/libeval_compiler/libeval_compiler.cpp | 58 +++++++++++++++-----
 pcbnew/drc/drc_rule_parser.cpp               |  5 +-
 pcbnew/pcb_expr_evaluator.cpp                |  3 +-
 3 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/common/libeval_compiler/libeval_compiler.cpp b/common/libeval_compiler/libeval_compiler.cpp
index 20e6e350c7..d8b97f6613 100644
--- a/common/libeval_compiler/libeval_compiler.cpp
+++ b/common/libeval_compiler/libeval_compiler.cpp
@@ -892,7 +892,7 @@ bool COMPILER::generateUCode( UCODE* aCode, CONTEXT* aPreflightContext )
                     {
                         // Preflight the function call
 
-                        for( auto pnode : params )
+                        for( TREE_NODE* pnode : params )
                         {
                             VALUE*   param = aPreflightContext->AllocValue();
                             param->Set( *pnode->value.str );
@@ -918,7 +918,7 @@ bool COMPILER::generateUCode( UCODE* aCode, CONTEXT* aPreflightContext )
 
                     node->leaf[0]->isVisited = true;
                     node->leaf[1]->isVisited = true;
-                    node->leaf[1]->leaf[0]->isVisited = true;;
+                    node->leaf[1]->leaf[0]->isVisited = true;
                     node->leaf[1]->leaf[1]->isVisited = true;
 
                     // Our non-terminal-node stacking algorithm can't handle doubly-nested
@@ -926,16 +926,38 @@ bool COMPILER::generateUCode( UCODE* aCode, CONTEXT* aPreflightContext )
                     // a TR_OP_FUNC_CALL and its function parameter
                     stack.pop_back();
                     stack.push_back( node->leaf[1] );
-                    //stack.push_back( node->leaf[1]->leaf[1] );
-                    for( auto pnode : params )
-                    {
+
+                    for( TREE_NODE* pnode : params )
                         stack.push_back( pnode );
-                    }
 
                     node->leaf[1]->SetUop( TR_OP_METHOD_CALL, func, std::move( vref ) );
                     node->isTerminal = false;
                     break;
                 }
+
+                default:
+                    // leaf[0]: object
+                    // leaf[1]: malformed syntax
+
+                    wxString itemName = *node->leaf[0]->value.str;
+                    wxString propName = *node->leaf[1]->value.str;
+                    std::unique_ptr<VAR_REF> vref = aCode->CreateVarRef( itemName, propName );
+
+                    if( !vref )
+                    {
+                        msg.Printf( _( "Unrecognized item '%s'" ), itemName );
+                        reportError( CST_CODEGEN, msg, node->leaf[0]->srcPos - (int) itemName.length() );
+                    }
+
+                    msg.Printf( _( "Unrecognized property '%s'" ), propName );
+                    reportError( CST_CODEGEN, msg, node->leaf[0]->srcPos + 1 );
+
+                    node->leaf[0]->isVisited = true;
+                    node->leaf[1]->isVisited = true;
+
+                    node->SetUop( TR_UOP_PUSH_VALUE, 0.0 );
+                    node->isTerminal = true;
+                    break;
             }
             break;
         }
@@ -1001,13 +1023,13 @@ bool COMPILER::generateUCode( UCODE* aCode, CONTEXT* aPreflightContext )
             if( node->leaf[0] && !node->leaf[0]->isVisited )
             {
                 stack.push_back( node->leaf[0] );
-                node->leaf[0]->isVisited = true;;
+                node->leaf[0]->isVisited = true;
                 continue;
             }
             else if( node->leaf[1] && !node->leaf[1]->isVisited )
             {
                 stack.push_back( node->leaf[1] );
-                node->leaf[1]->isVisited = true;;
+                node->leaf[1]->isVisited = true;
             }
 
             continue;
@@ -1149,15 +1171,21 @@ VALUE* UCODE::Run( CONTEXT* ctx )
         return &g_false;
     }
 
-    assert( ctx->SP() == 1 );
+    if( ctx->SP() == 1 )
+    {
+        return ctx->Pop();
+    }
+    else
+    {
+        // If stack is corrupted after execution it suggests a problem with the compiler, not
+        // the rule....
 
-    // non-well-formed rules should not be fired
-    // fixme: not sure it's a good idea, if stack is corrupted after execution it means
-    // a problem with the compiler, not the rule...
-    if( ctx->SP() != 1 )
+        // do not use "assert"; it crashes outright on OSX
+        wxASSERT( ctx->SP() == 1 );
+
+        // non-well-formed rules should not be fired on a release build
         return &g_false;
-
-    return ctx->Pop();
+    }
 }
 
 
diff --git a/pcbnew/drc/drc_rule_parser.cpp b/pcbnew/drc/drc_rule_parser.cpp
index 29893a3f98..332a7ca57a 100644
--- a/pcbnew/drc/drc_rule_parser.cpp
+++ b/pcbnew/drc/drc_rule_parser.cpp
@@ -438,7 +438,7 @@ void DRC_RULES_PARSER::parseValueWithUnits( const wxString& aExpr, int& aResult
         {
             wxString msg = wxString::Format( _( "ERROR: <a href='%d:%d'>%s</a>%s" ),
                                              CurLineNumber(),
-                                             CurOffset(),
+                                             CurOffset() + aOffset,
                                              first,
                                              rest );
 
@@ -450,7 +450,8 @@ void DRC_RULES_PARSER::parseValueWithUnits( const wxString& aExpr, int& aResult
                                              first,
                                              rest );
 
-            THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() );
+            THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(),
+                               CurOffset() + aOffset );
         }
     };
 
diff --git a/pcbnew/pcb_expr_evaluator.cpp b/pcbnew/pcb_expr_evaluator.cpp
index d1369b6ded..9cafe92452 100644
--- a/pcbnew/pcb_expr_evaluator.cpp
+++ b/pcbnew/pcb_expr_evaluator.cpp
@@ -578,7 +578,8 @@ bool PCB_EXPR_EVALUATOR::Evaluate( const wxString& aExpr )
     PCB_EXPR_UCODE   ucode;
     PCB_EXPR_CONTEXT preflightContext( F_Cu );
 
-    m_compiler.Compile( aExpr.ToUTF8().data(), &ucode, &preflightContext );
+    if( !m_compiler.Compile( aExpr.ToUTF8().data(), &ucode, &preflightContext ) )
+        return false;
 
     PCB_EXPR_CONTEXT evaluationContext( F_Cu );
     LIBEVAL::VALUE*  result = ucode.Run( &evaluationContext );