Skip to content

Tools/jit has several bytes and bytearray mixups #129805

@sobolevn

Description

@sobolevn

Bug report

Stensil defines body as bytearray

@dataclasses.dataclass
class Stencil:
"""
A contiguous block of machine code or data to be copied-and-patched.
Analogous to a section or segment in an object file.
"""
body: bytearray = dataclasses.field(default_factory=bytearray, init=False)

But, it is passed to functions that expect bytes, this now works for historic reasons. But, since [email protected] or [email protected] with --strict-bytes turned on - it won't. See https://github.com/python/mypy/blob/master/CHANGELOG.md#--strict-bytes

Examples:

diff --git Tools/jit/_stencils.py Tools/jit/_stencils.py
index ee761a73fa8..8b6957f8bdb 100644
--- Tools/jit/_stencils.py
+++ Tools/jit/_stencils.py
@@ -141,7 +141,11 @@ class Hole:
     def __post_init__(self) -> None:
         self.func = _PATCH_FUNCS[self.kind]
 
-    def fold(self, other: typing.Self, body: bytes) -> typing.Self | None:
+    def fold(
+        self,
+        other: typing.Self,
+        body: bytes | bytearray,
+    ) -> typing.Self | None:
         """Combine two holes into a single hole, if possible."""
         instruction_a = int.from_bytes(
             body[self.offset : self.offset + 4], byteorder=sys.byteorder
diff --git Tools/jit/_targets.py Tools/jit/_targets.py
index d23ced19842..4015fc564ad 100644
--- Tools/jit/_targets.py
+++ Tools/jit/_targets.py
@@ -97,7 +97,7 @@ def _handle_section(self, section: _S, group: _stencils.StencilGroup) -> None:
         raise NotImplementedError(type(self))
 
     def _handle_relocation(
-        self, base: int, relocation: _R, raw: bytes
+        self, base: int, relocation: _R, raw: bytes | bytearray
     ) -> _stencils.Hole:
         raise NotImplementedError(type(self))
 
@@ -257,7 +257,7 @@ def _unwrap_dllimport(self, name: str) -> tuple[_stencils.HoleValue, str | None]
         return _stencils.symbol_to_value(name)
 
     def _handle_relocation(
-        self, base: int, relocation: _schema.COFFRelocation, raw: bytes
+        self, base: int, relocation: _schema.COFFRelocation, raw: bytes | bytearray
     ) -> _stencils.Hole:
         match relocation:
             case {
@@ -348,7 +348,10 @@ def _handle_section(
             }, section_type
 
     def _handle_relocation(
-        self, base: int, relocation: _schema.ELFRelocation, raw: bytes
+        self,
+        base: int,
+        relocation: _schema.ELFRelocation,
+        raw: bytes | bytearray,
     ) -> _stencils.Hole:
         symbol: str | None
         match relocation:
@@ -424,7 +427,10 @@ def _handle_section(
             stencil.holes.append(hole)
 
     def _handle_relocation(
-        self, base: int, relocation: _schema.MachORelocation, raw: bytes
+        self,
+        base: int,
+        relocation: _schema.MachORelocation,
+        raw: bytes | bytearray,
     ) -> _stencils.Hole:
         symbol: str | None
         match relocation:

I propose to proactively fix this by using bytes | bytearray. Why? Because this union won't allow to mutate byte objects. Why not collections.abc.Buffer? jit requires python3.11+, and Buffer is available since 3.12, we also don't want to add typing_extensions package as a single dependency. We also don't want to do some cool TYPE_CHECKING hacks, when we can just use a union.

Linked PRs

Metadata

Metadata

Assignees

Labels

topic-JITtype-bugAn unexpected behavior, bug, or error

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions