From 6f0d6112c399145fc17663e2efefcc433e051f95 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 12 Apr 2024 11:40:13 +0200 Subject: [PATCH 1/2] gh-117755: Fix mimalloc for huge allocation on s390x Fix mimalloc allocator for huge memory allocation (around 8,589,934,592 GiB) on s390x. * Abort allocation early in mimalloc if the number of slices doesn't fit into uint32_t, to prevent a integer overflow (cast 64-bit size_t to uint32_t). * Add test_large_alloc() to test_bigaddrspace (test skipped on 32-bit platforms). * Reenable test_maxcontext_exact_arith() of test_decimal on s390x. * Reenable test_constructor() tests of test_io on s390x. --- Lib/test/test_bigaddrspace.py | 62 ++++++++++++++++++- Lib/test/test_decimal.py | 6 +- Lib/test/test_io.py | 10 --- ...-04-12-12-28-49.gh-issue-117755.6ct8kU.rst | 2 + Objects/mimalloc/segment.c | 6 ++ 5 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-04-12-12-28-49.gh-issue-117755.6ct8kU.rst diff --git a/Lib/test/test_bigaddrspace.py b/Lib/test/test_bigaddrspace.py index 50272e99607fd3..509fa19e5a8be6 100644 --- a/Lib/test/test_bigaddrspace.py +++ b/Lib/test/test_bigaddrspace.py @@ -11,9 +11,25 @@ from test import support from test.support import bigaddrspacetest, MAX_Py_ssize_t -import unittest +import contextlib import operator +import os import sys +import unittest + + +@contextlib.contextmanager +def ignore_stderr(): + fd = 2 + old_stderr = os.dup(fd) + try: + # Redirect stderr to /dev/null + with open(os.devnull, 'wb') as null: + os.dup2(null.fileno(), fd) + yield + finally: + os.dup2(old_stderr, fd) + os.close(old_stderr) class BytesTest(unittest.TestCase): @@ -52,6 +68,50 @@ def test_repeat(self): finally: x = None + @unittest.skipUnless(sys.maxsize >= 0x7FFFFFFF_FFFFFFFF, + 'need 64-bit size') + def test_large_alloc(self): + debug_bytes = 0 + if support.check_impl_detail(cpython=True) and support.Py_DEBUG: + try: + from _testcapi import SIZEOF_SIZE_T + except ImportError: + if sys.maxsize > 0xffff_ffff: + SIZEOF_SIZE_T = 8 + else: + SIZEOF_SIZE_T = 4 + + # The debug hook on memory allocator adds 3 size_t per memory block + # See PYMEM_DEBUG_EXTRA_BYTES in Objects/obmalloc.c. + debug_bytes = SIZEOF_SIZE_T * 3 + + try: + from _testinternalcapi import pymem_getallocatorsname + if not pymem_getallocatorsname().endswith('_debug'): + # PYTHONMALLOC env var is used and disables the debug hook + debug_bytes = 0 + except (ImportError, RuntimeError): + pass + + def allocate(size): + length = size - sys.getsizeof(b'') - debug_bytes + # allocate 'size' bytes + return b'x' * length + + # 64-bit size which cannot be allocated on any reasonable hardware + # (in 2024) and must fail immediately with MemoryError. + for size in ( + # gh-114331: test_decimal.test_maxcontext_exact_arith() + 0x0BAFC246_72035E58, + # gh-117755: test_io.test_constructor() + 0x7FFFFFFF_FFFFFFFF, + ): + with self.subTest(size=size): + with self.assertRaises(MemoryError): + # ignore "mimalloc: error: unable to allocate memory" + with ignore_stderr(): + allocate(size) + class StrTest(unittest.TestCase): diff --git a/Lib/test/test_decimal.py b/Lib/test/test_decimal.py index 05dcb25a7e5950..f23ea8af0c8772 100644 --- a/Lib/test/test_decimal.py +++ b/Lib/test/test_decimal.py @@ -38,8 +38,7 @@ check_disallow_instantiation) from test.support import (TestFailed, run_with_locale, cpython_only, - darwin_malloc_err_warning, is_emscripten, - skip_on_s390x) + darwin_malloc_err_warning, is_emscripten) from test.support.import_helper import import_fresh_module from test.support import threading_helper from test.support import warnings_helper @@ -5651,9 +5650,6 @@ def __abs__(self): @unittest.skipIf(check_sanitizer(address=True, memory=True), "ASAN/MSAN sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") - # gh-114331: The test allocates 784 271 641 GiB and mimalloc does not fail - # to allocate it when using mimalloc on s390x. - @skip_on_s390x def test_maxcontext_exact_arith(self): # Make sure that exact operations do not raise MemoryError due diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index c3dc0572c58c27..331c26ce564506 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -41,7 +41,6 @@ assert_python_ok, assert_python_failure, run_python_until_end) from test.support import ( import_helper, is_apple, os_helper, skip_if_sanitizer, threading_helper, warnings_helper, - skip_on_s390x ) from test.support.os_helper import FakePath @@ -1701,9 +1700,6 @@ class CBufferedReaderTest(BufferedReaderTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") - # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes - # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. - @skip_on_s390x def test_constructor(self): BufferedReaderTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more @@ -2072,9 +2068,6 @@ class CBufferedWriterTest(BufferedWriterTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") - # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes - # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. - @skip_on_s390x def test_constructor(self): BufferedWriterTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more @@ -2597,9 +2590,6 @@ class CBufferedRandomTest(BufferedRandomTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") - # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes - # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. - @skip_on_s390x def test_constructor(self): BufferedRandomTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-12-12-28-49.gh-issue-117755.6ct8kU.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-12-12-28-49.gh-issue-117755.6ct8kU.rst new file mode 100644 index 00000000000000..a65ec43e25d1c5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-04-12-12-28-49.gh-issue-117755.6ct8kU.rst @@ -0,0 +1,2 @@ +Fix mimalloc allocator for huge memory allocation (around 8,589,934,592 GiB) on +s390x. Patch by Victor Stinner. diff --git a/Objects/mimalloc/segment.c b/Objects/mimalloc/segment.c index 08b156433653a4..0b4d3abc07a93c 100644 --- a/Objects/mimalloc/segment.c +++ b/Objects/mimalloc/segment.c @@ -814,6 +814,9 @@ static mi_segment_t* mi_segment_os_alloc( size_t required, size_t page_alignment const size_t extra = align_offset - info_size; // recalculate due to potential guard pages *psegment_slices = mi_segment_calculate_slices(required + extra, ppre_size, pinfo_slices); + + // mi_page_t.slice_count type is uint32_t + if (*psegment_slices > (size_t)UINT32_MAX) return NULL; } const size_t segment_size = (*psegment_slices) * MI_SEGMENT_SLICE_SIZE; @@ -865,6 +868,9 @@ static mi_segment_t* mi_segment_alloc(size_t required, size_t page_alignment, mi size_t pre_size; size_t segment_slices = mi_segment_calculate_slices(required, &pre_size, &info_slices); + // mi_page_t.slice_count type is uint32_t + if (segment_slices > (size_t)UINT32_MAX) return NULL; + // Commit eagerly only if not the first N lazy segments (to reduce impact of many threads that allocate just a little) const bool eager_delay = (// !_mi_os_has_overcommit() && // never delay on overcommit systems _mi_current_thread_count() > 1 && // do not delay for the first N threads From 14d1a1138975fcdbdad040c0c1a1c850cae47dbc Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 16 Apr 2024 18:24:27 +0200 Subject: [PATCH 2/2] Restore tests --- Lib/test/test_bigaddrspace.py | 62 +---------------------------------- Lib/test/test_decimal.py | 6 +++- Lib/test/test_io.py | 10 ++++++ 3 files changed, 16 insertions(+), 62 deletions(-) diff --git a/Lib/test/test_bigaddrspace.py b/Lib/test/test_bigaddrspace.py index 509fa19e5a8be6..50272e99607fd3 100644 --- a/Lib/test/test_bigaddrspace.py +++ b/Lib/test/test_bigaddrspace.py @@ -11,25 +11,9 @@ from test import support from test.support import bigaddrspacetest, MAX_Py_ssize_t -import contextlib +import unittest import operator -import os import sys -import unittest - - -@contextlib.contextmanager -def ignore_stderr(): - fd = 2 - old_stderr = os.dup(fd) - try: - # Redirect stderr to /dev/null - with open(os.devnull, 'wb') as null: - os.dup2(null.fileno(), fd) - yield - finally: - os.dup2(old_stderr, fd) - os.close(old_stderr) class BytesTest(unittest.TestCase): @@ -68,50 +52,6 @@ def test_repeat(self): finally: x = None - @unittest.skipUnless(sys.maxsize >= 0x7FFFFFFF_FFFFFFFF, - 'need 64-bit size') - def test_large_alloc(self): - debug_bytes = 0 - if support.check_impl_detail(cpython=True) and support.Py_DEBUG: - try: - from _testcapi import SIZEOF_SIZE_T - except ImportError: - if sys.maxsize > 0xffff_ffff: - SIZEOF_SIZE_T = 8 - else: - SIZEOF_SIZE_T = 4 - - # The debug hook on memory allocator adds 3 size_t per memory block - # See PYMEM_DEBUG_EXTRA_BYTES in Objects/obmalloc.c. - debug_bytes = SIZEOF_SIZE_T * 3 - - try: - from _testinternalcapi import pymem_getallocatorsname - if not pymem_getallocatorsname().endswith('_debug'): - # PYTHONMALLOC env var is used and disables the debug hook - debug_bytes = 0 - except (ImportError, RuntimeError): - pass - - def allocate(size): - length = size - sys.getsizeof(b'') - debug_bytes - # allocate 'size' bytes - return b'x' * length - - # 64-bit size which cannot be allocated on any reasonable hardware - # (in 2024) and must fail immediately with MemoryError. - for size in ( - # gh-114331: test_decimal.test_maxcontext_exact_arith() - 0x0BAFC246_72035E58, - # gh-117755: test_io.test_constructor() - 0x7FFFFFFF_FFFFFFFF, - ): - with self.subTest(size=size): - with self.assertRaises(MemoryError): - # ignore "mimalloc: error: unable to allocate memory" - with ignore_stderr(): - allocate(size) - class StrTest(unittest.TestCase): diff --git a/Lib/test/test_decimal.py b/Lib/test/test_decimal.py index f23ea8af0c8772..05dcb25a7e5950 100644 --- a/Lib/test/test_decimal.py +++ b/Lib/test/test_decimal.py @@ -38,7 +38,8 @@ check_disallow_instantiation) from test.support import (TestFailed, run_with_locale, cpython_only, - darwin_malloc_err_warning, is_emscripten) + darwin_malloc_err_warning, is_emscripten, + skip_on_s390x) from test.support.import_helper import import_fresh_module from test.support import threading_helper from test.support import warnings_helper @@ -5650,6 +5651,9 @@ def __abs__(self): @unittest.skipIf(check_sanitizer(address=True, memory=True), "ASAN/MSAN sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") + # gh-114331: The test allocates 784 271 641 GiB and mimalloc does not fail + # to allocate it when using mimalloc on s390x. + @skip_on_s390x def test_maxcontext_exact_arith(self): # Make sure that exact operations do not raise MemoryError due diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 331c26ce564506..c3dc0572c58c27 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -41,6 +41,7 @@ assert_python_ok, assert_python_failure, run_python_until_end) from test.support import ( import_helper, is_apple, os_helper, skip_if_sanitizer, threading_helper, warnings_helper, + skip_on_s390x ) from test.support.os_helper import FakePath @@ -1700,6 +1701,9 @@ class CBufferedReaderTest(BufferedReaderTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") + # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes + # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. + @skip_on_s390x def test_constructor(self): BufferedReaderTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more @@ -2068,6 +2072,9 @@ class CBufferedWriterTest(BufferedWriterTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") + # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes + # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. + @skip_on_s390x def test_constructor(self): BufferedWriterTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more @@ -2590,6 +2597,9 @@ class CBufferedRandomTest(BufferedRandomTest, SizeofTest): @skip_if_sanitizer(memory=True, address=True, thread=True, reason="sanitizer defaults to crashing " "instead of returning NULL for malloc failure.") + # gh-117755: The test allocates 9 223 372 036 854 775 807 bytes + # (0x7fffffffffffffff) and mimalloc fails with a division by zero on s390x. + @skip_on_s390x def test_constructor(self): BufferedRandomTest.test_constructor(self) # The allocation can succeed on 32-bit builds, e.g. with more