dfilter: Allow arithmetic expression to commute

Allow an arithmetic expression like 1 + some.field. If we
cannot assign a type to the LHS commute the terms and
try again.

Before:

    Filter: _ws.ftypes.int32 + 1 == 10

    Syntax tree:
     0 TEST_ANY_EQ:
       1 OP_ADD:
         2 FIELD(_ws.ftypes.int32 <FT_INT32>)
         2 FVALUE(1 <FT_INT32>)
       1 FVALUE(10 <FT_INT32>)

    Instructions:
    00000 READ_TREE		_ws.ftypes.int32 <FT_INT32> -> reg#0
    00001 IF_FALSE_GOTO	4
    00002 ADD		reg#0 + 1 <FT_INT32> -> reg#1
    00003 ANY_EQ		reg#1 == 10 <FT_INT32>
    00004 RETURN

    Filter: 1 + _ws.ftypes.int32 == 10
    dftest: Constant arithmetic expression on the LHS is invalid.
    	1 + _ws.ftypes.int32 == 10
    	^

After:

    Filter: _ws.ftypes.int32 + 1 == 10

    Syntax tree:
     0 TEST_ANY_EQ:
       1 OP_ADD:
         2 FIELD(_ws.ftypes.int32 <FT_INT32>)
         2 FVALUE(1 <FT_INT32>)
       1 FVALUE(10 <FT_INT32>)

    Instructions:
    00000 READ_TREE		_ws.ftypes.int32 <FT_INT32> -> reg#0
    00001 IF_FALSE_GOTO	4
    00002 ADD		reg#0 + 1 <FT_INT32> -> reg#1
    00003 ANY_EQ		reg#1 == 10 <FT_INT32>
    00004 RETURN

    Filter: 1 + _ws.ftypes.int32 == 10

    Syntax tree:
     0 TEST_ANY_EQ:
       1 OP_ADD:
         2 FVALUE(1 <FT_INT32>)
         2 FIELD(_ws.ftypes.int32 <FT_INT32>)
       1 FVALUE(10 <FT_INT32>)

    Instructions:
    00000 READ_TREE		_ws.ftypes.int32 <FT_INT32> -> reg#0
    00001 IF_FALSE_GOTO	4
    00002 ADD		1 <FT_INT32> + reg#0 -> reg#1
    00003 ANY_EQ		reg#1 == 10 <FT_INT32>
    00004 RETURN

4 files changed, 184 insertions, 107 deletions

