Repository: numba/numba
Pull request: Fix PyLowering mishandling starargs
Fix #8059.



Problem statement:
receive segfault when using objmode and starargs
```
from numba import njit, objmode, types, typed, gdb_init

def simple_func(arg):
    if arg is None:
        return False
    else:
        return True


def test_misc13():
    @njit
    def foo():
        func_name = "simple_func"
        args = typed.List(["arg"])
        # gdb_init()
        with objmode(ret=types.unicode_type):
            ret = str(globals()[func_name](*args))
        print(ret)
        return ret

    pv = foo.py_func()
    jv = foo()
    assert pv == jv

```
This piece of code can be executed by `foo.py_func()`, but report a segfault as below when executing `foo()`. I tried to replace `args` with reflected list, type infer will report an `TypingError`, so I used `typed.List`. If I change `*args` to `args[0]`, no segfault was reported. But in the real case, the `func_name` can accept multiple args, so I need to use `*args`.

```
test_deque.py::test_misc13 True
Fatal Python error: Segmentation fault

Current thread 0x00007f0ef5aad700 (most recent call first):
  File "/home/dali.da/code/asdc/sdc/tests/tests_ant/test_deque.py", line 883 in test_misc13
  File "/home/dali.da/.pyenv/versions/3.7.12/lib/python3.7/site-packages/_pytest/python.py", line 192 in pytest_pyfunc_call
  File "/home/dali.da/.pyenv/versions/3.7.12/lib/python3.7/site-packages/pluggy/_callers.py", line 39 in _multicall
```

Hints from issue discussion:
@dlee992 thank you for submitting this!

One quick note before I go to debug it. I noticed that you tend to report reproducers without the import statements. For ease of reproducibility it makes sense to submit fully runnable reproducers, since this makes it easier to just copy and paste the example without having to patch it up. Something to keep in mind for future reports,  thank you!
@dlee992 thank you, I can confirm I am able to repro on current `main` at `4684b9cf9d5e42ec0e144c3f238432b078627a1c`.
> One quick note before I go to debug it. I noticed that you tend to report reproducers without the import statements. For ease of reproducibility it makes sense to submit fully runnable reproducers, since this makes it easier to just copy and paste the example without having to patch it up. Something to keep in mind for future reports, thank you!

Sry, I have added the import statements and will notice this in the future.

> > One quick note before I go to debug it. I noticed that you tend to report reproducers without the import statements. For ease of reproducibility it makes sense to submit fully runnable reproducers, since this makes it easier to just copy and paste the example without having to patch it up. Something to keep in mind for future reports, thank you!
> 
> Sry, I have added the import statements and will notice this in the future.

Thank you!
Some variations on this also segfault:
```python
from numba import njit, objmode, typed

def consumer(*x):
    print(x)

@njit
def gen():
    yield 1

@njit
def foo():
    # x = set((1,)) # segfault
    # x = [1,] # blocked by typing reporting lack of support
    # x = gen() # segfault
    # x = "abc" # segfault
    # x = ('a', 'b', 'c') # works
    # x = (1,) # works
    # x = range(4) # TypeError!?
    # x = iter([1, 2]) # TypeError!?
    # x = {1:1,} # segfault
    with objmode():
        consumer(*x)

foo()

```
Ok, i think this is a a objmode lowering problem:

```python
from numba import jit

def consumer(*x):
    print(x)


@jit(forceobj=True)
def foo(x):
    consumer(*x)

foo("ijo")
```
^ this segfault
Oh~~ there's more problem

```
from numba import jit

def consumer(*x):
    print(x)


@jit(forceobj=True)
def foo(x):
    consumer(1, 2, *x)

foo("ijo")
```
The `LIST_TO_TUPLE`  transform cannot just use `operator.add` because one of the operand may not be a tuple.

Py3.9 Bytecode:
```
>          0	NOP(arg=None, lineno=63)
           2	LOAD_GLOBAL(arg=0, lineno=63)
           4	LOAD_CONST(arg=1, lineno=63)
           6	LOAD_CONST(arg=2, lineno=63)
           8	BUILD_LIST(arg=2, lineno=63)
          10	LOAD_FAST(arg=0, lineno=63)
          12	LIST_EXTEND(arg=1, lineno=63)
          14	LIST_TO_TUPLE(arg=None, lineno=63)
          16	CALL_FUNCTION_EX(arg=0, lineno=63)
          18	POP_TOP(arg=None, lineno=63)
          20	LOAD_CONST(arg=0, lineno=63)
          22	RETURN_VALUE(arg=None, lineno=63)
```
The Numba IR:

