Description

Yhg1s

opened on Jul 20, 2023

Issue body actions

When using MemorySanitizer, maybe_small_long() in Objects/longobject.c is flagged as using uninitialized memory when passed certain values, which means Python main, 312 and 3.11 can't even be built correctly with --with-memorysanitizer:

% CC=clang-13 CXX=clang++-13 ./configure --with-memory-sanitizer
[...]
% make
[...]
./_bootstrap_python Programs/_freeze_module.py abc Lib/abc.py Python/frozen_modules/abc.h
==21198==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7c5058 in maybe_small_long Objects/longobject.c:62:13
    #1 0x7c5058 in long_bitwise Objects/longobject.c:5267:24
    #2 0x7b81a6 in long_and Objects/longobject.c:5279:12
[...]
  Uninitialized value was created by a heap allocation
    #0 0x45288d in __interceptor_malloc (_bootstrap_python+0x45288d)
    #1 0x878da6 in _PyMem_RawMalloc Objects/obmalloc.c:45:12
    #2 0x87bb6f in PyObject_Malloc Objects/obmalloc.c:801:12
    #3 0x7bf9fc in _PyLong_New Objects/longobject.c:158:14
    #4 0x7bf9fc in long_bitwise Objects/longobject.c:5225:9
    #5 0x7b81a6 in long_and Objects/longobject.c:5279:12
[...]
SUMMARY: MemorySanitizer: use-of-uninitialized-value Objects/longobject.c:62:13 in maybe_small_long
Exiting
make: *** [Makefile:1332: Python/frozen_modules/abc.h] Error 1

The source of the problem is _PyLong_CompactValue (from Include/cpython/longintrepr.h) multiplying ob_digit[0] by size, where ob_digit[0] may be uninitialized when size is 0 (the simplest reproducer for that is int('0')). (In Python 3.11 this same logic happens in medium_value() in longobject.c instead. Python 3.10 had similar logic in the MEDIUJM_VALUE macro in longobject.c, but it had an explicit check for size being 0.)

(I believe, but I'm not entirely sure, the code relies on undefined behaviour, which we should avoid -- although it's only undefined for very esoteric reasons. Even so, the simple solution is to add back the check for sign being 0. If that really has a noticeable impact, we could add the branch only when using MemorySanitizer.)

I believe Greg was working on getting us to a point where we could have a MSan buildbot (#79395), although I don't know what the state of that is now.

Activity

1. Yhg1s

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jul 20, 2023
2. Yhg1s

   assigned

   gpshead

   and

   markshannon

   on Jul 20, 2023
3. AlexWaygood commented on Jul 20, 2023

   

   AlexWaygood

   on Jul 20, 2023

   Member

   More actions

   Duplicate of #102509?
4. gpshead commented on Jul 20, 2023

   

   gpshead

   on Jul 20, 2023

   Member

   More actions

   I believe Greg was working on getting us to a point where we could have a MSan buildbot (#79395), although I don't know what the state of that is now.

   I started down that path at one point in 2018-19ish... but found MSAN so painful to setup as it really doesn't work well in an environment where you haven't compiled the entire system (all transitive dependency dynamically linked libraries) with MSAN. Without that it relied on an MSAN baked in exclusion list of known "misbehaviors" from various libraries otherwise and that list vs actual implementation was never sufficient for results I'd want to surface in front of people. We should revisit this at some point, maybe the feasibility has changed? (internally at work where we build our entire stack, MSAN works pretty well, because a team exists to keep it relevant)

   Regardless, this may well be a dup as Alex pointed out (thanks!). Investigation required; there's some interesting discussion over there.

   Reacted by Alex Waygood
5. markshannon commented on Jul 28, 2023

   

   markshannon

   on Jul 28, 2023

   Member

   More actions

   Duplicate of #102509?

   Yes, closing.
6. markshannon

   closed this as not plannedon Jul 28, 2023
7. illia-v commented on Aug 5, 2023

   

   illia-v

   on Aug 5, 2023

   Contributor

   More actions

   @gpshead the MSAN tests can be set up using CIFuzz pretty cleanly, please check #107653

Issue actions