diff --git a/epan/dfilter/dfunctions.c b/epan/dfilter/dfunctions.c
index f77b327dcc..ba8de42705 100644
--- a/epan/dfilter/dfunctions.c
+++ b/epan/dfilter/dfunctions.c
@@ -342,7 +342,7 @@ ul_semcheck_compare(dfwork_t *dfw, const char *func_name, ftenum_t lhs_ftype,
     arg = param_list->data;
 
     if (stnode_type_id(arg) == STTYPE_ARITHMETIC) {
-        ftype = check_arithmetic_expr(dfw, arg, lhs_ftype);
+        ftype = check_arithmetic(dfw, arg, lhs_ftype);
     }
     else if (stnode_type_id(arg) == STTYPE_LITERAL && lhs_ftype != FT_NONE) {
         fv = dfilter_fvalue_from_literal(dfw, lhs_ftype, arg, FALSE, NULL);
@@ -365,7 +365,7 @@ ul_semcheck_compare(dfwork_t *dfw, const char *func_name, ftenum_t lhs_ftype,
         arg = l->data;
 
         if (stnode_type_id(arg) == STTYPE_ARITHMETIC) {
-            ft_arg = check_arithmetic_expr(dfw, arg, ftype);
+            ft_arg = check_arithmetic(dfw, arg, ftype);
         }
         else if (stnode_type_id(arg) == STTYPE_LITERAL && ftype != FT_NONE) {
             fv = dfilter_fvalue_from_literal(dfw, ftype, arg, FALSE, NULL);
@@ -410,7 +410,7 @@ ul_semcheck_absolute_value(dfwork_t *dfw, const char *func_name, ftenum_t lhs_ft
     st_node = param_list->data;
 
     if (stnode_type_id(st_node) == STTYPE_ARITHMETIC) {
-        ftype = check_arithmetic_expr(dfw, st_node, lhs_ftype);
+        ftype = check_arithmetic(dfw, st_node, lhs_ftype);
     }
     else if (stnode_type_id(st_node) == STTYPE_LITERAL) {
         if (lhs_ftype != FT_NONE) {
diff --git a/epan/dfilter/semcheck.c b/epan/dfilter/semcheck.c
index 96ea7e5a4f..1c8e4eeda4 100644
--- a/epan/dfilter/semcheck.c
+++ b/epan/dfilter/semcheck.c
@@ -37,6 +37,19 @@
 		dfilter_fail_throw(dfw, DF_ERROR_GENERIC, stnode_location(node), __VA_ARGS__); \
 	} while (0)
 
+typedef gboolean (*FtypeCanFunc)(enum ftenum);
+
+static ftenum_t
+check_arithmetic_LHS(dfwork_t *dfw, stnode_op_t st_op,
+			stnode_t *st_node, stnode_t *st_arg1, stnode_t *st_arg2,
+			ftenum_t lhs_ftype, int commute);
+
+static void
+check_relation(dfwork_t *dfw, stnode_op_t st_op,
+		FtypeCanFunc can_func, gboolean allow_partial_value,
+		stnode_t *st_node, stnode_t *st_arg1, stnode_t *st_arg2,
+		int commute);
+
 static void
 semcheck(dfwork_t *dfw, stnode_t *st_node);
 
@@ -44,8 +57,6 @@ static fvalue_t *
 mk_fvalue_from_val_string(dfwork_t *dfw, header_field_info *hfinfo, const char *s,
 				df_loc_t loc);
 
-typedef gboolean (*FtypeCanFunc)(enum ftenum);
-
 /* Compares to ftenum_t's and decides if they're
  * compatible or not (if they're the same basic type) */
 static gboolean
@@ -138,20 +149,6 @@ compatible_ftypes(ftenum_t a, ftenum_t b)
 	return FALSE;
 }
 
-static gboolean
-node_is_constant(stnode_t *node)
-{
-	switch (stnode_type_id(node)) {
-		case STTYPE_CHARCONST:
-		case STTYPE_STRING:
-		case STTYPE_LITERAL:
-			return TRUE;
-		default:
-			break;
-	}
-	return FALSE;
-}
-
 /* Don't set the error message if it's already set. */
 #define SET_ERROR(dfw, str) \
 	do {						\
@@ -687,7 +684,7 @@ check_relation_LHS_FIELD(dfwork_t *dfw, stnode_op_t st_op,
 		ws_assert(st_op == STNODE_OP_MATCHES);
 	}
 	else if (type2 == STTYPE_ARITHMETIC) {
-		ftype2 = check_arithmetic_expr(dfw, st_arg2, ftype1);
+		ftype2 = check_arithmetic(dfw, st_arg2, ftype1);
 
 		if (!compatible_ftypes(ftype1, ftype2)) {
 			FAIL(dfw, st_arg2, "%s and %s are not of compatible types.",
@@ -768,7 +765,7 @@ check_relation_LHS_SLICE(dfwork_t *dfw, stnode_op_t st_op,
 		ws_assert(st_op == STNODE_OP_MATCHES);
 	}
 	else if (type2 == STTYPE_ARITHMETIC) {
-		ftype2 = check_arithmetic_expr(dfw, st_arg2, FT_BYTES);
+		ftype2 = check_arithmetic(dfw, st_arg2, FT_BYTES);
 
 		if (!compatible_ftypes(FT_BYTES, ftype2)) {
 			FAIL(dfw, st_arg2, "%s and %s are not of compatible types.",
@@ -867,7 +864,7 @@ check_relation_LHS_FUNCTION(dfwork_t *dfw, stnode_op_t st_op,
 		ws_assert(st_op == STNODE_OP_MATCHES);
 	}
 	else if (type2 == STTYPE_ARITHMETIC) {
-		ftype2 = check_arithmetic_expr(dfw, st_arg2, ftype1);
+		ftype2 = check_arithmetic(dfw, st_arg2, ftype1);
 
 		if (!compatible_ftypes(ftype1, ftype2)) {
 			FAIL(dfw, st_arg2, "%s and %s are not of compatible types.",
@@ -885,31 +882,97 @@ check_relation_LHS_FUNCTION(dfwork_t *dfw, stnode_op_t st_op,
 }
 
 static void
-check_relation_LHS_ARITHMETIC(dfwork_t *dfw, stnode_op_t st_op _U_,
-		FtypeCanFunc can_func _U_, gboolean allow_partial_value,
-		stnode_t *st_node, stnode_t *st_arg1, stnode_t *st_arg2)
+check_relation_LHS_ARITHMETIC(dfwork_t *dfw, stnode_op_t st_op,
+		FtypeCanFunc can_func, gboolean allow_partial_value,
+		stnode_t *st_node, stnode_t *st_arg1, stnode_t *st_arg2,
+		int commute)
 {
-	stnode_t		*entity;
-	sttype_id_t		entity_type;
+	sttype_id_t		type2;
+	ftenum_t		ftype1, ftype2;
+	fvalue_t		*fvalue;
 
 	LOG_NODE(st_node);
 
-	check_arithmetic_expr(dfw, st_arg1, FT_NONE);
+	ftype1 = check_arithmetic(dfw, st_arg1, FT_NONE);
+	if (ftype1 == FT_NONE) {
+		check_relation(dfw, st_op, can_func, allow_partial_value,
+				st_node, st_arg2, st_arg1, commute - 1);
+		return;
+	}
+
+	if (!can_func(ftype1)) {
+		FAIL(dfw, st_arg1, "Result with type %s cannot participate in %s comparison.",
+				ftype_pretty_name(ftype1),
+				stnode_todisplay(st_node));
+	}
+
+	type2 = stnode_type_id(st_arg2);
 
-	sttype_oper_get(st_arg1, NULL, &entity, NULL);
-	entity_type = stnode_type_id(entity);
+	if (IS_FIELD_ENTITY(type2)) {
+		ftype2 = sttype_field_ftenum(st_arg2);
 
-	if (IS_FIELD_ENTITY(entity_type)) {
-		check_relation_LHS_FIELD(dfw, st_op, can_func, allow_partial_value, st_node, entity, st_arg2);
+		if (!compatible_ftypes(ftype1, ftype2)) {
+			FAIL(dfw, st_arg2, "%s and %s are not of compatible types.",
+					stnode_todisplay(st_arg1), stnode_todisplay(st_arg2));
+		}
+		if (!can_func(ftype2)) {
+			FAIL(dfw, st_arg2, "%s (type=%s) cannot participate in specified comparison.",
+					stnode_todisplay(st_arg2), ftype_pretty_name(ftype2));
+		}
+	}
+	else if (type2 == STTYPE_STRING) {
+		fvalue = dfilter_fvalue_from_string(dfw, ftype1, st_arg2, NULL);
+		stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 	}
-	else if (entity_type == STTYPE_FUNCTION) {
-		check_relation_LHS_FUNCTION(dfw, st_op, can_func, allow_partial_value, st_node, entity, st_arg2);
+	else if (type2 == STTYPE_LITERAL) {
+		fvalue = dfilter_fvalue_from_literal(dfw, ftype1, st_arg2, allow_partial_value, NULL);
+		stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
+	}
+	else if (type2 == STTYPE_CHARCONST) {
+		fvalue = dfilter_fvalue_from_charconst(dfw, ftype1, st_arg2);
+		stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
+	}
+	else if (type2 == STTYPE_SLICE) {
+		check_slice_sanity(dfw, st_arg2, ftype1);
+		if (!is_bytes_type(ftype1)) {
+			if (!ftype_can_slice(ftype1)) {
+				FAIL(dfw, st_arg1, "Result is a %s and cannot be converted into a sequence of bytes.",
+						ftype_pretty_name(ftype1));
+			}
+
+			/* Convert expression result to bytes */
+			convert_to_bytes(st_arg1);
+		}
 	}
-	else if (entity_type == STTYPE_SLICE) {
-		check_relation_LHS_SLICE(dfw, st_op, can_func, allow_partial_value, st_node, entity, st_arg2);
+	else if (type2 == STTYPE_FUNCTION) {
+		ftype2 = check_function(dfw, st_arg2, ftype1);
+
+		if (!compatible_ftypes(ftype1, ftype2)) {
+			FAIL(dfw, st_arg2, "Result (type=%s) and return value of %s() (type=%s) are not of compatible types.",
+					ftype_pretty_name(ftype1),
+					sttype_function_name(st_arg2), ftype_pretty_name(ftype2));
+		}
+
+		if (!can_func(ftype2)) {
+			FAIL(dfw, st_arg2, "return value of %s() (type=%s) cannot participate in specified comparison.",
+					sttype_function_name(st_arg2), ftype_pretty_name(ftype2));
+		}
 	}
-	else if (entity_type == STTYPE_ARITHMETIC) {
-		check_relation_LHS_ARITHMETIC(dfw, st_op, can_func, allow_partial_value, st_node, entity, st_arg2);
+	else if (type2 == STTYPE_PCRE) {
+		ws_assert(st_op == STNODE_OP_MATCHES);
+	}
+	else if (type2 == STTYPE_ARITHMETIC) {
+		ftype2 = check_arithmetic(dfw, st_arg2, ftype1);
+
+		if (!compatible_ftypes(ftype1, ftype2)) {
+			FAIL(dfw, st_arg2, "%s and %s are not of compatible types.",
+					stnode_todisplay(st_arg1), stnode_todisplay(st_arg2));
+		}
+
+		if (!can_func(ftype2)) {
+			FAIL(dfw, st_arg2, "%s (type=%s) cannot participate in specified comparison.",
+					stnode_todisplay(st_arg2), ftype_pretty_name(ftype2));
+		}
 	}
 	else {
 		ws_assert_not_reached();
@@ -947,7 +1010,7 @@ check_relation(dfwork_t *dfw, stnode_op_t st_op,
 			break;
 		case STTYPE_ARITHMETIC:
 			check_relation_LHS_ARITHMETIC(dfw, st_op, can_func,
-					allow_partial_value, st_node, st_arg1, st_arg2);
+					allow_partial_value, st_node, st_arg1, st_arg2, commute);
 			break;
 		case STTYPE_LITERAL:
 		case STTYPE_STRING:
@@ -956,7 +1019,7 @@ check_relation(dfwork_t *dfw, stnode_op_t st_op,
 			   don't have a type to assign them. Commute the LHS with the RHS and retry
 			   the relation semantic check. */
 			check_relation(dfw, st_op, can_func,
-					allow_partial_value,st_node, st_arg2, st_arg1, --commute);
+					allow_partial_value,st_node, st_arg2, st_arg1, commute - 1);
 			break;
 		default:
 			/* Should not happen. */
@@ -1160,72 +1223,23 @@ op_to_error_msg(stnode_op_t st_op)
 }
 
 static ftenum_t
-check_arithmetic_entity(dfwork_t *dfw, stnode_t *st_arg, ftenum_t lhs_ftype)
-{
-	sttype_id_t		type;
-	ftenum_t		ftype;
-
-	LOG_NODE(st_arg);
-
-	/* lhs_ftype variable determines the type for this entity. If LHS type
-	 * is none we must have been passed an entity with a definite type
-	 * (field, function, etc). */
-
-	type = stnode_type_id(st_arg);
-
-	if (type == STTYPE_LITERAL) {
-		/* numeric constant */
-		ws_assert(lhs_ftype != FT_NONE);
-		fvalue_t *fvalue = dfilter_fvalue_from_literal(dfw, lhs_ftype, st_arg, FALSE, NULL);
-		stnode_replace(st_arg, STTYPE_FVALUE, fvalue);
-		ftype = fvalue_type_ftenum(fvalue);
-	}
-	else if (type == STTYPE_FIELD || type == STTYPE_REFERENCE) {
-		header_field_info *hfinfo = sttype_field_hfinfo(st_arg);
-		ftype = hfinfo->type;
-	}
-	else if (type == STTYPE_FUNCTION) {
-		ftype = check_function(dfw, st_arg, lhs_ftype);
-	}
-	else if (type == STTYPE_SLICE) {
-		check_slice_sanity(dfw, st_arg, lhs_ftype);
-
-		ftype = FT_BYTES;
-	}
-	else if (type == STTYPE_FVALUE) {
-		ftype = fvalue_type_ftenum(stnode_data(st_arg));
-	}
-	else {
-		FAIL(dfw, st_arg, "%s is not a valid arithmetic operand.",
-				stnode_todisplay(st_arg));
-	}
-
-	return ftype;
-}
-
-ftenum_t
-check_arithmetic_expr(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype)
+check_arithmetic_LHS(dfwork_t *dfw, stnode_op_t st_op,
+			stnode_t *st_node, stnode_t *st_arg1, stnode_t *st_arg2,
+			ftenum_t lhs_ftype, int commute)
 {
-	stnode_op_t		st_op;
-	stnode_t		*st_arg1, *st_arg2;
 	ftenum_t		ftype1, ftype2;
 	FtypeCanFunc 		can_func = NULL;
 
 	LOG_NODE(st_node);
 
-	if (stnode_type_id(st_node) != STTYPE_ARITHMETIC) {
-		return check_arithmetic_entity(dfw, st_node, lhs_ftype);
-	}
-
-	sttype_oper_get(st_node, &st_op, &st_arg1, &st_arg2);
-
-	/* On the LHS we require a field-like value as the first term. */
-	if (lhs_ftype == FT_NONE && node_is_constant(st_arg1)) {
-		FAIL(dfw, st_arg1, "Constant arithmetic expression on the LHS is invalid.");
+	if (commute < 0) {
+		return FT_NONE;
 	}
 
 	if (st_op == STNODE_OP_UNARY_MINUS) {
-		ftype1 = check_arithmetic_expr(dfw, st_arg1, lhs_ftype);
+		ftype1 = check_arithmetic(dfw, st_arg1, lhs_ftype);
+		if (ftype1 == FT_NONE)
+			return FT_NONE;
 		if (!ftype_can_unary_minus(ftype1)) {
 			FAIL(dfw, st_arg1, "%s %s.",
 				ftype_name(ftype1), op_to_error_msg(st_op));
@@ -1269,13 +1283,16 @@ check_arithmetic_expr(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype)
 			ws_assert_not_reached();
 	}
 
-	ftype1 = check_arithmetic_expr(dfw, st_arg1, lhs_ftype);
+	ftype1 = check_arithmetic(dfw, st_arg1, lhs_ftype);
+	if (ftype1 == FT_NONE) {
+		return check_arithmetic_LHS(dfw, st_op, st_node, st_arg2, st_arg1, lhs_ftype, commute - 1);
+	}
 	if (!can_func(ftype1)) {
 		FAIL(dfw, st_arg1, "%s %s.",
 			ftype_name(ftype1), op_to_error_msg(st_op));
 	}
 
-	ftype2 = check_arithmetic_expr(dfw, st_arg2, ftype1);
+	ftype2 = check_arithmetic(dfw, st_arg2, ftype1);
 	if (!can_func(ftype2)) {
 		FAIL(dfw, st_arg2, "%s %s.",
 			ftype_name(ftype2), op_to_error_msg(st_op));
@@ -1289,6 +1306,63 @@ check_arithmetic_expr(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype)
 	return ftype1;
 }
 
+ftenum_t
+check_arithmetic(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype)
+{
+	sttype_id_t		type;
+	stnode_op_t		st_op;
+	stnode_t		*st_arg1, *st_arg2;
+	ftenum_t		ftype;
+
+	LOG_NODE(st_node);
+
+	type = stnode_type_id(st_node);
+
+	switch (type) {
+		case STTYPE_LITERAL:
+			if (lhs_ftype != FT_NONE) {
+				fvalue_t *fvalue = dfilter_fvalue_from_literal(dfw, lhs_ftype, st_node, FALSE, NULL);
+				stnode_replace(st_node, STTYPE_FVALUE, fvalue);
+				ftype = fvalue_type_ftenum(fvalue);
+			}
+			else {
+				ftype = FT_NONE;
+			}
+			break;
+
+		case STTYPE_FIELD:
+		case STTYPE_REFERENCE:
+		{
+			header_field_info *hfinfo = sttype_field_hfinfo(st_node);
+			ftype = hfinfo->type;
+			break;
+		}
+		case STTYPE_FUNCTION:
+			ftype = check_function(dfw, st_node, lhs_ftype);
+			break;
+
+		case STTYPE_SLICE:
+			check_slice_sanity(dfw, st_node, lhs_ftype);
+			ftype = FT_BYTES;
+			break;
+
+		case STTYPE_FVALUE:
+			ftype = fvalue_type_ftenum(stnode_data(st_node));
+			break;
+
+		case STTYPE_ARITHMETIC:
+			sttype_oper_get(st_node, &st_op, &st_arg1, &st_arg2);
+			ftype = check_arithmetic_LHS(dfw, st_op, st_node, st_arg1, st_arg2, lhs_ftype, 1);
+			break;
+
+		default:
+			FAIL(dfw, st_node, "%s is not a valid arithmetic operation.",
+				stnode_todisplay(st_node));
+	}
+
+	return ftype;
+}
+
 
 /* Check the entire syntax tree. */
 static void
@@ -1301,7 +1375,7 @@ semcheck(dfwork_t *dfw, stnode_t *st_node)
 			check_test(dfw, st_node);
 			break;
 		case STTYPE_ARITHMETIC:
-			check_arithmetic_expr(dfw, st_node, FT_NONE);
+			check_arithmetic(dfw, st_node, FT_NONE);
 			break;
 		case STTYPE_SLICE:
 			check_slice_sanity(dfw, st_node, FT_NONE);
diff --git a/epan/dfilter/semcheck.h b/epan/dfilter/semcheck.h
index 259186c14e..53567dc743 100644
--- a/epan/dfilter/semcheck.h
+++ b/epan/dfilter/semcheck.h
@@ -17,7 +17,7 @@ gboolean
 dfw_semcheck(dfwork_t *dfw);
 
 ftenum_t
-check_arithmetic_expr(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype);
+check_arithmetic(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype);
 
 ftenum_t
 check_function(dfwork_t *dfw, stnode_t *st_node, ftenum_t lhs_ftype);
diff --git a/test/suite_dfilter/group_syntax.py b/test/suite_dfilter/group_syntax.py
index c7746ec181..e669e25415 100644
--- a/test/suite_dfilter/group_syntax.py
+++ b/test/suite_dfilter/group_syntax.py
@@ -231,10 +231,9 @@ class case_unary_minus(unittest.TestCase):
         dfilter = "tcp.window_size_scalefactor == +tcp.dstport"
         checkDFilterCount(dfilter, 0)
 
-    def test_unary_3(self, checkDFilterFail):
-        error = 'Constant arithmetic expression on the LHS is invalid'
+    def test_unary_3(self, checkDFilterCount):
         dfilter = "-2 == tcp.dstport"
-        checkDFilterFail(dfilter, error)
+        checkDFilterCount(dfilter, 0)
 
     def test_unary_4(self, checkDFilterCount):
         dfilter = "tcp.window_size_scalefactor == -{tcp.dstport * 20}"
@@ -256,9 +255,13 @@ class case_arithmetic(unittest.TestCase):
         dfilter = "udp.dstport == 66+1"
         checkDFilterCount(dfilter, 2)
 
-    def test_add_3(self, checkDFilterFail):
-        error = 'Constant arithmetic expression on the LHS is invalid'
-        dfilter = "2 + 3 == frame.number"
+    def test_add_4(self, checkDFilterCount):
+        dfilter = "1 + 2 == frame.number"
+        checkDFilterCount(dfilter, 1)
+
+    def test_add_5(self, checkDFilterFail):
+        error = 'Constant expression is invalid'
+        dfilter = "1 + 2 == 2 + 1"
         checkDFilterFail(dfilter, error)
 
     def test_sub_1(self, checkDFilterCount):