Tweak lineno compilation

Fixes GH-18985
Closes GH-22833

zend_compile_expr() and zend_compile_var() now restore CG(zend_lineno) after
compilation of the node has finished. This makes the compiled lineno more
predictable.

Note that there are many cases left that are incorrect. For example:

    <?php

    return
        foo();

    L0004 0000 INIT_FCALL_BY_NAME 0 string("foo")
    L0004 0001 T0 = DO_FCALL_BY_NAME
    L0004 0002 RETURN T0
    L0007 0003 RETURN int(1)

The zend_ast_create_n() functions set lineno to the lineno of the first non-null
child. In this case that's foo(), even though it's not on the same line as
return. What we'd need instead is backing up CG(zend_lineno) after the return
keyword in the parser so we can restore it later. However, this would be a
rather invasive change, given the same issue exists for many ast kinds, so a
better solution might just be to re-commit
e528762.

I don't feel comfortable backporting this as a bug fix, but we might address the
issue in a simpler, less comprehensive manner for older branches.

Author: iluuu1994

NEWS

@@ -6,6 +6,8 @@ PHP                                                                        NEWS
   . Implemented partial function application RFC. (Arnaud)
   . Fixed bug GH-22263 (reset typed property default on every unserialize
     failure path). (David Carlier)
+  . Fixed bug GH-18985 (Wrong line numbers for match with constant arms).
+    (ilutov)
 
 - DOM:
   . Fixed bug GH-22825 (DOMElement::setAttribute() fails silently when the DTD

Zend/Optimizer/zend_optimizer.c

@@ -1052,7 +1052,10 @@ static void zend_optimize(zend_op_array      *op_array,
 	}
 
 	if (ctx->debug_level & ZEND_DUMP_BEFORE_OPTIMIZER) {
-		zend_dump_op_array(op_array, ZEND_DUMP_LIVE_RANGES, "before optimizer", NULL);
+		uint32_t additional_dump_flags = (ctx->debug_level & ZEND_DUMP_LINE_NUMBERS_PASSTHRU)
+			? ZEND_DUMP_LINE_NUMBERS
+			: 0;
+		zend_dump_op_array(op_array, ZEND_DUMP_LIVE_RANGES|additional_dump_flags, "before optimizer", NULL);
 	}
 
 	/* pass 1 (Simple local optimizations)

Zend/Optimizer/zend_optimizer.h

@@ -80,6 +80,7 @@
 #define ZEND_DUMP_DFA_SSA           (1<<27)
 #define ZEND_DUMP_DFA_SSA_VARS      (1<<28)
 #define ZEND_DUMP_SCCP              (1<<29)
+#define ZEND_DUMP_LINE_NUMBERS_PASSTHRU (1<<30)
 
 typedef struct _zend_script {
 	zend_string   *filename;

Zend/zend_compile.c

@@ -7125,6 +7125,8 @@ static void zend_compile_match(znode *result, zend_ast *ast)
 		zend_ast *arm_ast = arms->child[i];
 		zend_ast *body_ast = arm_ast->child[1];
 
+		CG(zend_lineno) = zend_ast_get_lineno(arm_ast);
+
 		if (arm_ast->child[0] != NULL) {
 			zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);
 
@@ -10728,6 +10730,8 @@ static void zend_compile_binary_op(znode *result, zend_ast *ast) /* {{{ */
 	zend_compile_expr(&left_node, left_ast);
 	zend_compile_expr(&right_node, right_ast);
 
+	CG(zend_lineno) = ast->lineno;
+
 	if (left_node.op_type == IS_CONST && right_node.op_type == IS_CONST) {
 		if (zend_try_ct_eval_binary_op(&result->u.constant, opcode,
 				&left_node.u.constant, &right_node.u.constant)
@@ -12335,9 +12339,6 @@ static void zend_compile_stmt(zend_ast *ast) /* {{{ */
 
 static void zend_compile_expr_inner(znode *result, zend_ast *ast) /* {{{ */
 {
-	/* CG(zend_lineno) = ast->lineno; */
-	CG(zend_lineno) = zend_ast_get_lineno(ast);
-
 	if (CG(memoize_mode) != ZEND_MEMOIZE_NONE) {
 		zend_compile_memoized_expr(result, ast, BP_VAR_R);
 		return;
@@ -12479,6 +12480,9 @@ static void zend_compile_expr(znode *result, zend_ast *ast)
 {
 	zend_check_stack_limit();
 
+	uint32_t prev_lineno = CG(zend_lineno);
+	CG(zend_lineno) = zend_ast_get_lineno(ast);
+
 	uint32_t checkpoint = zend_short_circuiting_checkpoint();
 	zend_compile_expr_inner(result, ast);
 	zend_short_circuiting_commit(checkpoint, result, ast);
@@ -12488,12 +12492,12 @@ static void zend_compile_expr(znode *result, zend_ast *ast)
 		ZEND_ASSERT(result->op_type != IS_VAR);
 	}
 #endif
+
+	CG(zend_lineno) = prev_lineno;
 }
 
 static zend_op *zend_compile_var_inner(znode *result, zend_ast *ast, uint32_t type, bool by_ref)
 {
-	CG(zend_lineno) = zend_ast_get_lineno(ast);
-
 	if (CG(memoize_mode) != ZEND_MEMOIZE_NONE) {
 		switch (ast->kind) {
 			case ZEND_AST_CALL:
@@ -12554,6 +12558,9 @@ static zend_op *zend_compile_var(znode *result, zend_ast *ast, uint32_t type, bo
 {
 	zend_check_stack_limit();
 
+	uint32_t prev_lineno = CG(zend_lineno);
+	CG(zend_lineno) = zend_ast_get_lineno(ast);
+
 	uint32_t checkpoint = zend_short_circuiting_checkpoint();
 	zend_op *opcode = zend_compile_var_inner(result, ast, type, by_ref);
 	zend_short_circuiting_commit(checkpoint, result, ast);
@@ -12567,32 +12574,47 @@ static zend_op *zend_compile_var(znode *result, zend_ast *ast, uint32_t type, bo
 		ZEND_ASSERT(result->op_type != IS_VAR);
 	}
 #endif
+
+	CG(zend_lineno) = prev_lineno;
+
 	return opcode;
 }
 
 static zend_op *zend_delayed_compile_var(znode *result, zend_ast *ast, uint32_t type, bool by_ref) /* {{{ */
 {
 	zend_check_stack_limit();
 
+	uint32_t prev_lineno = CG(zend_lineno);
+	CG(zend_lineno) = zend_ast_get_lineno(ast);
+
+	zend_op *opline;
 	switch (ast->kind) {
 		case ZEND_AST_VAR:
-			return zend_compile_simple_var(result, ast, type, true);
+			opline = zend_compile_simple_var(result, ast, type, true);
+			break;
 		case ZEND_AST_DIM:
-			return zend_delayed_compile_dim(result, ast, type, by_ref);
+			opline = zend_delayed_compile_dim(result, ast, type, by_ref);
+			break;
 		case ZEND_AST_PROP:
 		case ZEND_AST_NULLSAFE_PROP:
 		{
-			zend_op *opline = zend_delayed_compile_prop(result, ast, type);
+			opline = zend_delayed_compile_prop(result, ast, type);
 			if (by_ref) {
 				opline->extended_value |= ZEND_FETCH_REF;
 			}
-			return opline;
+			break;
 		}
 		case ZEND_AST_STATIC_PROP:
-			return zend_compile_static_prop(result, ast, type, by_ref, true);
+			opline = zend_compile_static_prop(result, ast, type, by_ref, true);
+			break;
 		default:
-			return zend_compile_var(result, ast, type, false);
+			opline = zend_compile_var(result, ast, type, false);
+			break;
 	}
+
+	CG(zend_lineno) = prev_lineno;
+
+	return opline;
 }
 /* }}} */
 

ext/opcache/tests/gh18985.phpt

@@ -0,0 +1,35 @@
+--TEST--
+GH-18985: Wrong lineno for multiline expressions
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable_cli=1
+opcache.opt_debug_level=0x40010000
+--FILE--
+<?php
+
+echo match(15) {
+    13 => "A",
+    15 => "B",
+    default => "C",
+};
+
+?>
+--EXPECTF--
+$_main:
+     ; (lines=9, args=0, vars=0, tmps=%s)
+     ; (before optimizer)
+     ; %sgh18985.php:1-10
+     ; return  [] RANGE[0..0]
+L0003 0000 MATCH int(15) 13: 0001, 15: 0003, default: 0005
+L0004 0001 T1 = QM_ASSIGN string("A")
+L0004 0002 JMP 0007
+L0005 0003 T1 = QM_ASSIGN string("B")
+L0005 0004 JMP 0007
+L0006 0005 T1 = QM_ASSIGN string("C")
+L0006 0006 JMP 0007
+L0003 0007 ECHO T1
+L0010 0008 RETURN int(1)
+LIVE RANGES:
+     1: 0006 - 0007 (tmp/var)
+B

ext/opcache/tests/jit/shift_right_004.phpt

@@ -30,9 +30,9 @@ Warning: Undefined array key 0 in %sshift_right_004.php on line 7
 
 Deprecated: Implicit conversion from float %f to int loses precision in %sshift_right_004.php on line 8
 
-Warning: A non-numeric value encountered in %sshift_right_004.php on line 7
+Warning: A non-numeric value encountered in %sshift_right_004.php on line 6
 
-Warning: A non-numeric value encountered in %sshift_right_004.php on line 7
+Warning: A non-numeric value encountered in %sshift_right_004.php on line 6
 
 Fatal error: Uncaught ArithmeticError: Bit shift by negative number in %sshift_right_004.php:8
 Stack trace:

ext/opcache/tests/jit/switch_001.phpt

@@ -16,7 +16,7 @@ foo();
 ?>
 DONE
 --EXPECTF--
-Warning: Undefined variable $y in %sswitch_001.php on line 4
+Warning: Undefined variable $y in %sswitch_001.php on line 3
 
-Warning: Undefined variable $y in %sswitch_001.php on line 5
+Warning: Undefined variable $y in %sswitch_001.php on line 3
 DONE