Skip to content

Handle oversized __hash__ results without panicking#6561

Merged
youknowone merged 5 commits intomainfrom
copilot/fix-hash-wrapper-panic
Dec 28, 2025
Merged

Handle oversized __hash__ results without panicking#6561
youknowone merged 5 commits intomainfrom
copilot/fix-hash-wrapper-panic

Conversation

Copy link
Contributor

Copilot AI commented Dec 28, 2025 •
edited by coderabbitai bot
Loading

Calling hash() on an object whose __hash__ returns a very large integer could panic in the hash wrapper when the value didn’t fit into i64.

  • Hash normalization: Leave in-range integer hash results unchanged; when overflow occurs, normalize via the shared hash_bigint helper so arbitrarily large __hash__ results are safely reduced to a valid PyHash instead of panicking.
  • Regression coverage: Add a snippet test ensuring a class returning 1 << 63 from __hash__ hashes to the expected value.

Example:

class Evil:
    def __hash__(self):
        return 1 << 63

assert hash(Evil()) == 4
Original prompt

This section details on the original issue you should resolve

<issue_title>Panic in hash wrapping when user hash returns oversized int</issue_title>
<issue_description>### What happened?

Calling hash() on an object whose __hash__ returns an oversized integer (e.g., 1 << 63) causes a panic in hash_wrapper. The wrapper attempts to convert the BigInt to i64, then falls back to % u64::MAX, but that remainder can still be outside i64 and leads to unwrap() on None instead of raising a Python exception or normalizing safely.

Proof of Concept:

class Evil:
    def __hash__(self):
        return 1 << 63

hash(Evil())
Affected Versions
RustPython Version Status Exit Code
Python 3.13.0alpha (heads/main-dirty:21300f689, Dec 13 2025, 22:16:49) [RustPython 0.4.0 with rustc 1.90.0-nightly (11ad40bb8 2025-06-28)] Panic 1
Vulnerable Code 
fn hash_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyHash> {
    let hash_obj = vm.call_special_method(zelf, identifier!(vm, __hash__), ())?;
    let py_int = hash_obj
        .downcast_ref::<PyInt>()
        .ok_or_else(|| vm.new_type_error("__hash__ method should return an integer"))?;
    let big_int = py_int.as_bigint();
    let hash: PyHash = big_int
        .to_i64()
        .unwrap_or_else(|| (big_int % BigInt::from(u64::MAX)).to_i64().unwrap()); // Bug: u64::MAX should be i64::Max
    Ok(hash)
}
Rust Output 
thread 'main' panicked at crates/vm/src/types/slot.rs:369:72:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/11ad40bb839ca16f74784b4ab72596ad85587298/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/11ad40bb839ca16f74784b4ab72596ad85587298/library/core/src/panicking.rs:75:14
   2: core::panicking::panic
             at /rustc/11ad40bb839ca16f74784b4ab72596ad85587298/library/core/src/panicking.rs:145:5
   3: core::option::unwrap_failed
             at /rustc/11ad40bb839ca16f74784b4ab72596ad85587298/library/core/src/option.rs:2072:5
   4: core::option::Option<T>::unwrap
             at /home/jackfromeast/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1005:21
   5: rustpython_vm::types::slot::hash_wrapper::{{closure}}
             at ./crates/vm/src/types/slot.rs:369:72
   6: core::option::Option<T>::unwrap_or_else
             at /home/jackfromeast/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1050:21
   7: rustpython_vm::types::slot::hash_wrapper
             at ./crates/vm/src/types/slot.rs:369:10
   8: rustpython_vm::protocol::object::<impl rustpython_vm::object::core::PyObject>::hash
             at ./crates/vm/src/protocol/object.rs:663:20
   9: rustpython_vm::stdlib::builtins::builtins::hash
             at ./crates/vm/src/stdlib/builtins.rs:440:13
  10: core::ops::function::Fn::call
             at /home/jackfromeast/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:79:5
  11: rustpython_vm::function::builtin::<impl rustpython_vm::function::builtin::sealed::PyNativeFnInternal<(rustpython_vm::function::builtin::OwnedParam<T1>,),R,rustpython_vm::vm::VirtualMachine> for F>::call_
             at ./crates/vm/src/function/builtin.rs:126:17
  12: <F as rustpython_vm::function::builtin::IntoPyNativeFn<(T,R,VM)>>::call
             at ./crates/vm/src/function/builtin.rs:92:14
  13: rustpython_vm::function::builtin::into_func::{{closure}}
             at ./crates/vm/src/function/builtin.rs:50:40
  14: <rustpython_vm::builtins::builtin_func::PyNativeFunction as rustpython_vm::types::slot::Callable>::call
             at ./crates/vm/src/builtins/builtin_func.rs:73:9
  15: rustpython_vm::types::slot::Callable::slot_call
             at ./crates/vm/src/types/slot.rs:1028:9
  16: rustpython_vm::protocol::callable::PyCallable::invoke
             at ./crates/vm/src/protocol/callable.rs:52:22
  17: rustpython_vm::protocol::callable::<impl rustpython_vm::object::core::PyObject>::call_with_args
             at ./crates/vm/src/protocol/callable.rs:33:18
  18: rustpython_vm::protocol::callable::<impl rustpython_vm::object::core::PyObject>::call
             at ./crates/vm/src/protocol/callable.rs:22:14
  19: rustpython_vm::frame::ExecutingFrame::execute_call
             at ./crates/vm/src/frame.rs:1880:30
  20: rustpython_vm::frame::ExecutingFrame::execute_instruction
             at ./crates/vm/src/frame.rs:667:22
  21: rustpython_vm::frame::ExecutingFrame::run
             at ./crates/vm/src/frame.rs:372:31
  22: rustpython_vm::frame::<impl rustpython_vm::object::core::Py<rustpython_vm::frame::Frame>>::run::{{closure}}
             at ./crates/vm/src/frame.r...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes RustPython/RustPython#6550

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Tests**
  * Added comprehensive test cases for hash function behavior, validating edge cases and handling of built-in types.

