bpo-35193: Fix an off by one error in the RETURN_VALUE case. (GH-10418)

Fix an off by one error in the peephole optimizer when checking for unreachable code beyond a return.

Do a bounds check within find_op so it can return before going past the end as a safety measure.

7db3c48833 (diff-a33329ae6ae0bb295d742f0caf93c137)
introduced this off by one error while fixing another one nearby.

This bug was shipped in all Python 3.6 and 3.7 releases.

The included unittest won't fail unless you do a clang msan build.

Lib/test/test_compile.py

@@ -1,3 +1,4 @@
+import dis
 import math
 import os
 import unittest
@@ -621,6 +622,24 @@ if 1:
         self.check_constant(f1, frozenset({0}))
         self.assertTrue(f1(0))
 
+    # This is a regression test for a CPython specific peephole optimizer
+    # implementation bug present in a few releases.  It's assertion verifies
+    # that peephole optimization was actually done though that isn't an
+    # indication of the bugs presence or not (crashing is).
+    @support.cpython_only
+    def test_peephole_opt_unreachable_code_array_access_in_bounds(self):
+        """Regression test for issue35193 when run under clang msan."""
+        def unused_code_at_end():
+            return 3
+            raise RuntimeError("unreachable")
+        # The above function definition will trigger the out of bounds
+        # bug in the peephole optimizer as it scans opcodes past the
+        # RETURN_VALUE opcode.  This does not always crash an interpreter.
+        # When you build with the clang memory sanitizer it reliably aborts.
+        self.assertEqual(
+            'RETURN_VALUE',
+            list(dis.get_instructions(unused_code_at_end))[-1].opname)
+
     def test_dont_merge_constants(self):
         # Issue #25843: compile() must not merge constants which are equal
         # but have a different type.

Misc/NEWS.d/next/Core and Builtins/2018-11-08-15-00-58.bpo-35193.HzPS6R.rst

@@ -0,0 +1,3 @@
+Fix an off by one error in the bytecode peephole optimizer where it could read
+bytes beyond the end of bounds of an array when removing unreachable code.
+This bug was present in every release of Python 3.6 and 3.7 until now.

Python/peephole.c

@@ -50,9 +50,9 @@ lastn_const_start(const _Py_CODEUNIT *codestr, Py_ssize_t i, Py_ssize_t n)
 
 /* Scans through EXTENDED ARGs, seeking the index of the effective opcode */
 static Py_ssize_t
-find_op(const _Py_CODEUNIT *codestr, Py_ssize_t i)
+find_op(const _Py_CODEUNIT *codestr, Py_ssize_t codelen, Py_ssize_t i)
 {
-    while (_Py_OPCODE(codestr[i]) == EXTENDED_ARG) {
+    while (i < codelen && _Py_OPCODE(codestr[i]) == EXTENDED_ARG) {
         i++;
     }
     return i;
@@ -128,8 +128,9 @@ copy_op_arg(_Py_CODEUNIT *codestr, Py_ssize_t i, unsigned char op,
    Called with codestr pointing to the first LOAD_CONST.
 */
 static Py_ssize_t
-fold_tuple_on_constants(_Py_CODEUNIT *codestr, Py_ssize_t c_start,
-                        Py_ssize_t opcode_end, PyObject *consts, int n)
+fold_tuple_on_constants(_Py_CODEUNIT *codestr, Py_ssize_t codelen,
+                        Py_ssize_t c_start, Py_ssize_t opcode_end,
+                        PyObject *consts, int n)
 {
     /* Pre-conditions */
     assert(PyList_CheckExact(consts));
@@ -142,7 +143,7 @@ fold_tuple_on_constants(_Py_CODEUNIT *codestr, Py_ssize_t c_start,
 
     for (Py_ssize_t i = 0, pos = c_start; i < n; i++, pos++) {
         assert(pos < opcode_end);
-        pos = find_op(codestr, pos);
+        pos = find_op(codestr, codelen, pos);
         assert(_Py_OPCODE(codestr[pos]) == LOAD_CONST);
 
         unsigned int arg = get_arg(codestr, pos);
@@ -265,7 +266,7 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
         goto exitError;
     assert(PyList_Check(consts));
 
-    for (i=find_op(codestr, 0) ; i<codelen ; i=nexti) {
+    for (i=find_op(codestr, codelen, 0) ; i<codelen ; i=nexti) {
         opcode = _Py_OPCODE(codestr[i]);
         op_start = i;
         while (op_start >= 1 && _Py_OPCODE(codestr[op_start-1]) == EXTENDED_ARG) {
@@ -303,7 +304,8 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
                 if (j > 0 && lastlc >= j) {
                     h = lastn_const_start(codestr, op_start, j);
                     if (ISBASICBLOCK(blocks, h, op_start)) {
-                        h = fold_tuple_on_constants(codestr, h, i+1, consts, j);
+                        h = fold_tuple_on_constants(codestr, codelen,
+                                                    h, i+1, consts, j);
                         break;
                     }
                 }
@@ -340,7 +342,7 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
             case JUMP_IF_FALSE_OR_POP:
             case JUMP_IF_TRUE_OR_POP:
                 h = get_arg(codestr, i) / sizeof(_Py_CODEUNIT);
-                tgt = find_op(codestr, h);
+                tgt = find_op(codestr, codelen, h);
 
                 j = _Py_OPCODE(codestr[tgt]);
                 if (CONDITIONAL_JUMP(j)) {
@@ -374,7 +376,7 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
             case JUMP_FORWARD:
             case JUMP_ABSOLUTE:
                 h = GETJUMPTGT(codestr, i);
-                tgt = find_op(codestr, h);
+                tgt = find_op(codestr, codelen, h);
                 /* Replace JUMP_* to a RETURN into just a RETURN */
                 if (UNCONDITIONAL_JUMP(opcode) &&
                     _Py_OPCODE(codestr[tgt]) == RETURN_VALUE) {
@@ -417,7 +419,7 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
                 }
                 if (h > i + 1) {
                     fill_nops(codestr, i + 1, h);
-                    nexti = find_op(codestr, h);
+                    nexti = find_op(codestr, codelen, h);
                 }
                 break;
         }