Skip to content

Commit b88d271

Browse files
maurycymiss-islington
authored andcommitted
gh-151292: Change total_samples:u32 in the binary format to total_samples:u64 (GH-153425)
(cherry picked from commit 1d19650) Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
1 parent 6404803 commit b88d271

6 files changed

Lines changed: 49 additions & 24 deletions

File tree

Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -999,9 +999,9 @@ class TestBinaryFormatValidation(BinaryFormatTestBase):
999999
"""Tests for malformed binary files."""
10001000

10011001
HDR_OFF_SAMPLES = 28
1002-
HDR_OFF_THREADS = 32
1003-
HDR_OFF_STR_TABLE = 36
1004-
HDR_OFF_FRAME_TABLE = 44
1002+
HDR_OFF_THREADS = 36
1003+
HDR_OFF_STR_TABLE = 40
1004+
HDR_OFF_FRAME_TABLE = 48
10051005
FILE_HEADER_PLACEHOLDER_SIZE = 64
10061006
FILE_FOOTER_SIZE = 32
10071007
FTR_OFF_STRINGS = 0
@@ -1039,7 +1039,7 @@ def test_replay_rejects_sample_count_mismatch(self):
10391039

10401040
with open(filename, "r+b") as raw:
10411041
raw.seek(self.HDR_OFF_SAMPLES)
1042-
raw.write(struct.pack("=I", 2))
1042+
raw.write(struct.pack("=Q", 2))
10431043

10441044
with BinaryReader(filename) as reader:
10451045
self.assertEqual(reader.get_info()["sample_count"], 2)
@@ -1149,6 +1149,31 @@ def test_open_accepts_frame_count_at_capacity_boundary(self):
11491149
f"possible {max_frames}",
11501150
)
11511151

1152+
def test_sample_count_reads_full_64_bits(self):
1153+
"""sample_count values requiring the upper 32 bits decode correctly."""
1154+
filename = self.create_binary_file([], compression="none")
1155+
big_count = 0x1_0002_0003
1156+
1157+
with open(filename, "r+b") as raw:
1158+
raw.seek(self.HDR_OFF_SAMPLES)
1159+
raw.write(struct.pack("=Q", big_count))
1160+
1161+
with BinaryReader(filename) as reader:
1162+
self.assertEqual(reader.get_info()["sample_count"], big_count)
1163+
1164+
def test_sample_count_boundary_values(self):
1165+
"""Values above the old u32 ceiling decode fine."""
1166+
filename = self.create_binary_file([], compression="none")
1167+
1168+
for value in (0xFFFFFFFF - 1, 0xFFFFFFFF, 0xFFFFFFFF + 1):
1169+
with self.subTest(value=value):
1170+
with open(filename, "r+b") as raw:
1171+
raw.seek(self.HDR_OFF_SAMPLES)
1172+
raw.write(struct.pack("=Q", value))
1173+
1174+
with BinaryReader(filename) as reader:
1175+
self.assertEqual(reader.get_info()["sample_count"], value)
1176+
11521177

11531178
class TestBinaryEncodings(BinaryFormatTestBase):
11541179
"""Tests specifically targeting different stack encodings."""
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Store the sample count in :mod:`profiling.sampling` binary format, as a
2+
64-bit integer, widening from previously used 32-bit integer. This breaks
3+
the existing recordings. Long or thread-heavy profiling sessions will no
4+
longer fail because of :exc:`OverflowError`. Patch by Maurycy
5+
Pawłowski-Wieroński.

