Skip to content

gh-141370: Fix undefined behavior when using Py_ABS() #141548

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

gh-141370: Fix undefined behavior when using Py_ABS() #141548

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1acabab
gh-141370: Fix undefined behavior when using Py_ABS()
serhiy-storchaka Nov 14, 2025
445bcc8
Merge branch 'main' into Py_ABC-UB
skirpichev Nov 15, 2025
887853d
Merge branch 'main' into Py_ABC-UB
serhiy-storchaka Nov 17, 2025
df24c4f
Use more efficient implementation.
serhiy-storchaka Nov 17, 2025
a584478
Merge remote-tracking branch 'refs/remotes/origin/Py_ABC-UB' into Py_…
serhiy-storchaka Nov 17, 2025
b292561
Try other code. Add a comment.
serhiy-storchaka Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/pymacro.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
#define Py_MAX(x, y) (((x) > (y)) ? (x) : (y))

/* Absolute value of the number x */
#define _Py_ABS_CAST(T,x) ((x) >= 0 ? ((T) (x)) : (- (((T) ((x) + 1)) - 1)))
Copy link
Member

@picnixz picnixz Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define _Py_ABS_CAST(T,x) ((x) >= 0 ? ((T) (x)) : (- (((T) ((x) + 1)) - 1)))
#define _Py_ABS_CAST(T, x) ((x) >= 0 ? ((T) (x)) : (- (((T) ((x) + 1)) - 1)))

Copy link
Member

@picnixz picnixz Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity:

_Py_ABS_CAST(uint8_t, (int8_t)-128) == 128

The (T)((x) + 1) - 1 is:

(
 (
   (uint8_t) (
     (-128) + 1  // -127 (still int8_t)
   ) // 129 (2's complement on 8 bits)
 ) - 1 // 128 (as an uint8_t)
)

Since $-128\bmod{256} = 128$, we are good. For another number say -5 we have:

(
 (
   (uint8_t) (
     (-5) + 1  // -4 (still int8_t)
   ) // 252 (2's complement on 8 bits)
 ) - 1 // 251 (as an uint8_t)
)

And now $-251 \bmod{256} = 5$ and we're good.

#define Py_ABS(x) ((x) < 0 ? -(x) : (x))

#define _Py_XSTRINGIFY(x) #x
Expand Down
11 changes: 11 additions & 0 deletions Lib/test/test_bytes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,17 @@ def test_hex_separator_basics(self):
self.assertEqual(three_bytes.hex(':', 2), 'b9:01ef')
self.assertEqual(three_bytes.hex(':', 1), 'b9:01:ef')
self.assertEqual(three_bytes.hex('*', -2), 'b901*ef')
self.assertEqual(three_bytes.hex(sep=':', bytes_per_sep=2), 'b9:01ef')
self.assertEqual(three_bytes.hex(sep='*', bytes_per_sep=-2), 'b901*ef')
for bytes_per_sep in 3, -3, 2**31-1, -(2**31-1):
with self.subTest(bytes_per_sep=bytes_per_sep):
self.assertEqual(three_bytes.hex(':', bytes_per_sep), 'b901ef')
for bytes_per_sep in 2**31, -2**31, 2**1000, -2**1000:
with self.subTest(bytes_per_sep=bytes_per_sep):
try:
self.assertEqual(three_bytes.hex(':', bytes_per_sep), 'b901ef')
except OverflowError:
pass

value = b'{s\005\000\000\000worldi\002\000\000\000s\005\000\000\000helloi\001\000\000\0000'
self.assertEqual(value.hex('.', 8), '7b7305000000776f.726c646902000000.730500000068656c.6c6f690100000030')
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_marshal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ def test_ints(self):
for expected in (-n, n):
self.helper(expected)
n = n >> 1
n = 1 << 100
while n:
for expected in (-n, -n+1, n-1, n):
self.helper(expected)
n = n >> 1

def test_int64(self):
# Simulate int marshaling with TYPE_INT64.
Expand Down
19 changes: 19 additions & 0 deletions Lib/test/test_memoryview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,25 @@ def test_memoryview_hex(self):
m2 = m1[::-1]
self.assertEqual(m2.hex(), '30' * 200000)

def test_memoryview_hex_separator(self):
x = bytes(range(97, 102))
m1 = memoryview(x)
m2 = m1[::-1]
self.assertEqual(m2.hex(':'), '65:64:63:62:61')
self.assertEqual(m2.hex(':', 2), '65:6463:6261')
self.assertEqual(m2.hex(':', -2), '6564:6362:61')
self.assertEqual(m2.hex(sep=':', bytes_per_sep=2), '65:6463:6261')
self.assertEqual(m2.hex(sep=':', bytes_per_sep=-2), '6564:6362:61')
for bytes_per_sep in 5, -5, 2**31-1, -(2**31-1):
with self.subTest(bytes_per_sep=bytes_per_sep):
self.assertEqual(m2.hex(':', bytes_per_sep), '6564636261')
for bytes_per_sep in 2**31, -2**31, 2**1000, -2**1000:
with self.subTest(bytes_per_sep=bytes_per_sep):
try:
self.assertEqual(m2.hex(':', bytes_per_sep), '6564636261')
except OverflowError:
pass

def test_copy(self):
m = memoryview(b'abc')
with self.assertRaises(TypeError):
Expand Down
2 changes: 1 addition & 1 deletion Python/marshal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@
}
if (!long_export.digits) {
int8_t sign = long_export.value < 0 ? -1 : 1;
uint64_t abs_value = Py_ABS(long_export.value);
uint64_t abs_value = _Py_ABS_CAST(uint64_t, long_export.value);

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 313 in Python/marshal.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]
uint64_t d = abs_value;
long l = 0;

Expand Down
3 changes: 1 addition & 2 deletions Python/pystrhex.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
else {
bytes_per_sep_group = 0;
}

unsigned int abs_bytes_per_sep = Py_ABS(bytes_per_sep_group);
unsigned int abs_bytes_per_sep = _Py_ABS_CAST(unsigned int, bytes_per_sep_group);

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / Build and test (arm64) 

unary minus operator applied to unsigned type, result still unsigned [C:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 45 in Python/pystrhex.c

View workflow job for this annotation

GitHub Actions / Windows / Build and test (x64) 

unary minus operator applied to unsigned type, result still unsigned [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]
Py_ssize_t resultlen = 0;
if (bytes_per_sep_group && arglen > 0) {
/* How many sep characters we'll be inserting. */
Expand Down
Loading