From 53f51f1c9e4bb758d4863ff4e106f5e979d0959e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 28 Mar 2022 18:06:26 +0100 Subject: [PATCH 1/8] bpo-47172: Make virtual opcodes > 256. Make is_jump detect only actual jumps, use is_block_push for the exception block setup opcodes. --- Python/compile.c | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 06edcf1810e640..e118dbd47f548e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -72,17 +72,19 @@ /* Pseudo-instructions used in the compiler, * but turned into NOPs by the assembler. */ -#define SETUP_FINALLY 255 -#define SETUP_CLEANUP 254 -#define SETUP_WITH 253 -#define POP_BLOCK 252 +#define SETUP_FINALLY 256 +#define SETUP_CLEANUP 257 +#define SETUP_WITH 258 +#define POP_BLOCK 259 + +#define IS_VIRTUAL_OPCODE(opcode) ((opcode) >= 256) #define IS_TOP_LEVEL_AWAIT(c) ( \ (c->c_flags->cf_flags & PyCF_ALLOW_TOP_LEVEL_AWAIT) \ && (c->u->u_ste->ste_type == ModuleBlock)) struct instr { - unsigned char i_opcode; + int i_opcode; int i_oparg; /* target block (if jump instruction) */ struct basicblock_ *i_target; @@ -124,10 +126,17 @@ is_relative_jump(struct instr *i) return is_bit_set_in_table(_PyOpcode_RelativeJump, i->i_opcode); } +static inline int +is_block_push(struct instr *instr) +{ + int opcode = instr->i_opcode; + return opcode == SETUP_FINALLY || opcode == SETUP_WITH || opcode == SETUP_CLEANUP; +} + static inline int is_jump(struct instr *i) { - return i->i_opcode >= SETUP_WITH || is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); + return is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); } static int @@ -157,6 +166,7 @@ write_instr(_Py_CODEUNIT *codestr, struct instr *instruction, int ilen) *codestr++ = _Py_MAKECODEUNIT(EXTENDED_ARG, (oparg >> 8) & 0xFF); /* fall through */ case 1: + assert(!IS_VIRTUAL_OPCODE(opcode)); *codestr++ = _Py_MAKECODEUNIT(opcode, oparg & 0xFF); break; default: @@ -7034,7 +7044,7 @@ stackdepth(struct compiler *c) maxdepth = new_depth; } assert(depth >= 0); /* invalid code or bug in stackdepth() */ - if (is_jump(instr)) { + if (is_jump(instr) || is_block_push(instr)) { effect = stack_effect(instr->i_opcode, instr->i_oparg, 1); assert(effect != PY_INVALID_STACK_EFFECT); int target_depth = depth + effect; @@ -7152,13 +7162,6 @@ assemble_emit_table_pair(struct assembler* a, PyObject** table, int* offset, return 1; } -static int -is_block_push(struct instr *instr) -{ - int opcode = instr->i_opcode; - return opcode == SETUP_FINALLY || opcode == SETUP_WITH || opcode == SETUP_CLEANUP; -} - static basicblock * push_except_block(ExceptStack *stack, struct instr *setup) { assert(is_block_push(setup)); @@ -7593,6 +7596,7 @@ assemble_jump_offsets(struct assembler *a, struct compiler *c) bsize = b->b_offset; for (i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; + assert(!IS_VIRTUAL_OPCODE(instr->i_opcode)); int isize = instr_size(instr); /* Relative jumps are computed relative to the instruction pointer after fetching @@ -8586,6 +8590,7 @@ apply_static_swaps(basicblock *block, int i) static bool jump_thread(struct instr *inst, struct instr *target, int opcode) { + assert(!IS_VIRTUAL_OPCODE(opcode)); assert(is_jump(inst)); assert(is_jump(target)); // bpo-45773: If inst->i_target == target->i_target, then nothing actually @@ -8615,7 +8620,7 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts) struct instr *inst = &bb->b_instr[i]; int oparg = inst->i_oparg; int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0; - if (is_jump(inst)) { + if (is_jump(inst) || is_block_push(inst)) { /* Skip over empty basic blocks. */ while (inst->i_target->b_iused == 0) { inst->i_target = inst->i_target->b_next; @@ -8975,8 +8980,9 @@ mark_reachable(struct assembler *a) { } for (int i = 0; i < b->b_iused; i++) { basicblock *target; - if (is_jump(&b->b_instr[i])) { - target = b->b_instr[i].i_target; + struct instr *instr = &b->b_instr[i]; + if (is_jump(instr) || is_block_push(instr)) { + target = instr->i_target; if (target->b_predecessors == 0) { *sp++ = target; } @@ -9183,13 +9189,6 @@ duplicate_exits_without_lineno(struct compiler *c) */ for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { if (b->b_iused > 0 && is_jump(&b->b_instr[b->b_iused-1])) { - switch (b->b_instr[b->b_iused-1].i_opcode) { - /* Note: Only actual jumps, not exception handlers */ - case SETUP_WITH: - case SETUP_FINALLY: - case SETUP_CLEANUP: - continue; - } basicblock *target = b->b_instr[b->b_iused-1].i_target; if (is_exit_without_lineno(target) && target->b_predecessors > 1) { basicblock *new_target = compiler_copy_block(c, target); From bbd52571546d00d9aa4221c67c39e58835c32c56 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 30 Mar 2022 18:55:33 +0100 Subject: [PATCH 2/8] remove one more unnecessary check after an is_jump --- Python/compile.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index e118dbd47f548e..78d0a8876e4ea4 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -9058,13 +9058,6 @@ propagate_line_numbers(struct assembler *a) { } } if (is_jump(&b->b_instr[b->b_iused-1])) { - switch (b->b_instr[b->b_iused-1].i_opcode) { - /* Note: Only actual jumps, not exception handlers */ - case SETUP_WITH: - case SETUP_FINALLY: - case SETUP_CLEANUP: - continue; - } basicblock *target = b->b_instr[b->b_iused-1].i_target; if (target->b_predecessors == 1) { if (target->b_instr[0].i_lineno < 0) { From 89745562812bb03ea8eb3a1060b0ea1abb38d7d3 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 30 Mar 2022 19:17:18 +0100 Subject: [PATCH 3/8] updated followign Brandt's review --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 78d0a8876e4ea4..3c6df1b2c69279 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -153,6 +153,7 @@ static void write_instr(_Py_CODEUNIT *codestr, struct instr *instruction, int ilen) { int opcode = instruction->i_opcode; + assert(!IS_VIRTUAL_OPCODE(opcode)); int oparg = HAS_ARG(opcode) ? instruction->i_oparg : 0; int caches = _PyOpcode_Caches[opcode]; switch (ilen - caches) { @@ -166,7 +167,6 @@ write_instr(_Py_CODEUNIT *codestr, struct instr *instruction, int ilen) *codestr++ = _Py_MAKECODEUNIT(EXTENDED_ARG, (oparg >> 8) & 0xFF); /* fall through */ case 1: - assert(!IS_VIRTUAL_OPCODE(opcode)); *codestr++ = _Py_MAKECODEUNIT(opcode, oparg & 0xFF); break; default: From 927c547f4d4d363b0c28d8adf3394e69e9bdf705 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 30 Mar 2022 19:35:38 +0100 Subject: [PATCH 4/8] move assertion into instr_size --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 3c6df1b2c69279..9005955958d6b1 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -143,6 +143,7 @@ static int instr_size(struct instr *instruction) { int opcode = instruction->i_opcode; + assert(!IS_VIRTUAL_OPCODE(opcode)); int oparg = HAS_ARG(opcode) ? instruction->i_oparg : 0; int extended_args = (0xFFFFFF < oparg) + (0xFFFF < oparg) + (0xFF < oparg); int caches = _PyOpcode_Caches[opcode]; @@ -7596,7 +7597,6 @@ assemble_jump_offsets(struct assembler *a, struct compiler *c) bsize = b->b_offset; for (i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; - assert(!IS_VIRTUAL_OPCODE(instr->i_opcode)); int isize = instr_size(instr); /* Relative jumps are computed relative to the instruction pointer after fetching From bff07747fce5cfbe94b39af1ec808286ca9392cd Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 30 Mar 2022 19:43:15 +0100 Subject: [PATCH 5/8] Guard is_bit_set_in_table call. Co-authored-by: Brandt Bucher --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 9005955958d6b1..188a2392e98147 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -136,7 +136,7 @@ is_block_push(struct instr *instr) static inline int is_jump(struct instr *i) { - return is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); + return !IS_VIRTUAL_OPCODE(i->i_opcode) && is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); } static int From 937139d210539a672279a5c3a14d32c89b7e059e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 30 Mar 2022 20:05:53 +0100 Subject: [PATCH 6/8] move bounds check into is_bit_set_in_table --- Python/compile.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 188a2392e98147..9240dd9f38291b 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -116,8 +116,13 @@ is_bit_set_in_table(const uint32_t *table, int bitindex) { * Word is indexed by (bitindex>>ln(size of int in bits)). * Bit within word is the low bits of bitindex. */ - uint32_t word = table[bitindex >> LOG_BITS_PER_INT]; - return (word >> (bitindex & MASK_LOW_LOG_BITS)) & 1; + if (bitindex < 256) { + uint32_t word = table[bitindex >> LOG_BITS_PER_INT]; + return (word >> (bitindex & MASK_LOW_LOG_BITS)) & 1; + } + else { + return 0; + } } static inline int @@ -136,7 +141,7 @@ is_block_push(struct instr *instr) static inline int is_jump(struct instr *i) { - return !IS_VIRTUAL_OPCODE(i->i_opcode) && is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); + return is_bit_set_in_table(_PyOpcode_Jump, i->i_opcode); } static int From c7ae11b8e344571ed7059de9fbe5064700d1391c Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 31 Mar 2022 18:14:07 +0100 Subject: [PATCH 7/8] make virtual opcodes negative --- Python/compile.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 41c449b8adad2e..0f23af8f825794 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -72,13 +72,13 @@ /* Pseudo-instructions used in the compiler, * but turned into NOPs by the assembler. */ -#define SETUP_FINALLY 256 -#define SETUP_CLEANUP 257 -#define SETUP_WITH 258 -#define POP_BLOCK 259 -#define JUMP 260 +#define SETUP_FINALLY -1 +#define SETUP_CLEANUP -2 +#define SETUP_WITH -3 +#define POP_BLOCK -4 +#define JUMP -5 -#define IS_VIRTUAL_OPCODE(opcode) ((opcode) >= 256) +#define IS_VIRTUAL_OPCODE(opcode) ((opcode) < 0) #define IS_TOP_LEVEL_AWAIT(c) ( \ (c->c_flags->cf_flags & PyCF_ALLOW_TOP_LEVEL_AWAIT) \ @@ -117,7 +117,7 @@ is_bit_set_in_table(const uint32_t *table, int bitindex) { * Word is indexed by (bitindex>>ln(size of int in bits)). * Bit within word is the low bits of bitindex. */ - if (bitindex < 256) { + if (bitindex >= 0) { uint32_t word = table[bitindex >> LOG_BITS_PER_INT]; return (word >> (bitindex & MASK_LOW_LOG_BITS)) & 1; } @@ -1477,7 +1477,7 @@ static int add_jump_to_block(struct compiler *c, int opcode, int col_offset, int end_col_offset, basicblock *target) { - assert(HAS_ARG(opcode)); + assert(HAS_ARG(opcode) || IS_VIRTUAL_OPCODE(opcode)); assert(target != NULL); if (compiler_use_new_implicit_block_if_needed(c) < 0) { From da4293169cb3ccb6dabfab9b25faea423d7eef4d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 1 Apr 2022 14:29:39 +0100 Subject: [PATCH 8/8] added MIN_VIRTUAL_OPCODE/MAX_ALLOWED_OPCODE etc --- Python/compile.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 0f23af8f825794..5bd30fcf942abb 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -78,6 +78,12 @@ #define POP_BLOCK -4 #define JUMP -5 +#define MIN_VIRTUAL_OPCODE -5 +#define MAX_ALLOWED_OPCODE 254 + +#define IS_WITHIN_OPCODE_RANGE(opcode) \ + ((opcode) >= MIN_VIRTUAL_OPCODE && (opcode) <= MAX_ALLOWED_OPCODE) + #define IS_VIRTUAL_OPCODE(opcode) ((opcode) < 0) #define IS_TOP_LEVEL_AWAIT(c) ( \ @@ -117,7 +123,7 @@ is_bit_set_in_table(const uint32_t *table, int bitindex) { * Word is indexed by (bitindex>>ln(size of int in bits)). * Bit within word is the low bits of bitindex. */ - if (bitindex >= 0) { + if (bitindex >= 0 && bitindex < 256) { uint32_t word = table[bitindex >> LOG_BITS_PER_INT]; return (word >> (bitindex & MASK_LOW_LOG_BITS)) & 1; } @@ -1192,6 +1198,7 @@ static int compiler_addop_line(struct compiler *c, int opcode, int line, int end_line, int col_offset, int end_col_offset) { + assert(IS_WITHIN_OPCODE_RANGE(opcode)); assert(!HAS_ARG(opcode) || IS_ARTIFICIAL(opcode)); if (compiler_use_new_implicit_block_if_needed(c) < 0) { @@ -1434,6 +1441,7 @@ compiler_addop_i_line(struct compiler *c, int opcode, Py_ssize_t oparg, The argument of a concrete bytecode instruction is limited to 8-bit. EXTENDED_ARG is used for 16, 24, and 32-bit arguments. */ + assert(IS_WITHIN_OPCODE_RANGE(opcode)); assert(HAS_ARG(opcode)); assert(0 <= oparg && oparg <= 2147483647); @@ -1477,6 +1485,7 @@ static int add_jump_to_block(struct compiler *c, int opcode, int col_offset, int end_col_offset, basicblock *target) { + assert(IS_WITHIN_OPCODE_RANGE(opcode)); assert(HAS_ARG(opcode) || IS_VIRTUAL_OPCODE(opcode)); assert(target != NULL);