Skip to content

Commit eec8bf5

Browse files
jgarzikclaude
andcommitted
Import CPython test_tuple.py, fix tuple+/*/index and add bugs.md
Import adapted CPython tuple tests (19 tests) and fix bugs: - Fix tuple+tuple: add sq_concat fallback in BINARY_OP when tp_as_number is NULL - Fix tuple*=int: add sq_repeat fallback for NB_INPLACE_MULTIPLY - Fix tuple['x'] segfault: add int_type check before int_to_i64 - Fix tuple.index() ignoring start/stop parameters - Add bugs.md documenting all 14 bugs found during test import Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fa8f761 commit eec8bf5

6 files changed

Lines changed: 265 additions & 4 deletions

File tree

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ gen-cpython-tests:
7676
@$(PYTHON) -m py_compile tests/cpython/test_augassign.py
7777
@echo "Compiling tests/cpython/test_list.py..."
7878
@$(PYTHON) -m py_compile tests/cpython/test_list.py
79+
@echo "Compiling tests/cpython/test_tuple.py..."
80+
@$(PYTHON) -m py_compile tests/cpython/test_tuple.py
7981
@echo "Done."
8082

8183
check-cpython: $(TARGET) gen-cpython-tests
@@ -99,3 +101,5 @@ check-cpython: $(TARGET) gen-cpython-tests
99101
@./apython tests/cpython/__pycache__/test_augassign.cpython-312.pyc
100102
@echo "Running CPython test_list.py..."
101103
@./apython tests/cpython/__pycache__/test_list.cpython-312.pyc
104+
@echo "Running CPython test_tuple.py..."
105+
@./apython tests/cpython/__pycache__/test_tuple.cpython-312.pyc

bugs.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Bug Log - CPython Container Test Import
2+
3+
## Bugs Found & Fixed
4+
5+
### 1. Negative-step slicelength missing guard (list.asm, tuple.asm)
6+
**Symptom**: `a[3:3:-2]` causes OOM (allocates huge array)
7+
**Root cause**: Negative step slicelength formula `(start-stop-1)/abs(step)+1` used unsigned div without checking `start <= stop` first. When start==stop, `start-stop-1 = -1 = 0xFFFFFFFFFFFFFFFF` unsigned.
8+
**Fix**: Add `jle .empty` guard after `sub rax, r14` in both list and tuple getslice.
9+
10+
### 2. slice_indices start/stop clamping for negative step (slice.asm)
11+
**Symptom**: `a[100:-100:-1]` segfaults (out-of-bounds read)
12+
**Root cause**: `start >= length` clamped to `length` for all steps, but negative step needs `length-1`. `stop < 0` after adding length clamped to 0 for all steps, but negative step needs `-1`.
13+
**Fix**: Conditional clamping based on step sign, matching CPython's PySlice_AdjustIndices.
14+
15+
### 3. slice_indices NONE_SENTINEL collision with sys.maxsize (slice.asm)
16+
**Symptom**: `a[1::sys.maxsize]` treats step as None (defaults to 1)
17+
**Root cause**: NONE_SENTINEL = 0x7FFFFFFFFFFFFFFF == sys.maxsize. When step = sys.maxsize, the comparison `cmp rax, NONE_SENTINEL` matched and step was treated as None.
18+
**Fix**: Check actual payload against `none_singleton` pointer AND tag for `TAG_NONE` or `TAG_PTR` with None, instead of comparing integer values.
19+
20+
### 4. `del a[i]` not decrementing ob_size (list.asm)
21+
**Symptom**: `del a[1]` on `[0, 1]` gives `[0, ]` (size still 2, second slot cleared)
22+
**Root cause**: `list_ass_subscript` with value=NULL always called `list_setitem` which replaces the slot with NULL but doesn't remove it or update size.
23+
**Fix**: Add `.las_int_delete` path that DECREFs old, shifts elements via memmove, decrements `ob_size`.
24+
25+
### 5. `list[string_key]` segfault (list.asm, tuple.asm)
26+
**Symptom**: `a['x']` segfaults instead of raising TypeError
27+
**Root cause**: `list_subscript` assumed non-SmallInt, non-slice keys were heap ints and called `int_to_i64` on them, crashing on strings.
28+
**Fix**: Check `ob_type == int_type` before calling `int_to_i64`; raise TypeError otherwise.
29+
30+
### 6. list.index() ignoring start/stop parameters (methods.asm)
31+
**Symptom**: `[-2,-1,0,0,1,2].index(0, 3)` returns 2 instead of 3
32+
**Root cause**: `list_method_index` always searched from index 0 regardless of nargs.
33+
**Fix**: Check nargs >= 3/4 and extract start/stop from args[2]/args[3] with negative index handling.
34+
35+
### 7. tuple*int dispatch missing (opcodes_misc.asm)
36+
**Symptom**: `(1,2)*2` raises TypeError
37+
**Root cause**: BINARY_OP only checked right operand for sq_repeat when left was SmallInt. When left was a sequence (tuple/list) and right was SmallInt, it fell through to tp_as_number which is NULL for tuples.
38+
**Fix**: Add `.binop_try_left_seq` check in `.binop_not_smallint_left` path.
39+
40+
### 8. tuple+tuple dispatch missing (opcodes_misc.asm)
41+
**Symptom**: `(1,2) + (3,4)` raises TypeError
42+
**Root cause**: BINARY_OP for NB_ADD only checked tp_as_number, not tp_as_sequence.sq_concat.
43+
**Fix**: Add `.binop_try_seq_fallback` that checks sq_concat for ADD and sq_repeat for MULTIPLY when tp_as_number is NULL.
44+
45+
### 9. list_inplace_repeat tag realloc using wrong size (list.asm)
46+
**Symptom**: `s *= 10` corrupts memory / changes list identity
47+
**Root cause**: After payload `ap_realloc` (which returns new ptr in rax), the tag realloc used `rax` (the pointer!) as the size instead of the actual new_size from the stack.
48+
**Fix**: `mov rsi, [rsp]` to load new_size from stack instead of `mov rsi, rax`.
49+
50+
### 10. list_inplace_multiply dispatched to sq_repeat (opcodes_misc.asm)
51+
**Symptom**: `s *= 10` changed list identity (`id(s)` differed after)
52+
**Root cause**: NB_INPLACE_MULTIPLY was dispatched to sq_repeat (creates new object) instead of nb_imul/sq_inplace_repeat (mutates in place).
53+
**Fix**: Remove NB_INPLACE_MULTIPLY from the left-seq sq_repeat dispatch; let it fall through to nb_imul.
54+
55+
### 11. List extended slice self-assignment corruption (list.asm)
56+
**Symptom**: `a[::-1] = a` gives `[0, 1, 1, 0]` instead of `[3, 2, 1, 0]`
57+
**Root cause**: Extended slice loop reads from source and writes to target simultaneously; when source == target, writes corrupt subsequent reads.
58+
**Fix**: Detect self-assignment (`r12 == rbx`), create shallow copy via `list_copy()`, use copy as source. Store copy in LAS_TEMP for cleanup.
59+
60+
### 12. List step-1 slice self-assignment corruption (list.asm)
61+
**Symptom**: `a[1:] = a` gives `[1, 1, 1, 1, 1, 1]` instead of `[1, 1, 2, 3, 4, 5]`
62+
**Root cause**: Same as #11 but for the step-1 path. Also: `call list_copy` clobbered `rcx` (old_len).
63+
**Fix**: Self-assignment detection + `list_copy` + save/restore `rcx` around the call.
64+
65+
### 13. Exhausted list iterator not marked (iter.asm, opcodes_build.asm)
66+
**Symptom**: `list(exhausted_iter)` picks up newly-appended elements
67+
**Root cause**: `FOR_ITER_LIST` specialized opcode didn't mark iterators as exhausted (set `it_seq = NULL`) when done. Only the generic `list_iter_next` path did.
68+
**Fix**: Add `it_seq = NULL` + DECREF in `FOR_ITER_LIST`'s `.fil_exhausted` path. Guard dealloc against NULL `it_seq`.
69+
70+
### 14. tuple.index() ignoring start/stop parameters (methods.asm)
71+
**Symptom**: Same as #6 but for tuples
72+
**Fix**: Same pattern as list.index - check nargs, extract start/stop.
73+
74+
## New Infrastructure Added
75+
- `list_copy()` - standalone shallow copy function
76+
- `ALWAYS_EQ`, `NEVER_EQ`, `C_RECURSION_LIMIT` in `lib/test/support/__init__.py`
77+
- `lib/test/seq_tests.py` - adapted base test class
78+
- `lib/test/list_tests.py` - adapted list test class

src/methods.asm

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6378,20 +6378,66 @@ END_FUNC list_method_reversed
63786378

63796379
;; ============================================================================
63806380
;; tuple_method_index(args, nargs) -> SmallInt index
6381-
;; args[0]=self (tuple), args[1]=value
6381+
;; args[0]=self (tuple), args[1]=value, optional args[2]=start, args[3]=stop
63826382
;; ============================================================================
6383-
DEF_FUNC tuple_method_index
6383+
DEF_FUNC tuple_method_index, 16
63846384
push rbx
63856385
push r12
63866386
push r13
63876387
push r14
63886388

6389+
mov [rbp - 8], rdi ; save args
6390+
mov [rbp - 16], rsi ; save nargs
63896391
mov rbx, [rdi] ; self (tuple)
63906392
mov r12, [rdi + 16] ; value to find (payload)
63916393
mov r14d, [rdi + 24] ; value tag
6392-
mov r13, [rbx + PyTupleObject.ob_size]
6394+
mov r13, [rbx + PyTupleObject.ob_size] ; default stop = size
63936395

6396+
xor ecx, ecx ; default start = 0
6397+
6398+
; Check for optional start arg (nargs >= 3)
6399+
cmp qword [rbp - 16], 3
6400+
jl .ti_have_bounds
6401+
mov rax, [rbp - 8]
6402+
push rcx
6403+
mov rdi, [rax + 32] ; args[2] payload
6404+
mov edx, [rax + 40] ; args[2] tag
6405+
call int_to_i64
6406+
pop rcx
6407+
mov rcx, rax
6408+
; Handle negative start
6409+
test rcx, rcx
6410+
jns .ti_start_pos
6411+
add rcx, r13
6412+
test rcx, rcx
6413+
jns .ti_start_pos
63946414
xor ecx, ecx
6415+
.ti_start_pos:
6416+
6417+
; Check for optional stop arg (nargs >= 4)
6418+
cmp qword [rbp - 16], 4
6419+
jl .ti_have_bounds
6420+
mov rax, [rbp - 8]
6421+
push rcx
6422+
mov rdi, [rax + 48] ; args[3] payload
6423+
mov edx, [rax + 56] ; args[3] tag
6424+
call int_to_i64
6425+
pop rcx
6426+
; Handle negative stop
6427+
test rax, rax
6428+
jns .ti_stop_pos
6429+
add rax, r13
6430+
test rax, rax
6431+
jns .ti_stop_pos
6432+
xor eax, eax
6433+
.ti_stop_pos:
6434+
cmp rax, r13
6435+
jle .ti_stop_ok
6436+
mov rax, r13
6437+
.ti_stop_ok:
6438+
mov r13, rax ; r13 = stop
6439+
6440+
.ti_have_bounds:
63956441
.tindex_loop:
63966442
cmp rcx, r13
63976443
jge .tindex_not_found

src/opcodes_misc.asm

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,14 @@ DEF_FUNC_BARE op_binary_op
286286
lea rax, [rel int_type]
287287
jmp .binop_have_type
288288
.binop_have_type:
289+
push rax ; save type ptr for sq fallback
289290
mov rax, [rax + PyTypeObject.tp_as_number]
290291
test rax, rax
291-
jz .binop_try_dunder
292+
jnz .binop_have_number
293+
pop rax ; restore type ptr
294+
jmp .binop_try_seq_fallback
295+
.binop_have_number:
296+
add rsp, 8 ; discard saved type ptr
292297
jmp .binop_call_method
293298

294299
.use_float_methods:
@@ -394,6 +399,41 @@ DEF_FUNC_BARE op_binary_op
394399
add rbx, 2
395400
DISPATCH
396401

402+
.binop_try_seq_fallback:
403+
; rax = type ptr. Check if type has tp_as_sequence for ADD/MUL ops.
404+
mov rax, [rax + PyTypeObject.tp_as_sequence]
405+
test rax, rax
406+
jz .binop_try_dunder
407+
; NB_ADD (0) or NB_INPLACE_ADD (13) → sq_concat / sq_inplace_concat
408+
cmp r9d, 0 ; NB_ADD
409+
je .binop_seq_concat
410+
cmp r9d, 13 ; NB_INPLACE_ADD
411+
je .binop_seq_concat
412+
; NB_MULTIPLY (5) or NB_INPLACE_MULTIPLY (18) → sq_repeat
413+
cmp r9d, 5
414+
je .binop_seq_repeat_left
415+
cmp r9d, 18 ; NB_INPLACE_MULTIPLY
416+
je .binop_seq_repeat_left
417+
jmp .binop_try_dunder
418+
419+
.binop_seq_concat:
420+
mov rax, [rax + PySequenceMethods.sq_concat]
421+
test rax, rax
422+
jz .binop_try_dunder
423+
; sq_concat(left, right): rdi=left, rsi=right already set
424+
call rax
425+
jmp .binop_have_result
426+
427+
.binop_seq_repeat_left:
428+
mov rax, [rax + PySequenceMethods.sq_repeat]
429+
test rax, rax
430+
jz .binop_try_dunder
431+
; sq_repeat(left=sequence, right=count)
432+
mov edx, [rsp + BO_RTAG]
433+
mov ecx, edx
434+
call rax
435+
jmp .binop_have_result
436+
397437
.binop_try_dunder:
398438
; Try dunder method on heaptype objects
399439
extern binop_dunder_table

src/pyo/tuple.asm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ DEF_FUNC tuple_subscript
168168
lea rcx, [rel slice_type]
169169
cmp rax, rcx
170170
je .ts_slice
171+
; Check if key is actually an int type
172+
lea rcx, [rel int_type]
173+
cmp rax, rcx
174+
jne .ts_type_error
171175

172176
.ts_int:
173177
mov rdi, rsi ; key
@@ -186,6 +190,11 @@ DEF_FUNC tuple_subscript
186190
pop rbx
187191
leave
188192
ret
193+
194+
.ts_type_error:
195+
lea rdi, [rel exc_TypeError_type]
196+
CSTRING rsi, "tuple indices must be integers or slices"
197+
call raise_exception
189198
END_FUNC tuple_subscript
190199

191200
; tuple_len(PyTupleObject *tuple) -> int64_t

tests/cpython/test_tuple.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
"""CPython test_tuple.py adapted for apython."""
2+
from test import seq_tests
3+
import unittest
4+
5+
class TupleTest(seq_tests.CommonTest):
6+
type2test = tuple
7+
8+
def test_constructors(self):
9+
self.assertEqual(tuple(), ())
10+
t0_3 = (0, 1, 2, 3)
11+
t0_3_bis = tuple(t0_3)
12+
# Skip identity test: tuple(t) is t is a CPython optimization
13+
self.assertEqual(t0_3, t0_3_bis)
14+
self.assertEqual(tuple([]), ())
15+
self.assertEqual(tuple([0, 1, 2, 3]), (0, 1, 2, 3))
16+
self.assertEqual(tuple(''), ())
17+
self.assertEqual(tuple('spam'), ('s', 'p', 'a', 'm'))
18+
self.assertEqual(tuple(x for x in range(10) if x % 2),
19+
(1, 3, 5, 7, 9))
20+
21+
def test_truth(self):
22+
super().test_truth()
23+
self.assertTrue(not ())
24+
self.assertTrue((42, ))
25+
26+
def test_len(self):
27+
super().test_len()
28+
self.assertEqual(len(()), 0)
29+
self.assertEqual(len((0,)), 1)
30+
self.assertEqual(len((0, 1, 2)), 3)
31+
32+
def test_iadd(self):
33+
super().test_iadd()
34+
u = (0, 1)
35+
u2 = u
36+
u += (2, 3)
37+
self.assertTrue(u is not u2)
38+
39+
def test_imul(self):
40+
super().test_imul()
41+
u = (0, 1)
42+
u2 = u
43+
u *= 3
44+
self.assertTrue(u is not u2)
45+
46+
def test_tupleresizebug(self):
47+
# Check that a specific bug in _PyTuple_Resize() is squashed.
48+
def f():
49+
for i in range(1000):
50+
yield i
51+
self.assertEqual(list(tuple(f())), list(range(1000)))
52+
53+
def test_repr(self):
54+
l0 = tuple()
55+
l2 = (0, 1, 2)
56+
a0 = self.type2test(l0)
57+
a2 = self.type2test(l2)
58+
59+
self.assertEqual(str(a0), repr(l0))
60+
self.assertEqual(str(a2), repr(l2))
61+
self.assertEqual(repr(a0), "()")
62+
self.assertEqual(repr(a2), "(0, 1, 2)")
63+
64+
def test_repr_large(self):
65+
# Check the repr of large tuple objects
66+
def check(n):
67+
l = (0,) * n
68+
s = repr(l)
69+
self.assertEqual(s,
70+
'(' + ', '.join(['0'] * n) + ')')
71+
check(10) # check our checking code
72+
check(1000000)
73+
74+
def test_lexicographic_ordering(self):
75+
# Issue 21100
76+
a = self.type2test([1, 2])
77+
b = self.type2test([1, 2, 0])
78+
c = self.type2test([1, 3])
79+
self.assertLess(a, b)
80+
self.assertLess(b, c)
81+
82+
83+
if __name__ == "__main__":
84+
unittest.main()

0 commit comments

Comments
 (0)