Modules/_remote_debugging/binary_io.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ extern "C" {
5050
#define HDR_OFF_INTERVAL (HDR_OFF_START_TIME + HDR_SIZE_START_TIME)
5151
#define HDR_SIZE_INTERVAL 8
5252
#define HDR_OFF_SAMPLES (HDR_OFF_INTERVAL + HDR_SIZE_INTERVAL)
53-
#define HDR_SIZE_SAMPLES 4
53+
#define HDR_SIZE_SAMPLES 8
5454
#define HDR_OFF_THREADS (HDR_OFF_SAMPLES + HDR_SIZE_SAMPLES)
5555
#define HDR_SIZE_THREADS 4
5656
#define HDR_OFF_STR_TABLE (HDR_OFF_THREADS + HDR_SIZE_THREADS)
@@ -260,7 +260,7 @@ typedef struct {
260260
/* Metadata */
261261
uint64_t start_time_us;
262262
uint64_t sample_interval_us;
263-
uint32_t total_samples;
263+
uint64_t total_samples;
264264

265265
/* String hash table: PyObject* -> uint32_t index */
266266
_Py_hashtable_t *string_hash;
@@ -326,7 +326,7 @@ typedef struct {
326326
int needs_swap; /* Non-zero if file was written on different-endian system */
327327
uint64_t start_time_us;
328328
uint64_t sample_interval_us;
329-
uint32_t sample_count;
329+
uint64_t sample_count;
330330
uint32_t thread_count;
331331
uint64_t string_table_offset;
332332
uint64_t frame_table_offset;

Modules/_remote_debugging/binary_io_reader.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ reader_parse_header(BinaryReader *reader, const uint8_t *data, size_t file_size)
8383

8484
/* Read header fields with byte-swapping if needed */
8585
uint64_t start_time_us, sample_interval_us, string_table_offset, frame_table_offset;
86-
uint32_t sample_count, thread_count, compression_type;
86+
uint64_t sample_count;
87+
uint32_t thread_count, compression_type;
8788

8889
memcpy(&start_time_us, &data[HDR_OFF_START_TIME], HDR_SIZE_START_TIME);
8990
memcpy(&sample_interval_us, &data[HDR_OFF_INTERVAL], HDR_SIZE_INTERVAL);
@@ -95,7 +96,7 @@ reader_parse_header(BinaryReader *reader, const uint8_t *data, size_t file_size)
9596

9697
reader->start_time_us = SWAP64_IF(reader->needs_swap, start_time_us);
9798
reader->sample_interval_us = SWAP64_IF(reader->needs_swap, sample_interval_us);
98-
reader->sample_count = SWAP32_IF(reader->needs_swap, sample_count);
99+
reader->sample_count = SWAP64_IF(reader->needs_swap, sample_count);
99100
reader->thread_count = SWAP32_IF(reader->needs_swap, thread_count);
100101
reader->string_table_offset = SWAP64_IF(reader->needs_swap, string_table_offset);
101102
reader->frame_table_offset = SWAP64_IF(reader->needs_swap, frame_table_offset);
@@ -993,10 +994,10 @@ emit_batch(RemoteDebuggingState *state, PyObject *collector,
993994

994995
/* Helper to invoke progress callback, returns -1 on error */
995996
static inline int
996-
invoke_progress_callback(PyObject *callback, Py_ssize_t current, uint32_t total)
997+
invoke_progress_callback(PyObject *callback, Py_ssize_t current, uint64_t total)
997998
{
998999
if (callback && callback != Py_None) {
999-
PyObject *result = PyObject_CallFunction(callback, "nI", current, total);
1000+
PyObject *result = PyObject_CallFunction(callback, "nK", current, (unsigned long long)total);
10001001
if (result) {
10011002
Py_DECREF(result);
10021003
} else {
@@ -1248,8 +1249,8 @@ binary_reader_replay(BinaryReader *reader, PyObject *collector, PyObject *progre
12481249

12491250
if ((uint64_t)replayed != reader->sample_count) {
12501251
PyErr_Format(PyExc_ValueError,
1251-
"Sample count mismatch: header declares %u samples but replay decoded %zd",
1252-
reader->sample_count, replayed);
1252+
"Sample count mismatch: header declares %llu samples but replay decoded %zd",
1253+
(unsigned long long)reader->sample_count, replayed);
12531254
return -1;
12541255
}
12551256

@@ -1270,7 +1271,7 @@ binary_reader_get_info(BinaryReader *reader)
12701271
return NULL;
12711272
}
12721273
return Py_BuildValue(
1273-
"{s:I, s:N, s:K, s:K, s:I, s:I, s:I, s:I, s:i}",
1274+
"{s:I, s:N, s:K, s:K, s:K, s:I, s:I, s:I, s:i}",
12741275
"version", BINARY_FORMAT_VERSION,
12751276
"python_version", py_version,
12761277
"start_time_us", reader->start_time_us,

Modules/_remote_debugging/binary_io_writer.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -924,12 +924,6 @@ static int
924924
process_thread_sample(BinaryWriter *writer, PyObject *thread_info,
925925
uint32_t interpreter_id, uint64_t timestamp_us)
926926
{
927-
if (writer->total_samples >= UINT32_MAX) {
928-
PyErr_SetString(PyExc_OverflowError,
929-
"too many samples for binary format");
930-
return -1;
931-
}
932-
933927
PyObject *thread_id_obj = PyStructSequence_GET_ITEM(thread_info, 0);
934928
PyObject *status_obj = PyStructSequence_GET_ITEM(thread_info, 1);
935929
PyObject *frame_list = PyStructSequence_GET_ITEM(thread_info, 2);

Modules/_remote_debugging/module.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
typedef struct {
1515
PyObject_HEAD
1616
BinaryWriter *writer;
17-
uint32_t cached_total_samples; /* Preserved after finalize */
17+
uint64_t cached_total_samples; /* Preserved after finalize */
1818
} BinaryWriterObject;
1919

2020
typedef struct {
@@ -1916,9 +1916,9 @@ BinaryWriter_get_total_samples(PyObject *op, void *closure)
19161916
BinaryWriterObject *self = BinaryWriter_CAST(op);
19171917
if (!self->writer) {
19181918
/* Use cached value after finalize/close */
1919-
return PyLong_FromUnsignedLong(self->cached_total_samples);
1919+
return PyLong_FromUnsignedLongLong(self->cached_total_samples);
19201920
}
1921-
return PyLong_FromUnsignedLong(self->writer->total_samples);
1921+
return PyLong_FromUnsignedLongLong(self->writer->total_samples);
19221922
}
19231923

19241924
static PyGetSetDef BinaryWriter_getset[] = {
@@ -2141,7 +2141,7 @@ BinaryReader_get_sample_count(BinaryReaderObject *self, void *closure)
21412141
if (!self->reader) {
21422142
return PyLong_FromLong(0);
21432143
}
2144-
return PyLong_FromUnsignedLong(self->reader->sample_count);
2144+
return PyLong_FromUnsignedLongLong(self->reader->sample_count);
21452145
}
21462146

21472147
static PyObject *

0 commit comments

Comments
 (0)