```
----------------------------------IR DUMP: foo----------------------------------
label 0:
    x = arg(0, name=x)                       ['x']
    $2load_global.0 = global(consumer: <function consumer at 0x7fa1600ea040>) ['$2load_global.0']
    $const4.1 = const(int, 1)                ['$const4.1']
    $const6.2 = const(int, 2)                ['$const6.2']
    $8build_list.3 = build_tuple(items=[Var($const4.1, chk.py:63), Var($const6.2, chk.py:63)]) ['$8build_list.3', '$const4.1', '$const6.2']
    $12list_extend.5_var_x = x               ['$12list_extend.5_var_x', 'x']
    $12list_extend.6 = $8build_list.3 + $12list_extend.5_var_x ['$12list_extend.5_var_x', '$12list_extend.6', '$8build_list.3']
    $14list_to_tuple.7 = $12list_extend.6    ['$12list_extend.6', '$14list_to_tuple.7']
    $16call_function_ex.8 = call $2load_global.0(*$14list_to_tuple.7, func=$2load_global.0, args=[], kws=[], vararg=$14list_to_tuple.7, varkwarg=None, target=None) ['$14list_to_tuple.7', '$16call_function_ex.8', '$2load_global.0']
    $const20.9 = const(NoneType, None)       ['$const20.9']
    $22return_value.10 = cast(value=$const20.9) ['$22return_value.10', '$const20.9']
    return $22return_value.10                ['$22return_value.10']
```
From OOB discussion with @sklam. One potential route to fix this is to add support for a `LIST_TO_TUPLE` opcode in the IR. Then push the bytecode transform for `LIST_TO_TUPLE` https://github.com/numba/numba/blob/468647dddde27ee8af124c97dfcd20c35c4a2bc6/numba/core/interpreter.py#L692-L723 out of the interpreter an into a standard function pass. 

At this point:
* The `nopython` mode compilation pipeline will need include the new `list_to_tuple` function pass, this allows `nopython` mode to continue handling `list_to_tuple` via a chain of tuple additions.
* The object mode support needs the `PyLowering` lowering implementation extending to be able to translate the Numba IR `list_to_tuple` node to `PyAPI` calls.

This leads to `nopython` mode working in approximately the same way as it does now and `objmode` working much more like CPython, which is the intended behaviour.

Commit message: Fix incorrect refct for `tuple(tuple_obj)`

Patch to review:
diff --git a/.flake8 b/.flake8
index b5e2c1abe61..9d77a1d5e38 100644
--- a/.flake8
+++ b/.flake8
@@ -150,7 +150,6 @@ exclude =
     numba/tests/test_numberctor.py
     numba/tests/test_globals.py
     numba/tests/test_typingerror.py
-    numba/tests/test_object_mode.py
     numba/tests/test_copy_propagate.py
     numba/tests/test_ctypes.py
     numba/tests/test_typeof.py
diff --git a/numba/core/interpreter.py b/numba/core/interpreter.py
index cf0cab1dbe1..34c0bc93c8a 100644
--- a/numba/core/interpreter.py
+++ b/numba/core/interpreter.py
@@ -831,7 +831,26 @@ def append_and_fix(x):
                                             bt = ir.Expr.build_tuple([arg,],
                                                                      expr.loc)
                                         else:
-                                            bt = arg
+                                            # Extend as tuple
+                                            gv_tuple = ir.Global(
+                                                name="tuple", value=tuple,
+                                                loc=expr.loc,
+                                            )
+                                            tuple_var = arg.scope.redefine(
+                                                "_list_extend_gv_tuple",
+                                                loc=expr.loc,
+                                            )
+                                            new_hole.append(
+                                                ir.Assign(
+                                                    target=tuple_var,
+                                                    value=gv_tuple,
+                                                    loc=expr.loc,
+                                                ),
+                                            )
+                                            bt = ir.Expr.call(
+                                                tuple_var, (arg,), (),
+                                                loc=expr.loc,
+                                            )
                                         var = ir.Var(arg.scope, tmp_name,
                                                      expr.loc)
                                         asgn = ir.Assign(bt, var, expr.loc)
@@ -1856,9 +1875,26 @@ def _build_tuple_unpack(self, inst, tuples, temps, is_assign):
                                loc=self.loc,)
             self.store(exc, temps[0])
         else:
+            loc = self.loc
             for other, tmp in zip(map(self.get, tuples[1:]), temps):
