Make fail_with_error() macro abort the parser on non-recoverable errors. (#1688)

Until now this macro was aborting the parsing with YYERROR, independently of the kind of error. YYERROR aborts the parsing in a recoverable way, allowing the parsing to continue with the next rule. However, some errors like ERROR_INSUFFICIENT_MEMORY may leave the compiler in an inconsistent state and we should not continue with the parsing.

.github/workflows/build.yml

@@ -26,6 +26,10 @@ jobs:
             protobuf-compiler \
             protobuf-c-compiler \
             libprotobuf-c-dev
+      - name: Print versions
+        run: |
+          bison --version
+          gcc --version
       - name: Prepare for building
         run: |
           ./bootstrap.sh

libyara/grammar.h

@@ -189,7 +189,7 @@ extern int yara_yydebug;
 #if ! defined YYSTYPE && ! defined YYSTYPE_IS_DECLARED
 union YYSTYPE
 {
-#line 308 "grammar.y"
+#line 323 "grammar.y"
 
   YR_EXPRESSION   expression;
   SIZED_STRING*   sized_string;

libyara/grammar.y

@@ -64,21 +64,36 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define FOR_EXPRESSION_ANY  2
 #define FOR_EXPRESSION_NONE 3
 
+
+// fail_with_error() is used in parser actions for aborting the parsing with
+// an error. If the error is recoverable (like syntax errors), the parser will
+// report the error and continue parsing the next rule. If the error is a
+// fatal, non-recoverable error, the parser will be completely aborted.
 #define fail_with_error(e) \
     { \
       compiler->last_error = e; \
       yyerror(yyscanner, compiler, NULL); \
-      YYERROR; \
+      switch (e) \
+      { \
+      case ERROR_INSUFFICIENT_MEMORY: \
+        YYABORT; \
+      default: \
+        YYERROR; \
+      } \
     }
 
-
+// fail_if_error() is used in parser actions for aborting the parsing if an
+// error has occurred. See fail_with_error for details.
 #define fail_if_error(e) \
     if (e != ERROR_SUCCESS) \
     { \
       fail_with_error(e); \
-    } \
+    }
 
 
+// check_type(expression, EXPRESSION_TYPE_INTEGER | EXPRESSION_TYPE_FLOAT) is
+// used to ensure that the type of "expression" is either integer or float,
+// the cleanup statements are executed if the condition is not met.
 #define check_type_with_cleanup(expression, expected_type, op, cleanup) \
     if (((expression.type) & (expected_type)) == 0) \
     { \
@@ -1806,9 +1821,8 @@ expression
         {
           yr_compiler_set_error_extra_info(
               compiler, "percentage must be between 1 and 100 (inclusive)");
-          compiler->last_error = ERROR_INVALID_PERCENTAGE;
-          yyerror(yyscanner, compiler, NULL);
-          YYERROR;
+
+          fail_with_error(ERROR_INVALID_PERCENTAGE);
         }
 
         yr_parser_emit_with_arg(yyscanner, OP_OF_PERCENT, OF_STRING_SET, NULL, NULL);
@@ -1826,9 +1840,8 @@ expression
         {
           yr_compiler_set_error_extra_info(
               compiler, "percentage must be between 1 and 100 (inclusive)");
-          compiler->last_error = ERROR_INVALID_PERCENTAGE;
-          yyerror(yyscanner, compiler, NULL);
-          YYERROR;
+
+          fail_with_error(ERROR_INVALID_PERCENTAGE);
         }
 
         yr_parser_emit_with_arg(yyscanner, OP_OF_PERCENT, OF_RULE_SET, NULL, NULL);

libyara/lexer.c

@@ -3517,12 +3517,6 @@ void yyerror(
   char message[512] = {'\0'};
   char* file_name = NULL;
 
-  /*
-    if error_message != NULL the error comes from yyparse internal code
-    else the error comes from my code and the error code is set in
-    compiler->last_error
-  */
-
   compiler->errors++;
 
   if (compiler->current_line != 0)
@@ -3549,6 +3543,10 @@ void yyerror(
         YR_RULES_TABLE,
         compiler->current_rule_idx * sizeof(YR_RULE));
 
+  // if error_message != NULL the error comes from yyparse internal code
+  // else the error comes from my code and the error code is set in
+  // compiler->last_error
+
   if (error_message != NULL)
   {
     yr_compiler_set_error_extra_info(compiler, error_message);

libyara/lexer.l

@@ -825,12 +825,6 @@ void yyerror(
   char message[512] = {'\0'};
   char* file_name = NULL;
 
-  /*
-    if error_message != NULL the error comes from yyparse internal code
-    else the error comes from my code and the error code is set in
-    compiler->last_error
-  */
-
   compiler->errors++;
 
   if (compiler->current_line != 0)
@@ -857,6 +851,10 @@ void yyerror(
         YR_RULES_TABLE,
         compiler->current_rule_idx * sizeof(YR_RULE));
 
+  // if error_message != NULL the error comes from yyparse internal code
+  // else the error comes from my code and the error code is set in
+  // compiler->last_error
+
   if (error_message != NULL)
   {
     yr_compiler_set_error_extra_info(compiler, error_message);