10BC0 gh-106149: Simplify stack depth calculation. Replace asserts by excep… · python/cpython@b0202a4 · GitHub
[go: up one dir, main page]
Skip to content

Commit b0202a4

Browse files
iritkatrieliritkatriel
authored
gh-106149: Simplify stack depth calculation. Replace asserts by exceptions. (#107255)
1 parent 14d074e commit b0202a4

File tree

4 files changed

+91
-42
lines changed

4 files changed

+91
-42
lines changed

‎Include/internal/pycore_flowgraph.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ int _PyCfgBuilder_Init(_PyCfgBuilder *g);
8989
void _PyCfgBuilder_Fini(_PyCfgBuilder *g);
9090

9191
int _PyCfg_OptimizeCodeUnit(_PyCfgBuilder *g, PyObject *consts, PyObject *const_cache,
92-
int code_flags, int nlocals, int nparams, int firstlineno);
93-
int _PyCfg_Stackdepth(_PyCfgBasicblock *entryblock, int code_flags);
92+
int nlocals, int nparams, int firstlineno);
93+
int _PyCfg_Stackdepth(_PyCfgBuilder *g);
9494
void _PyCfg_ConvertPseudoOps(_PyCfgBasicblock *entryblock);
9595
int _PyCfg_ResolveJumps(_PyCfgBuilder *g);
9696

‎Lib/test/test_peepholer.py‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -991,14 +991,15 @@ def test_conditional_jump_forward_non_const_condition(self):
991991
('LOAD_NAME', 1, 11),
992992
('POP_JUMP_IF_TRUE', lbl := self.Label(), 12),
993993
('LOAD_CONST', 2, 13),
994+
('RETURN_VALUE', 13),
994995
lbl,
995996
('LOAD_CONST', 3, 14),
996997
('RETURN_VALUE', 14),
997998
]
998999
expected_insts = [
9991000
('LOAD_NAME', 1, 11),
10001001
('POP_JUMP_IF_TRUE', lbl := self.Label(), 12),
1001-
('LOAD_CONST', 1, 13),
1002+
('RETURN_CONST', 1, 13),
10021003
lbl,
10031004
('RETURN_CONST', 2, 14),
10041005
]
@@ -1072,6 +1073,7 @@ def test_no_unsafe_static_swap(self):
10721073
('STORE_FAST', 1, 4),
10731074
('STORE_FAST', 1, 4),
10741075
('POP_TOP', 0, 4),
1076+
('LOAD_CONST', 0, 5),
10751077
('RETURN_VALUE', 5)
10761078
]
10771079
expected_insts = [
@@ -1080,7 +1082,7 @@ def test_no_unsafe_static_swap(self):
10801082
('NOP', 0, 3),
10811083
('STORE_FAST', 1, 4),
10821084
('POP_TOP', 0, 4),
1083-
('RETURN_VALUE', 5)
1085+
('RETURN_CONST', 0)
10841086
]
10851087
self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)
10861088

@@ -1092,6 +1094,7 @@ def test_dead_store_elimination_in_same_lineno(self):
10921094
('STORE_FAST', 1, 4),
10931095
('STORE_FAST', 1, 4),
10941096
('STORE_FAST', 1, 4),
1097+
('LOAD_CONST', 0, 5),
10951098
('RETURN_VALUE', 5)
10961099
]
10971100
expected_insts = [
@@ -1100,7 +1103,7 @@ def test_dead_store_elimination_in_same_lineno(self):
11001103
('NOP', 0, 3),
11011104
('POP_TOP', 0, 4),
11021105
('STORE_FAST', 1, 4),
1103-
('RETURN_VALUE', 5)
1106+
('RETURN_CONST', 0, 5)
11041107
]
11051108
self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)
11061109