-                out = ir.Expr.binop(fn=operator.add, lhs=first, rhs=other,
-                                    loc=self.loc)
+                # Emit as `first + tuple(other)`
+                gv_tuple = ir.Global(
+                    name="tuple", value=tuple,
+                    loc=loc,
+                )
+                tuple_var = self.store(
+                    gv_tuple, "_list_extend_gv_tuple", redefine=True,
+                )
+                tuplify_val = ir.Expr.call(
+                    tuple_var, (other,), (),
+                    loc=loc,
+                )
+                tuplify_var = self.store(tuplify_val, "_tuplify", redefine=True)
+
+                out = ir.Expr.binop(
+                    fn=operator.add, lhs=first, rhs=self.get(tuplify_var.name),
+                    loc=self.loc,
+                )
                 self.store(out, tmp)
                 first = self.get(tmp)
 
diff --git a/numba/core/pylowering.py b/numba/core/pylowering.py
index b33991e7122..017124c632b 100644
--- a/numba/core/pylowering.py
+++ b/numba/core/pylowering.py
@@ -289,8 +289,10 @@ def lower_expr(self, expr):
             args = self.pyapi.tuple_pack(argvals)
             if expr.vararg:
                 # Expand *args
-                new_args = self.pyapi.number_add(args,
-                                                 self.loadvar(expr.vararg.name))
+                varargs = self.pyapi.sequence_tuple(
+                                self.loadvar(expr.vararg.name))
+                new_args = self.pyapi.sequence_concat(args, varargs)
+                self.decref(varargs)
                 self.decref(args)
                 args = new_args
             if not expr.kws:
diff --git a/numba/core/pythonapi.py b/numba/core/pythonapi.py
index 421a491cca0..124e69b8fe2 100644
--- a/numba/core/pythonapi.py
+++ b/numba/core/pythonapi.py
@@ -697,6 +697,11 @@ def sequence_tuple(self, obj):
         fn = self._get_function(fnty, name="PySequence_Tuple")
         return self.builder.call(fn, [obj])
 
+    def sequence_concat(self, obj1, obj2):
+        fnty = ir.FunctionType(self.pyobj, [self.pyobj, self.pyobj])
+        fn = self._get_function(fnty, name="PySequence_Concat")
+        return self.builder.call(fn, [obj1, obj2])
+
     def list_new(self, szval):
         fnty = ir.FunctionType(self.pyobj, [self.py_ssize_t])
         fn = self._get_function(fnty, name="PyList_New")
diff --git a/numba/cpython/builtins.py b/numba/cpython/builtins.py
index 6fbb2cf4405..44534bf976d 100644
--- a/numba/cpython/builtins.py
+++ b/numba/cpython/builtins.py
@@ -436,7 +436,7 @@ def lower_empty_tuple(context, builder, sig, args):
 @lower_builtin(tuple, types.BaseTuple)
 def lower_tuple(context, builder, sig, args):
     val, = args
-    return impl_ret_untracked(context, builder, sig.return_type, val)
+    return impl_ret_borrowed(context, builder, sig.return_type, val)
 
 @overload(bool)
 def bool_sequence(x):
diff --git a/numba/tests/test_interpreter.py b/numba/tests/test_interpreter.py
index 3cf3a2a0983..00d7a96dbe0 100644
--- a/numba/tests/test_interpreter.py
+++ b/numba/tests/test_interpreter.py
@@ -1,7 +1,9 @@
 """
 Test bytecode fixes provided in interpreter.py
 """
-from numba import njit, objmode
+import unittest
+
+from numba import jit, njit, objmode
 from numba.core.errors import UnsupportedError
 from numba.tests.support import TestCase, MemoryLeakMixin, skip_unless_py10
 
@@ -107,7 +109,7 @@ def sum_jit_func(
     )
 
 
