Skip to content

Commit 0906d2a

Browse files
authored
gh-157468: Validate correct builtins used under JIT (GH-157766)
* Expose two possible builtin JIT gaps * Add trace and runtime guard for builtin dict * Check _GUARD_BUILTINS_IS_CANONICAL once per frame * Add check and fix for globals being folded
1 parent 1071f74 commit 0906d2a

11 files changed

Lines changed: 256 additions & 4 deletions

File tree

‎Include/internal/pycore_optimizer_types.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ typedef union {
140140

141141
typedef struct _Py_UOpsAbstractFrame {
142142
bool globals_watched;
143+
bool builtins_checked;
143144
// The version number of the globals dicts, once checked. 0 if unchecked.
144145
uint32_t globals_checked_version;
145146
// Max stacklen

‎Include/internal/pycore_uop_ids.h‎

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_uop_metadata.h‎

Lines changed: 21 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_capi/test_opt.py‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import builtins
12
import contextlib
23
import dis
34
import itertools
@@ -5276,6 +5277,119 @@ def jitted(funcs):
52765277
with self.assertRaises(NameError):
52775278
jitted([f, f_with_bad_globals])
52785279

5280+
def test_jitted_code_sees_changed_copied_builtins(self):
5281+
# Trace-time check. The traced function's builtins is a copy of the
5282+
# canonical dict with the same keys version, so a version check
5283+
# cannot tell them apart. The optimizer must see that func_builtins
5284+
# is not interp->builtins and keep _LOAD_GLOBAL_BUILTINS, which reads
5285+
# the frame's own dict, rather than fold a constant from the
5286+
# canonical one. No runtime guard is involved.
5287+
5288+
def f(n):
5289+
return [len("hello") for _ in range(n)]
5290+
5291+
copied_builtins = vars(builtins).copy()
5292+
f = types.FunctionType(f.__code__, {"__builtins__": copied_builtins})
5293+
5294+
f(TIER2_THRESHOLD)
5295+
ex = get_first_executor(f)
5296+
self.assertIsNotNone(ex)
5297+
# Not folded: the load must still consult the frame's builtins.
5298+
self.assertIn("_LOAD_GLOBAL_BUILTINS", get_opnames(ex))
5299+
5300+
# Replacing an existing value does not change the keys version.
5301+
copied_builtins["len"] = lambda s: 42
5302+
self.assertEqual(f(8), [42] * 8)
5303+
5304+
def test_jitted_code_sees_changed_copied_globals(self):
5305+
# Copying a dict must not carry over the keys version of the source.
5306+
# The optimizer folds a global to a constant guarded only by the
5307+
# globals keys version plus a watcher on the traced dict. A copy is
5308+
# not watched, and replacing an existing value does not change the
5309+
# keys version, so a function whose globals are a copy of the traced
5310+
# dict would otherwise pass _GUARD_GLOBALS_VERSION and see the stale
5311+
# constant.
5312+
def f(n):
5313+
for _ in range(n):
5314+
x = COPIED_GLOBAL
5315+
return x
5316+
5317+
for copy in (dict.copy, dict):
5318+
with self.subTest(copy=copy):
5319+
original = {"COPIED_GLOBAL": 1}
5320+
# A fresh code object, so that each subtest traces anew.
5321+
f_original = types.FunctionType(f.__code__.replace(), original)
5322+
self.assertEqual(f_original(TIER2_THRESHOLD), 1)
5323+
ex = get_first_executor(f_original)
5324+
self.assertIsNotNone(ex)
5325+
uops = get_opnames(ex)
5326+
self.assertIn("_GUARD_GLOBALS_VERSION", uops)
5327+
# The global was folded to a constant.
5328+
self.assertNotIn("_LOAD_GLOBAL_MODULE", uops)
5329+
5330+
copied = copy(original)
5331+
copied["COPIED_GLOBAL"] = 2
5332+
# Share the code object, so that the same executor is entered.
5333+
f_copied = types.FunctionType(f_original.__code__, copied)
5334+
self.assertEqual(f_copied(TIER2_THRESHOLD), 2)
5335+
self.assertEqual(f_original(TIER2_THRESHOLD), 1)
5336+
5337+
def test_jitted_code_sees_different_builtins(self):
5338+
# Runtime check. The traced function's builtins IS the canonical
5339+
# dict, so folding len to a constant is correct at trace time.
5340+
# A second function sharing the code object then enters the same
5341+
# executor with other builtins, so only the runtime guard on the
5342+
# executing frame's builtins can catch it.
5343+
def f(n):
5344+
return [len("hello") for _ in range(n)]
5345+
5346+
namespace = {"__builtins__": builtins}
5347+
f_canonical = types.FunctionType(f.__code__, namespace)
5348+
copied_builtins = vars(builtins).copy()
5349+
namespace["__builtins__"] = copied_builtins
5350+
f_copied = types.FunctionType(f.__code__, namespace)
5351+
5352+
5353+
f_canonical(TIER2_THRESHOLD)
5354+
ex = get_first_executor(f_canonical)
5355+
self.assertIsNotNone(ex)
5356+
self.assertIn("_GUARD_BUILTINS_IS_CANONICAL", get_opnames(ex))
5357+
5358+
copied_builtins["len"] = lambda s: 42
5359+
# The executor's owner still sees the canonical len.
5360+
self.assertEqual(f_canonical(8), [5] * 8)
5361+
# A different function enters the same executor with other builtins.
5362+
self.assertEqual(f_copied(8), [42] * 8)
5363+
5364+
def test_builtins_guard_emitted_once_per_frame(self):
5365+
# A frame's builtins cannot change once the frame is pushed, so
5366+
# repeated builtin loads in one frame share a single guard, just as
5367+
# they already share a single _GUARD_GLOBALS_VERSION.
5368+
5369+
def warmup(n):
5370+
x = 0
5371+
for _ in range(n):
5372+
x += len("ab")
5373+
return x
5374+
5375+
def one_frame(n):
5376+
x = 0
5377+
for _ in range(n):
5378+
x += len("ab") + abs(-1) + ord("c")
5379+
return x
5380+
5381+
# The optimizer context is reused for every compilation, so compile an
5382+
# unrelated trace first: state that is not reset per frame leaks here.
5383+
warmup(TIER2_THRESHOLD)
5384+
self.assertIsNotNone(get_first_executor(warmup))
5385+
5386+
_, ex = self._run_with_optimizer(one_frame, TIER2_THRESHOLD)
5387+
self.assertIsNotNone(ex)
5388+
uop_names = get_opnames(ex)
5389+
self.assertNotIn("_LOAD_GLOBAL_BUILTINS", uop_names) # all folded
5390+
self.assertEqual(uop_names.count("_GUARD_BUILTINS_IS_CANONICAL"), 1)
5391+
self.assertEqual(uop_names.count("_GUARD_GLOBALS_VERSION"), 1)
5392+
52795393
def test_reference_tracking_across_call_doesnt_crash(self):
52805394

52815395
def f1():
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fix the JIT optimizer folding a builtin name to a constant taken from the
2+
interpreter's builtins dictionary even when the running function uses a
3+
different ``__builtins__`` mapping, such as one created with
4+
``vars(builtins).copy()``. The optimizer now only folds when the function's
5+
builtins is the interpreter's, and the folded constant is guarded at runtime so
6+
that another function sharing the same code object but a different builtins
7+
mapping does not use it.

‎Objects/dictobject.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,10 @@ clone_combined_dict_keys(PyDictObject *orig)
10411041

10421042
memcpy(keys, orig->ma_keys, keys_size);
10431043

1044+
/* The keys version must be unique per keys object: the specializer
1045+
and the JIT optimizer rely on it to identify a dict's keys. */
1046+
keys->dk_version = 0;
1047+
10441048
/* After copying key/value pairs, we need to incref all
10451049
keys and values and they are about to be co-owned by a
10461050
new dict object. */

‎Python/bytecodes.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2356,6 +2356,10 @@ dummy_func(
23562356
STAT_INC(LOAD_GLOBAL, hit);
23572357
}
23582358

2359+
tier2 op(_GUARD_BUILTINS_IS_CANONICAL, (--)) {
2360+
DEOPT_IF(BUILTINS() != tstate->interp->builtins);
2361+
}
2362+
23592363
macro(LOAD_GLOBAL_MODULE) =
23602364
unused/1 + // Skip over the counter
23612365
NOP + // For guard insertion in the JIT optimizer

‎Python/executor_cases.c.h‎

Lines changed: 70 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/optimizer_bytecodes.c‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2531,13 +2531,24 @@ dummy_func(void) {
25312531
else if (interp->rare_events.builtin_dict >= _Py_MAX_ALLOWED_BUILTINS_MODIFICATIONS) {
25322532
/* Do nothing */
25332533
}
2534+
else if (ctx->frame->func == NULL ||
2535+
ctx->frame->func->func_builtins != builtins) {
2536+
/* Do nothing */
2537+
}
25342538
else {
25352539
if (!ctx->builtins_watched) {
25362540
PyDict_Watch(BUILTINS_WATCHER_ID, builtins);
25372541
ctx->builtins_watched = true;
25382542
}
2539-
if (ctx->frame->globals_checked_version != 0 && ctx->frame->globals_watched) {
2543+
if (ctx->frame->globals_checked_version != 0 &&
2544+
ctx->frame->globals_watched)
2545+
{
25402546
cnst = convert_global_to_const(this_instr, builtins);
2547+
if (cnst != NULL && !ctx->frame->builtins_checked) {
2548+
ctx->frame->builtins_checked = true;
2549+
ADD_OP(_GUARD_BUILTINS_IS_CANONICAL, 0, 0);
2550+
ADD_OP(this_instr->opcode, 0, (uintptr_t)cnst);
2551+
}
25412552
}
25422553
}
25432554
if (cnst == NULL) {

‎Python/optimizer_cases.c.h‎

Lines changed: 15 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)