@@ -1112,9 +1115,19 @@ def test_no_dead_store_elimination_in_different_lineno(self):
11121115
('STORE_FAST', 1, 4),
11131116
('STORE_FAST', 1, 5),
11141117
('STORE_FAST', 1, 6),
1118+
('LOAD_CONST', 0, 5),
11151119
('RETURN_VALUE', 5)
11161120
]
1117-
self.cfg_optimization_test(insts, insts, consts=list(range(3)), nlocals=1)
1121+
expected_insts = [
1122+
('LOAD_CONST', 0, 1),
1123+
('LOAD_CONST', 1, 2),
1124+
('LOAD_CONST', 2, 3),
1125+
('STORE_FAST', 1, 4),
1126+
('STORE_FAST', 1, 5),
1127+
('STORE_FAST', 1, 6),
1128+
('RETURN_CONST', 0, 5)
1129+
]
1130+
self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)
11181131

11191132

11201133
if __name__ == "__main__":

‎Python/compile.c‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7527,14 +7527,21 @@ build_cellfixedoffsets(_PyCompile_CodeUnitMetadata *umd)
75277527
return fixed;
75287528
}
75297529

7530+
#define IS_GENERATOR(CF) \
7531+
((CF) & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR))
7532+
75307533
static int
75317534
insert_prefix_instructions(_PyCompile_CodeUnitMetadata *umd, basicblock *entryblock,
75327535
int *fixed, int nfreevars, int code_flags)
75337536
{
75347537
assert(umd->u_firstlineno > 0);
75357538

75367539
/* Add the generator prefix instructions. */
7537-
if (code_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR)) {
7540+
if (IS_GENERATOR(code_flags)) {
7541+
/* Note that RETURN_GENERATOR + POP_TOP have a net stack effect
7542+
* of 0. This is because RETURN_GENERATOR pushes an element
7543+
* with _PyFrame_StackPush before switching stacks.
7544+
*/
75387545
cfg_instr make_gen = {
75397546
.i_opcode = RETURN_GENERATOR,
75407547
.i_oparg = 0,
@@ -7715,19 +7722,27 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache,
77157722
int nparams = (int)PyList_GET_SIZE(u->u_ste->ste_varnames);
77167723
int nlocals = (int)PyDict_GET_SIZE(u->u_metadata.u_varnames);
77177724
assert(u->u_metadata.u_firstlineno);
7718-
if (_PyCfg_OptimizeCodeUnit(&g, consts, const_cache, code_flags, nlocals,
7725+
if (_PyCfg_OptimizeCodeUnit(&g, consts, const_cache, nlocals,
77197726
nparams, u->u_metadata.u_firstlineno) < 0) {
77207727
goto error;
77217728
}
77227729

7723-
/** Assembly **/
7724-
int nlocalsplus = prepare_localsplus(&u->u_metadata, &g, code_flags);
7725-
if (nlocalsplus < 0) {
7730+
int stackdepth = _PyCfg_Stackdepth(&g);
7731+
if (stackdepth < 0) {
77267732
goto error;
77277733
}
77287734

7729-
int maxdepth = _PyCfg_Stackdepth(g.g_entryblock, code_flags);
7730-
if (maxdepth < 0) {
7735+
/* prepare_localsplus adds instructions for generators that push
7736+
* and pop an item on the stack. This assertion makes sure there
7737+
* is space on the stack for that.
7738+
* It should always be true, because at least one expression is
7739+
* required to turn a function into a generator.
7740+
*/
7741+
assert(!(IS_GENERATOR(code_flags) && stackdepth == 0));
7742+
7743+
/** Assembly **/
7744+
int nlocalsplus = prepare_localsplus(&u->u_metadata, &g, code_flags);
7745+
if (nlocalsplus < 0) {
77317746
goto error;
77327747
}
77337748

@@ -7746,7 +7761,7 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache,
77467761
}
77477762

77487763
co = _PyAssemble_MakeCodeObject(&u->u_metadata, const_cache, consts,
7749-
maxdepth, &optimized_instrs, nlocalsplus,
7764+
stackdepth, &optimized_instrs, nlocalsplus,
77507765
code_flags, filename);
77517766

77527767
error:
@@ -8190,8 +8205,8 @@ _PyCompile_OptimizeCfg(PyObject *instructions, PyObject *consts, int nlocals)
81908205
if (instructions_to_cfg(instructions, &g) < 0) {
81918206
goto error;
81928207
}
8193-
int code_flags = 0, nparams = 0, firstlineno = 1;
8194-
if (_PyCfg_OptimizeCodeUnit(&g, consts, const_cache, code_flags, nlocals,
8208+
int nparams = 0, firstlineno = 1;
8209+
if (_PyCfg_OptimizeCodeUnit(&g, consts, const_cache, nlocals,
81958210
nparams, firstlineno) < 0) {
81968211
goto error;
81978212
}
@@ -8226,14 +8241,14 @@ _PyCompile_Assemble(_PyCompile_CodeUnitMetadata *umd, PyObject *filename,
82268241
goto error;
82278242
}
82288243

8229-
int code_flags = 0;
8230-
int nlocalsplus = prepare_localsplus(umd, &g, code_flags);
8231-
if (nlocalsplus < 0) {
8244+
int stackdepth = _PyCfg_Stackdepth(&g);
8245+
if (stackdepth < 0) {
82328246
goto error;
82338247
}
82348248

8235-
int maxdepth = _PyCfg_Stackdepth(g.g_entryblock, code_flags);
8236-
if (maxdepth < 0) {
8249+
int code_flags = 0;
8250+
int nlocalsplus = prepare_localsplus(umd, &g, code_flags);
8251+
if (nlocalsplus < 0) {
82378252
goto error;
82388253
}
82398254

@@ -8256,7 +8271,7 @@ _PyCompile_Assemble(_PyCompile_CodeUnitMetadata *umd, PyObject *filename,
82568271
goto error;
82578272
}
82588273
co = _PyAssemble_MakeCodeObject(umd, const_cache,
8259-
consts, maxdepth, &optimized_instrs,
8274+
consts, stackdepth, &optimized_instrs,
82608275
nlocalsplus, code_flags, filename);
82618276
Py_DECREF(consts);
82628277

‎Python/flowgraph.c‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -615,23 +615,28 @@ make_cfg_traversal_stack(basicblock *entryblock) {
615615
return stack;
616616
}
617617

618-
Py_LOCAL_INLINE(void)
618+
Py_LOCAL_INLINE(int)
619619
stackdepth_push(basicblock ***sp, basicblock *b, int depth)
620620
{
621-
assert(b->b_startdepth < 0 || b->b_startdepth == depth);
621+
if (!(b->b_startdepth < 0 || b->b_startdepth == depth)) {
622+
PyErr_Format(PyExc_ValueError, "Invalid CFG, inconsistent stackdepth");
623+
return ERROR;
624+
}
622625
if (b->b_startdepth < depth && b->b_startdepth < 100) {
623626
assert(b->b_startdepth < 0);
624627
b->b_startdepth = depth;
625628
*(*sp)++ = b;
626629
}
630+
return SUCCESS;
627631
}
628632

629633
/* Find the flow path that needs the largest stack. We assume that
630634
* cycles in the flow graph have no net effect on the stack depth.
631635
*/
632636
int
633-
_PyCfg_Stackdepth(basicblock *entryblock, int code_flags)
637+
_PyCfg_Stackdepth(cfg_builder *g)
634638
{
639+
basicblock *entryblock = g->g_entryblock;
635640
for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
636641
b->b_startdepth = INT_MIN;
637642
}
@@ -640,42 +645,54 @@ _PyCfg_Stackdepth(basicblock *entryblock, int code_flags)
640645
return ERROR;
641646
}
642647

648+
649+
int stackdepth = -1;
643650
int maxdepth = 0;
644651
basicblock **sp = stack;
645-
if (code_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR)) {
646-
stackdepth_push(&sp, entryblock, 1);
647-
} else {
648-
stackdepth_push(&sp, entryblock, 0);
652+
if (stackdepth_push(&sp, entryblock, 0) < 0) {
653+
goto error;
649654
}
650-
651655
while (sp != stack) {
652656
basicblock *b = *--sp;
653657
int depth = b->b_startdepth;
654658
assert(depth >= 0);
655659
basicblock *next = b->b_next;
656660
for (int i = 0; i < b->b_iused; i++) {
657661
cfg_instr *instr = &b->b_instr[i];
658-
int effect = PyCompile_OpcodeStackEffectWithJump(instr->i_opcode, instr->i_oparg, 0);
662+
int effect = PyCompile_OpcodeStackEffectWithJump(
663+
instr->i_opcode, instr->i_oparg, 0);
659664
if (effect == PY_INVALID_STACK_EFFECT) {
660665
PyErr_Format(PyExc_SystemError,
661-
"compiler PyCompile_OpcodeStackEffectWithJump(opcode=%d, arg=%i) failed",
666+
"Invalid stack effect for opcode=%d, arg=%i",
662667
instr->i_opcode, instr->i_oparg);
663-
return ERROR;
668+
goto error;
664669
}
665670
int new_depth = depth + effect;
666-
assert(new_depth >= 0); /* invalid code or bug in stackdepth() */
671+
if (new_depth < 0) {
672+
PyErr_Format(PyExc_ValueError,
673+
"Invalid CFG, stack underflow");
674+
goto error;
675+
}
667676
if (new_depth > maxdepth) {
668677
maxdepth = new_depth;
669678
}
670679
if (HAS_TARGET(instr->i_opcode)) {
671-
effect = PyCompile_OpcodeStackEffectWithJump(instr->i_opcode, instr->i_oparg, 1);
672-
assert(effect != PY_INVALID_STACK_EFFECT);
680+
effect = PyCompile_OpcodeStackEffectWithJump(
681+
instr->i_opcode, instr->i_oparg, 1);
682+
if (effect == PY_INVALID_STACK_EFFECT) {
683+
PyErr_Format(PyExc_SystemError,
684+
"Invalid stack effect for opcode=%d, arg=%i",
685+
instr->i_opcode, instr->i_oparg);
686+
goto error;
687+
}
673688
int target_depth = depth + effect;
674689
assert(target_depth >= 0); /* invalid code or bug in stackdepth() */
675690
if (target_depth > maxdepth) {
676691
maxdepth = target_depth;
677692
}
678-
stackdepth_push(&sp, instr->i_target, target_depth);
693+
if (stackdepth_push(&sp, instr->i_target, target_depth) < 0) {
694+
goto error;
695+
}
679696
}
680697
depth = new_depth;
681698
assert(!IS_ASSEMBLER_OPCODE(instr->i_opcode));
@@ -689,11 +706,15 @@ _PyCfg_Stackdepth(basicblock *entryblock, int code_flags)
689706
}
690707
if (next != NULL) {
691708
assert(BB_HAS_FALLTHROUGH(b));
692-
stackdepth_push(&sp, next, depth);
709+
if (stackdepth_push(&sp, next, depth) < 0) {
710+
goto error;
711+
}
693712
}
694713
}
714+
stackdepth = maxdepth;
715+
error:
695716
PyMem_Free(stack);
696-
return maxdepth;
717+
return stackdepth;
697718
}
698719

699720
static int
@@ -2009,7 +2030,7 @@ mark_cold(basicblock *entryblock) {
20092030

20102031

20112032
static int
2012-
push_cold_blocks_to_end(cfg_builder *g, int code_flags) {
2033+
push_cold_blocks_to_end(cfg_builder *g) {
20132034
basicblock *entryblock = g->g_entryblock;
20142035
if (entryblock->b_next == NULL) {
20152036
/* single basicblock, no need to reorder */
@@ -2251,7 +2272,7 @@ resolve_line_numbers(cfg_builder *g, int firstlineno)
22512272

22522273
int
22532274
_PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,
2254-
int code_flags, int nlocals, int nparams, int firstlineno)
2275+
int nlocals, int nparams, int firstlineno)
22552276
{
22562277
assert(cfg_builder_check(g));
22572278
/** Preprocessing **/
@@ -2268,7 +2289,7 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,
22682289
g->g_entryblock, nlocals, nparams));
22692290
insert_superinstructions(g);
22702291

2271-
RETURN_IF_ERROR(push_cold_blocks_to_end(g, code_flags));
2292+
RETURN_IF_ERROR(push_cold_blocks_to_end(g));
22722293
RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno));
22732294
return SUCCESS;
22742295
}

0 commit comments

Comments
 (0)
0