-class TestCallFunctionExPeepHole(TestCase, MemoryLeakMixin):
+class TestCallFunctionExPeepHole(MemoryLeakMixin, TestCase):
     """
     gh #7812
 
@@ -618,3 +620,32 @@ def inline_func(flag):
         a = py_func(False)
         b = cfunc(False)
         self.assertEqual(a, b)
+
+
+class TestListExtendInStarArgNonTupleIterable(MemoryLeakMixin, TestCase):
+    """Test `fn(pos_arg0, pos_arg1, *args)` where args is a non-tuple iterable.
+
+    Python will generate LIST_EXTEND bytecode to combine the positional
+    arguments with the `*args`.
+
+    See #8059
+
+    NOTE: At the moment, there are no meaningful test for NoPython because the
+    lack of support for `tuple(iterable)` for most iterable types.
+    """
+    def test_list_extend_forceobj(self):
+        def consumer(*x):
+            return x
+
+        @jit(forceobj=True)
+        def foo(x):
+            return consumer(1, 2, *x)
+
+        got = foo("ijo")
+        expect = foo.py_func("ijo")
+        self.assertEqual(got, (1, 2, "i", "j", "o"))
+        self.assertEqual(got, expect)
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/numba/tests/test_object_mode.py b/numba/tests/test_object_mode.py
index 6c8c1bf8e29..e2dfedf822b 100644
--- a/numba/tests/test_object_mode.py
+++ b/numba/tests/test_object_mode.py
@@ -16,9 +16,11 @@ def complex_constant(n):
     tmp = n + 4
     return tmp + 3j
 
+
 def long_constant(n):
     return n + 100000000000000000000000000000000000000000000000
 
+
 def delitem_usecase(x):
     del x[:]
 
@@ -31,7 +33,7 @@ def loop_nest_3(x, y):
     n = 0
     for i in range(x):
         for j in range(y):
-            for k in range(x+y):
+            for k in range(x + y):
                 n += i * j
 
     return n
@@ -107,6 +109,20 @@ def test_delitem(self):
         with self.assertRaises(TypeError):
             cfunc(42)
 
+    def test_starargs_non_tuple(self):
+        def consumer(*x):
+            return x
+
+        @jit(forceobj=True)
+        def foo(x):
+            return consumer(*x)
+
+        arg = "ijo"
+        got = foo(arg)
+        expect = foo.py_func(arg)
+        self.assertEqual(got, tuple(arg))
+        self.assertEqual(got, expect)
+
 
 class TestObjectModeInvalidRewrite(TestCase):
     """


Code review comments:
diff_hunk: @@ -1856,9 +1875,26 @@ def _build_tuple_unpack(self, inst, tuples, temps, is_assign):
                                loc=self.loc,)
             self.store(exc, temps[0])
         else:
+            loc = self.loc
             for other, tmp in zip(map(self.get, tuples[1:]), temps):
-                out = ir.Expr.binop(fn=operator.add, lhs=first, rhs=other,
-                                    loc=self.loc)
+                # Emit as `first + tuple(other)`
+                gv_tuple = ir.Global(
+                    name="tuple", value=tuple,
+                    loc=loc,
+                )
+                tuple_var = self.store(
+                    gv_tuple, "_list_extend_gv_tuple", redefine=True,
+                )
+                tuplify_val = ir.Expr.call(
+                    tuple_var, (other,), (),
+                    loc=loc,
+                )
+                tuplify_var = self.store(tuplify_val, "_tuplify", redefine=True)
path: numba/core/interpreter.py
text: @user1:
```suggestion
                tuplify_var = self.store(tuplify_val, "$_tuplify", redefine=True)
```
internal var?

diff_hunk: @@ -1856,9 +1875,26 @@ def _build_tuple_unpack(self, inst, tuples, temps, is_assign):
                                loc=self.loc,)
             self.store(exc, temps[0])
         else:
+            loc = self.loc
             for other, tmp in zip(map(self.get, tuples[1:]), temps):
-                out = ir.Expr.binop(fn=operator.add, lhs=first, rhs=other,
-                                    loc=self.loc)
+                # Emit as `first + tuple(other)`
+                gv_tuple = ir.Global(
+                    name="tuple", value=tuple,
+                    loc=loc,
+                )
+                tuple_var = self.store(
+                    gv_tuple, "_list_extend_gv_tuple", redefine=True,
path: numba/core/interpreter.py
text: @user1:
```suggestion
                    gv_tuple, "$_list_extend_gv_tuple", redefine=True,
```
internal var?

diff_hunk: @@ -831,7 +831,26 @@ def append_and_fix(x):
                                             bt = ir.Expr.build_tuple([arg,],
                                                                      expr.loc)
                                         else:
-                                            bt = arg
+                                            # Extend as tuple
+                                            gv_tuple = ir.Global(
+                                                name="tuple", value=tuple,
+                                                loc=expr.loc,
+                                            )
+                                            tuple_var = arg.scope.redefine(
+                                                "_list_extend_gv_tuple",
path: numba/core/interpreter.py
text: @user1:
```suggestion
                                                "$_list_extend_gv_tuple",
```
internal var?