-
Notifications
You must be signed in to change notification settings - Fork 19
jit: module-scope LOAD_NAME builtins fold; interpreter: shutdown module teardown #1187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5ec94c7
4631599
b20abbd
6def77d
ddd7bbb
0e837d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| bridges_compiled=0 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_blackhole_adopted_multi_frame=0 | ||
| fbw_blackhole_adopted_single_frame=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| fbw_store_journal_rollback_failed=0 | ||
| field_pos_attached_misplaced=0 | ||
| field_pos_spec_misplaced=0 | ||
| guard_failures=2 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 | ||
| retraces_compiled=0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| bridges_compiled=0 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_blackhole_adopted_multi_frame=0 | ||
| fbw_blackhole_adopted_single_frame=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| fbw_store_journal_rollback_failed=0 | ||
| field_pos_attached_misplaced=0 | ||
| field_pos_spec_misplaced=0 | ||
| guard_failures=2 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 | ||
| retraces_compiled=0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # pyre-check: max-pypy-ratio=8 | ||
| # A module-scope LOAD_NAME whose name misses the module dict resolves through | ||
| # the frame's builtin module. The builtins cell folds under the module dict's | ||
| # version? (so a later global binding shadows the builtin) and the builtins | ||
| # dict's own version?. The second loop proves that invalidation is seen. | ||
| # Output verified against CPython and PyPy. | ||
|
|
||
| N = 90000000 | ||
| M = 400000 | ||
| s = "xx" | ||
|
|
||
| total = 0 | ||
| for i in range(N): | ||
| total = total + len(s) | ||
| print(total) | ||
|
|
||
| len = lambda x: 100 | ||
| shadowed = 0 | ||
| for i in range(M): | ||
| shadowed = shadowed + len(s) | ||
| print(shadowed) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| bridges_compiled=0 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_blackhole_adopted_multi_frame=0 | ||
| fbw_blackhole_adopted_single_frame=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| fbw_store_journal_rollback_failed=0 | ||
| field_pos_attached_misplaced=0 | ||
| field_pos_spec_misplaced=0 | ||
| guard_failures=2 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 | ||
| retraces_compiled=0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -473,7 +473,10 @@ pub unsafe fn w_hmac_dealloc(obj: PyObjectRef) { | |
| mod hmac_class { | ||
| use super::*; | ||
|
|
||
| #[crate::pyre_methods] | ||
| // lib_pypy/_hashlib/__init__.py:232: `class HMAC(HASH)`. HMAC keeps its | ||
| // own native payload layout, but its Python type relationship is the same | ||
| // TypeDef inheritance PyPy exposes. | ||
| #[crate::pyre_methods(base = hash_state_class::type_object())] | ||
| impl W_Hmac { | ||
| #[staticmethod] | ||
| fn __new__( | ||
|
|
@@ -566,13 +569,23 @@ mod hmac_class { | |
| fn resolve_hmac_digestmod(digestmod: PyObjectRef) -> Result<&'static str, crate::PyError> { | ||
| let name_obj = if unsafe { is_str(digestmod) } { | ||
| digestmod | ||
| } else if unsafe { | ||
| pyre_object::py_type_check(digestmod, &crate::function::BUILTIN_FUNCTION_TYPE) | ||
| } { | ||
| crate::baseobjspace::getattr_str(digestmod, "__name__")? | ||
| } else { | ||
| return Err(unsupported_digestmod("unsupported hash type")); | ||
| match crate::baseobjspace::getattr_str(digestmod, "__name__") { | ||
| Ok(name) => name, | ||
| // PyPy's structural rule is to accept every object that exposes | ||
| // `__name__` (lib_pypy/_hashlib/__init__.py:547-554). CPython | ||
| // 3.14 additionally normalizes a missing name to the module's | ||
| // public UnsupportedDigestmodError; preserve both without | ||
| // restricting the accepted object type again. | ||
| Err(err) if err.kind == crate::PyErrorKind::AttributeError => { | ||
| return Err(unsupported_digestmod("unsupported hash type")); | ||
| } | ||
| Err(err) => return Err(err), | ||
| } | ||
| }; | ||
| if !unsafe { is_str(name_obj) } { | ||
| return Err(unsupported_digestmod("unsupported hash type")); | ||
| } | ||
| let name = unsafe { w_str_get_wtf8(name_obj) }; | ||
| let bytes = name.as_bytes(); | ||
| let bytes = bytes.strip_prefix(b"openssl_").unwrap_or(bytes); | ||
|
|
@@ -671,11 +684,8 @@ fn pbkdf2_hmac(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | |
|
|
||
| check_digest_name(hash_name)?; | ||
| let requested = unsafe { w_str_get_wtf8(hash_name) }; | ||
| let name = lookup_digest_name(requested.as_bytes()).ok_or_else(|| { | ||
| crate::PyError::value_error(format!( | ||
| "[digital envelope routines] unsupported: {requested}" | ||
| )) | ||
| })?; | ||
| let name = lookup_digest_name(requested.as_bytes()) | ||
| .ok_or_else(|| unsupported_digestmod("unsupported hash type"))?; | ||
| let password = read_hash_buffer(password)?; | ||
| let salt = read_hash_buffer(salt)?; | ||
| let iterations = crate::baseobjspace::int_w(crate::baseobjspace::space_index(iterations)?)?; | ||
|
|
@@ -694,7 +704,7 @@ fn pbkdf2_hmac(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | |
| _ => pyre_native::hash::digest_output_size(name).unwrap_or(0), | ||
| }; | ||
| let result = pyre_native::hash::compute_pbkdf2_hmac(name, &password, &salt, iterations, dklen) | ||
| .ok_or_else(|| crate::PyError::value_error("unsupported hash type"))?; | ||
| .ok_or_else(|| unsupported_digestmod("unsupported hash type"))?; | ||
| Ok(w_bytes_from_bytes(&result)) | ||
| } | ||
|
|
||
|
|
@@ -788,8 +798,11 @@ fn scrypt_kdf(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | |
| } | ||
|
|
||
| // RFC 7914's dominant allocation is V[N] with 128*r-byte entries. OpenSSL | ||
| // also needs B[p] and a working block. Honor an explicit caller limit; | ||
| // maxmem=0 retains OpenSSL's implementation-defined default behavior. | ||
| // also needs B[p] and a working block. PyPy passes maxmem=0 through to | ||
| // EVP_PBE_scrypt (lib_pypy/_hashlib/__init__.py:430-433), where OpenSSL | ||
| // applies its private 32 MiB default. The Rust backend has no such layer, | ||
| // so spell out that same default here rather than treating zero as | ||
| // unlimited memory. | ||
| let memory = usize::try_from(n) | ||
| .ok() | ||
| .and_then(|n| n.checked_mul(r as usize)) | ||
|
|
@@ -799,9 +812,15 @@ fn scrypt_kdf(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | |
| .ok_or_else(|| { | ||
| crate::PyError::value_error("Invalid parameter combination for n, r, p, maxmem") | ||
| })?; | ||
| if maxmem != 0 && memory > maxmem { | ||
| const OPENSSL_DEFAULT_SCRYPT_MAXMEM: usize = 32 * 1024 * 1024; | ||
| let effective_maxmem = if maxmem == 0 { | ||
| OPENSSL_DEFAULT_SCRYPT_MAXMEM | ||
| } else { | ||
| maxmem | ||
| }; | ||
| if memory > effective_maxmem { | ||
| return Err(crate::PyError::value_error( | ||
| "Invalid parameter combination for n, r, p, maxmem", | ||
| "[digital envelope routines] memory limit exceeded", | ||
| )); | ||
| } | ||
| let output = pyre_native::hash::compute_scrypt(&password, &salt, log_n, r, p, dklen) | ||
|
|
@@ -834,6 +853,26 @@ fn blake2_new(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | |
| let key = read_hash_buffer(arg(3))?; | ||
| let salt = read_hash_buffer(arg(4))?; | ||
| let person = read_hash_buffer(arg(5))?; | ||
| let (max_key_size, salt_size, person_size) = match name { | ||
| "blake2b" => (64, 16, 16), | ||
| "blake2s" => (32, 8, 8), | ||
| _ => unreachable!(), | ||
| }; | ||
| if key.len() > max_key_size { | ||
| return Err(crate::PyError::value_error(format!( | ||
| "maximum key length is {max_key_size} bytes" | ||
| ))); | ||
| } | ||
| if salt.len() > salt_size { | ||
| return Err(crate::PyError::value_error(format!( | ||
| "maximum salt length is {salt_size} bytes" | ||
| ))); | ||
| } | ||
| if person.len() > person_size { | ||
| return Err(crate::PyError::value_error(format!( | ||
| "maximum person length is {person_size} bytes" | ||
| ))); | ||
|
Comment on lines
+866
to
+874
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Moving these size checks into AGENTS.md reference: AGENTS.md:L231-L233 Useful? React with 👍 / 👎. |
||
| } | ||
|
Comment on lines
+856
to
+875
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- target context ---'
sed -n '800,890p' pyre/pyre-interpreter/src/module/_hashlib/mod.rs
printf '%s\n' '--- read_hash_buffer definitions and calls ---'
rg -n -C 5 'read_hash_buffer' pyre
printf '%s\n' '--- BLAKE2 constructor call sites ---'
rg -n -C 4 '_blake2_new|blake2b|blake2s' pyre/pyre-interpreter/src/module/_hashlibRepository: youknowone/pyre Length of output: 22416 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- read_hash_buffer implementation ---'
sed -n '955,1010p' pyre/pyre-interpreter/src/module/_hashlib/mod.rs
printf '%s\n' '--- buffer protocol implementation references ---'
rg -n -C 5 'PyBUF_SIMPLE|buffer.*view|BufferView|view.*len|as_bytes|to_vec|Vec::from|copy_from_slice' pyre/pyre-interpreter/src pyre/pyre-object/src | head -n 240
printf '%s\n' '--- BLAKE2 app-level validation ---'
sed -n '1,180p' pyre/pyre-interpreter/src/module/_hashlib/_hashlib_app.pyRepository: youknowone/pyre Length of output: 21510 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- simple_buffer_bytes definition ---'
rg -n -C 10 'fn simple_buffer_bytes|simple_buffer_bytes' pyre
printf '%s\n' '--- buffer view types and byte conversion ---'
rg -n -C 8 'struct BufferView|impl BufferView|fn as_bytes|pub fn as_bytes|Buffer::|getbuffer|buffer.*bytes' pyre/pyre-object/src pyre/pyre-interpreter/src | head -n 320
printf '%s\n' '--- large-buffer constructors and allocation paths ---'
rg -n -C 5 'bytearray|w_bytearray|memoryview|Buffer::Owned|Vec<u8>|with_capacity' pyre/pyre-object/src | head -n 320Repository: youknowone/pyre Length of output: 50372 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- buffer acquisition implementation ---'
sed -n '8460,8615p' pyre/pyre-interpreter/src/baseobjspace.rs
printf '%s\n' '--- SimpleBufferBytes definition ---'
rg -n -C 12 'struct SimpleBufferBytes|impl SimpleBufferBytes|enum BufferRequest' pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre Length of output: 8542 🏁 Script executed: #!/bin/bash
set -e
sed -n '8613,8755p' pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre Length of output: 5373 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- _blake2 bindings and app-level callers ---'
rg -n -C 8 '_blake2_new|_blake2|blake2_new' pyre --glob '*.rs' --glob '*.py'
printf '%s\n' '--- buffer gather implementation ---'
rg -n -C 8 'fn memoryview_gather_bytes|memoryview_gather_bytes' pyre
printf '%s\n' '--- BLAKE2 tests and size limits ---'
rg -n -C 6 'maximum key length|maximum salt length|maximum person length|blake2b.*key|blake2s.*key|memoryview' pyre --glob '*test*' --glob '*.py' --glob '*.rs' | head -n 260Repository: youknowone/pyre Length of output: 50371 🏁 Script executed: #!/bin/bash
set -e
sed -n '35,175p' pyre/pyre-interpreter/src/module/_blake2/_blake2_app.pyRepository: youknowone/pyre Length of output: 5849 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
hashlib = Path("pyre/pyre-interpreter/src/module/_hashlib/mod.rs").read_text()
base = Path("pyre/pyre-interpreter/src/baseobjspace.rs").read_text()
builtins = Path("pyre/pyre-interpreter/src/builtins.rs").read_text()
def body(source, signature, end_marker):
start = source.index(signature)
end = source.index(end_marker, start)
return source[start:end]
blake = body(hashlib, "fn blake2_new(", "/// Keyword names")
reader = body(hashlib, "fn read_hash_buffer(", "/// Keyword names")
buffer = body(base, "fn buffer_bytes(", "/// Look up a descriptor")
gather = body(builtins, "pub(crate) unsafe fn memoryview_gather_bytes", "/// `PyBuffer_ToContiguous")
calls = [blake.index(f"read_hash_buffer(arg({i}))?") for i in (3, 4, 5)]
checks = [blake.index(token) for token in ("if key.len()", "if salt.len()", "if person.len()")]
assert calls == sorted(calls), "BLAKE2 parameter buffers are not read in source order"
assert max(calls) < min(checks), "A parameter buffer is checked before materialization"
assert "buf.as_bytes().to_vec()" in reader, "read_hash_buffer no longer copies the acquired view"
assert "memoryview_gather_bytes" in buffer, "buffer_bytes does not gather memoryview contents"
assert "-> Vec<u8>" in gather, "memoryview_gather_bytes is not Vec-returning"
print("read_hash_buffer copies acquired buffer contents")
print("buffer_bytes gathers memoryview contents into Vec<u8>")
print("all BLAKE2 key/salt/person buffers are materialized before size checks")
PYRepository: youknowone/pyre Length of output: 294 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
hashlib = Path("pyre/pyre-interpreter/src/module/_hashlib/mod.rs").read_text()
base = Path("pyre/pyre-interpreter/src/baseobjspace.rs").read_text()
builtins = Path("pyre/pyre-interpreter/src/builtins.rs").read_text()
def after(source, start_text, end_text):
start = source.index(start_text)
end = source.index(end_text, start)
return source[start:end]
blake = after(hashlib, "fn blake2_new(", "const NEW_KEYWORDS")
reader = after(hashlib, "fn read_hash_buffer(", "const NEW_KEYWORDS")
buffer = after(base, "fn buffer_bytes(", "/// Look up a descriptor")
gather = after(
builtins,
"pub(crate) unsafe fn memoryview_gather_bytes",
"/// `PyBuffer_ToContiguous",
)
calls = [blake.index(f"read_hash_buffer(arg({i}))?") for i in (3, 4, 5)]
checks = [blake.index(token) for token in ("if key.len()", "if salt.len()", "if person.len()")]
assert calls == sorted(calls)
assert max(calls) < min(checks)
assert "buf.as_bytes().to_vec()" in reader
assert "memoryview_gather_bytes" in buffer
assert "-> Vec<u8>" in gather
print("read_hash_buffer copies acquired buffer contents")
print("buffer_bytes gathers memoryview contents into Vec<u8>")
print("all BLAKE2 key/salt/person buffers are materialized before size checks")
PYRepository: youknowone/pyre Length of output: 294 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
hashlib = Path("pyre/pyre-interpreter/src/module/_hashlib/mod.rs").read_text()
base = Path("pyre/pyre-interpreter/src/baseobjspace.rs").read_text()
builtins = Path("pyre/pyre-interpreter/src/builtins.rs").read_text()
start = hashlib.index("fn blake2_new(")
param_reads = [
hashlib.index(f"read_hash_buffer(arg({i}))?", start)
for i in (3, 4, 5)
]
checks = [
hashlib.index(token, start)
for token in ("if key.len()", "if salt.len()", "if person.len()")
]
assert param_reads == sorted(param_reads)
assert max(param_reads) < min(checks)
assert "buf.as_bytes().to_vec()" in hashlib
assert "memoryview_gather_bytes" in base
assert "pub(crate) unsafe fn memoryview_gather_bytes" in builtins
assert "-> Vec<u8>" in builtins[builtins.index("pub(crate) unsafe fn memoryview_gather_bytes"):]
print("read_hash_buffer copies acquired buffer contents")
print("buffer_bytes gathers memoryview contents into Vec<u8>")
print("all BLAKE2 key/salt/person buffers are materialized before size checks")
PYRepository: youknowone/pyre Length of output: 327 Reject oversized BLAKE2 parameter buffers before copying them. Use a length-only buffer view for 🤖 Prompt for AI AgentsSource: MCP tools |
||
| let index = | ||
| |position| crate::baseobjspace::int_w(crate::baseobjspace::space_index(arg(position))?); | ||
| let digest_size = usize::try_from(index(2)?) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the benchmark pass Ruff.
Ruff reports unused loop variables, builtin shadowing, lambda assignment, and an unused lambda argument. Keep the module-scope
lenbinding for invalidation coverage, but use a named function and an explicit A001 suppression.Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 13-13: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
[error] 17-17: Variable
lenis shadowing a Python builtin(A001)
[error] 17-17: Do not assign a
lambdaexpression, use adefRewrite
lenas adef(E731)
[warning] 17-17: Unused lambda argument:
x(ARG005)
[warning] 19-19: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
🤖 Prompt for AI Agents
Source: Linters/SAST tools