* **Refactor**
  * Optimized internal hash computation logic.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 28, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

This PR fixes a panic in the hash wrapping logic when __hash__ returns an oversized integer. It replaces the unsafe modulo-based fallback with proper sentinel-based normalization using utilities from the common hash module, and adds test coverage for oversized hash values and unhashable built-in types.

Changes

Cohort / File(s) Summary
Hash computation fix
crates/vm/src/types/slot.rs		 Updated hash_wrapper to use fix_sentinel and hash_bigint utilities instead of manual fallback modulo on i64 conversion failure; adjusted imports and removed explicit PyHash type annotation.
Hash validation tests
extra_tests/snippets/builtin_hash.py		 Added test class Evil returning oversized hash (1 << 63), assertion that normalization produces correct result (4), and TypeError checks for unhashable mutable built-ins (dict, set, list).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • #6480: Modifies hash and __hash__ slot handling in the same crates/vm/src/types/slot.rs file, affecting hashing infrastructure.

Poem

🐰 A hash that was oversized and mean,
Made panics where safety should peen,
With sentinel fix,
And bigint tricks,
Now Evil returns four, crisp and clean! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: preventing panics when hash() is called with oversized hash results.
Linked Issues check ✅ Passed The PR directly addresses issue #6550 by fixing hash_wrapper to safely normalize oversized hash results using hash_bigint instead of panicking.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the hash_wrapper panic issue: imports, hash computation logic, and test cases for the fix.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch copilot/fix-hash-wrapper-panic
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee74713 and a14d5c1.

📒 Files selected for processing (2)
  • crates/vm/src/types/slot.rs
  • extra_tests/snippets/builtin_hash.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/vm/src/types/slot.rs
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.py: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Follow PEP 8 style for custom Python code
Use ruff for linting Python code

Files:

  • extra_tests/snippets/builtin_hash.py
🧬 Code graph analysis (2)
crates/vm/src/types/slot.rs (1)
crates/common/src/hash.rs (2)
  • fix_sentinel (149-151)
  • hash_bigint (132-141)
extra_tests/snippets/builtin_hash.py (1)
crates/vm/src/types/slot.rs (1)
  • hash (1489-1489)
🔇 Additional comments (3)
crates/vm/src/types/slot.rs (2)

8-8: LGTM: Proper imports for sentinel handling and bigint hashing.

The addition of fix_sentinel and hash_bigint imports provides the necessary utilities to safely normalize oversized hash values.

414-417: Excellent fix for the overflow panic.

The new logic properly handles oversized __hash__ results:

  1. Values fitting in i64 are converted and passed through fix_sentinel to handle the -1 sentinel
  2. Overflow cases use hash_bigint, which safely normalizes arbitrarily large integers via modulo arithmetic

This replaces the unsafe (big_int % u64::MAX).to_i64().unwrap() pattern that could still panic when the modulo result didn't fit in i64.

extra_tests/snippets/builtin_hash.py (1)

16-21: The test correctly exercises the hash overflow case.

The expected value of 4 matches CPython's behavior when __hash__ returns 1 << 63.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone
Fix hash wrapper overflow handling
fa233e2 
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix panic in hash wrapping for oversized integers Handle oversized __hash__ results without panicking Dec 28, 2025
Copilot AI requested a review from youknowone December 28, 2025 05:44
@github-actions
Copy link
Contributor

github-actions bot commented Dec 28, 2025

Code has been automatically formatted

The code in this PR has been formatted using cargo fmt --all.
Please pull the latest changes before pushing again:

git pull origin copilot/fix-hash-wrapper-panic

@youknowone
Copy link
Member

youknowone commented Dec 28, 2025

@copilot Investigate the proper behavior of CPython and ensure test_dataclasses work

@youknowone
Adjust __hash__ wrapper conversion
7fb03b2 
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 28, 2025

@copilot Investigate the proper behavior of CPython and ensure test_dataclasses work

Adjusted the hash wrapper to leave in-range integers unchanged and only normalize on overflow, matching CPython’s conversion; test_dataclasses now passes (commit 7fb03b2).

@youknowone youknowone marked this pull request as ready for review December 28, 2025 07:21
@youknowone
Copy link
Member

youknowone commented Dec 28, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 28, 2025

@youknowone: I'll review the changes in this PR. I'll pay special attention to:

  1. CPython behavior compatibility: Verifying that the hash normalization behavior matches CPython's handling of oversized __hash__ results
  2. test_dataclasses: Ensuring that the dataclasses tests work correctly with the new hash handling

I'll analyze the implementation to ensure that:

  • Oversized hash values are properly normalized using hash_bigint
  • The test coverage adequately validates the fix
  • No regressions are introduced in existing functionality
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@youknowone youknowone merged commit 7b36c9e into main Dec 28, 2025
13 checks passed
@youknowone youknowone deleted the copilot/fix-hash-wrapper-panic branch December 28, 2025 07:27
@youknowone youknowone mentioned this pull request Dec 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@youknowone youknowone youknowone approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@youknowone youknowone

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@youknowone