Get the test suite passing with clang Memory Sanitizer enabled

[gpshead]

BPO	35214
Nosy	@Yhg1s, @gpshead, @benjaminp, @alex, @serhiy-storchaka, @izbyshev, @pablogsal, @miss-islington, @epicfaace
PRs	bpo-35214: Initial clang MemorySanitizer support #10479 [3.7] bpo-35214: Initial clang MemorySanitizer support (GH-10479) #10492 [3.6] bpo-35214: Initial clang MemorySanitizer support (GH-10479) #10493 bpo-35214: Disable getc_unlocked() with MemorySanitizer. #10499 [3.7] bpo-35214: Disable getc_unlocked() with MemorySanitizer. (GH-10499) #10500 [3.6] bpo-35214: Disable getc_unlocked() with MemorySanitizer. (GH-10499) #10501 bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER define #10503 [3.7] bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER def. (GH-10503) #10504 [3.6] bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER def. (GH-10503) #10505 bpo-35214: Fix OOB memory access in unicode escape parser #10506 [3.7] bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506) #10522 [3.6] bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506) #10523 [2.7] bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506) #10538 bpo-35214: MSan workarounds for socket, time, and test_faulthandler. #11375 bpo-35214: MSan workarounds for socket, time, and test_faulthandler. #11375 bpo-35214: MSan workarounds for socket, time, and test_faulthandler. #11375 [3.7] bpo-35214: MSan workarounds for socket, time, and test_faulthandler. (GH-11375) #11378 [3.7] bpo-35214: MSan workarounds for socket, time, and test_faulthandler. (GH-11375) #11378 [3.7] bpo-35214: MSan workarounds for socket, time, and test_faulthandler. (GH-11375) #11378 bpo-35214: Skip test_io tests that'd cause a huge malloc under msan #11385 bpo-35214: Skip test_io tests that'd cause a huge malloc under msan #11385 bpo-35214: Skip test_io tests that'd cause a huge malloc under msan #11385 [3.7] bpo-35214: Skip test_io tests that'd cause a huge malloc under msan (GH-11385) #11388 [3.7] bpo-35214: Skip test_io tests that'd cause a huge malloc under msan (GH-11385) #11388 [3.7] bpo-35214: Skip test_io tests that'd cause a huge malloc under msan (GH-11385) #11388 bpo-35214: Annotate posix calls for clang MSan. #11389 bpo-35214: Annotate posix calls for clang MSan. #11389 bpo-35214: Annotate posix calls for clang MSan. #11389 [3.7] bpo-35214: Annotate posix calls for clang MSan. (GH-11389) #11391 [3.7] bpo-35214: Annotate posix calls for clang MSan. (GH-11389) #11391 [3.7] bpo-35214: Annotate posix calls for clang MSan. (GH-11389) #11391

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = None
created_at = <Date 2018-11-12.07:26:34.927>
labels = ['type-security', 'interpreter-core', '3.7', '3.8', 'extension-modules', 'build', 'tests']
title = 'Get the test suite passing with clang Memory Sanitizer enabled'
updated_at = <Date 2019-08-14.20:02:32.630>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2019-08-14.20:02:32.630>
actor = 'gregory.p.smith'
assignee = 'gregory.p.smith'
closed = False
closed_date = None
closer = None
components = ['Build', 'Extension Modules', 'Interpreter Core', 'Tests']
creation = <Date 2018-11-12.07:26:34.927>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 35214
keywords = ['patch']
message_count = 28.0
messages = ['329723', '329750', '329759', '329777', '329798', '329803', '329804', '329805', '329806', '329807', '329808', '329809', '329810', '329867', '329870', '329875', '329929', '331639', '331642', '332778', '332784', '332796', '332797', '332799', '332801', '332804', '349639', '349742']
nosy_count = 9.0
nosy_names = ['twouters', 'gregory.p.smith', 'benjamin.peterson', 'alex', 'serhiy.storchaka', 'izbyshev', 'pablogsal', 'miss-islington', 'epicfaace']
pr_nums = ['10479', '10492', '10493', '10499', '10500', '10501', '10503', '10504', '10505', '10506', '10522', '10523', '10538', '11375', '11375', '11375', '11378', '11378', '11378', '11385', '11385', '11385', '11388', '11388', '11388', '11389', '11389', '11389', '11391', '11391', '11391']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue35214'
versions = ['Python 3.7', 'Python 3.8']

[gpshead]

clang's memory sanitizer (-fsanitize=memory) turns up useful problems in code. I'm working on getting a CPython buildbot running it setup but would like our build to be cleaner to start with before I run that.

These are the initial fixes required for most of CPython to pass in an msan build. We've been using these with our interpreters at Google. (PR coming)

[gpshead]

New changeset 1584a00 by Gregory P. Smith in branch 'master':
bpo-35214: Initial clang MemorySanitizer support (GH-10479)
1584a00

[gpshead]

New changeset 5f4d05d by Gregory P. Smith in branch '3.7':
[3.7] bpo-35214: Initial clang MemorySanitizer support (GH-10479) (GH-10492)
5f4d05d

[gpshead]

New changeset 3b5b1c0 by Gregory P. Smith in branch '3.6':
[3.6] bpo-35214: Initial clang MemorySanitizer support (GH-10479) (GH-10493)
3b5b1c0

[benjaminp]

Can we prefix MEMORY_SANITIZER with _Py_?

[pablogsal]

I cannot initialize the interpreter after compiling with --with-memory-sanitizer:

❯ CC=clang ./configure --with-memory-sanitizer && make -j
❯ ./python
Python 3.8.0a0 (heads/master:1584a00815, Nov 13 2018, 03:29:18)
[Clang 7.0.0 (tags/RELEASE_700/final)] on linux
Type "help", "copyright", "credits" or "license" for more information.
==10989==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5592f18005c3 (/home/pablogsal/cpython/python+0x9a55c3)
#1 0x5592f175c176 (/home/pablogsal/cpython/python+0x901176)
#2 0x5592f17592da (/home/pablogsal/cpython/python+0x8fe2da)
#3 0x5592f1750f82 (/home/pablogsal/cpython/python+0x8f5f82)
#4 0x5592f174a336 (/home/pablogsal/cpython/python+0x8ef336)
#5 0x5592f174c906 (/home/pablogsal/cpython/python+0x8f1906)
#6 0x5592f14ae214 (/home/pablogsal/cpython/python+0x653214)
#7 0x5592f14a6915 (/home/pablogsal/cpython/python+0x64b915)
#8 0x5592f14a293c (/home/pablogsal/cpython/python+0x64793c)
#9 0x5592f0f5ad88 (/home/pablogsal/cpython/python+0xffd88)
#10 0x5592f0f5ce73 (/home/pablogsal/cpython/python+0x101e73)
#11 0x5592f0f4d908 (/home/pablogsal/cpython/python+0xf2908)
#12 0x7fd1a7381222 (/usr/lib/libc.so.6+0x24222)
#13 0x5592f0ed3cdd (/home/pablogsal/cpython/python+0x78cdd)

Uninitialized value was created by a heap allocation
#0 0x5592f0f02a0d (/home/pablogsal/cpython/python+0xa7a0d)
#1 0x7fd1a73cd790 (/usr/lib/libc.so.6+0x70790)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/home/pablogsal/github/cpython/python+0x9a55c3)
Exiting

❯ clang --version
clang version 7.0.0 (tags/RELEASE_700/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

❯ /lib/libc.so.6
GNU C Library (GNU libc) stable release version 2.28.
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 8.2.1 20180831.
libc ABIs: UNIQUE IFUNC ABSOLUTE
For bug reporting instructions, please see:
<https://bugs.archlinux.org/\>.

I am missing something or is this expected (as there are more PRs to come)?

[alex]

All libraries that are linked against, including libc, need to be compiled with MSAN. MSAN is not for the faint of heart.

[pablogsal]

Thanks @alex! I will try again after linking against ASAN-compiled version of my libraries.

[gpshead]

yeah, i've been surprised how far i was able to get from an oss CPython tree and pre-built clang 7 binary installation. If you have headers installed for common libraries like libssl-dev and libreadline-dev you can't produce an interpreter that'll work as those both lead to quick crashes. also, in my experience optimized builds tend to be more problematic (too early to say if what i'm seeing are actual problems).

also, make sure llvm-symbolize (no suffixes) is in your path for it to auto-symbolize the traces showing you where in the code it pointed out issues. when it points within shared libraries instead of the Python source tree - that's a hint that the library probably may need msan compilation.

I'm plodding through things to see if I can get a _useful_ buildbot setup (i don't care of some extension modules can't be covered for now) out of all of this.

[gpshead]

Can we prefix MEMORY_SANITIZER with _Py_?

Yes, I wondered if I should do that. not that I expect anyone would ever define it to mean anything else, but that seems like the right thing to do. #10503

[gpshead]

New changeset 3015fb8 by Gregory P. Smith in branch 'master':
bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER def. (GH-10503)
3015fb8

[miss-islington]

New changeset f6602f9 by Miss Islington (bot) in branch '3.7':
bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER def. (GH-10503)
f6602f9

[miss-islington]

New changeset 60cf265 by Miss Islington (bot) in branch '3.6':
bpo-35214: Add _Py_ prefix to MEMORY_SANITIZER def. (GH-10503)
60cf265

[gpshead]

New changeset 746b2d3 by Gregory P. Smith in branch 'master':
bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506)
746b2d3

[miss-islington]

New changeset fdc485a by Miss Islington (bot) in branch '3.6':
bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506)
fdc485a

[miss-islington]

New changeset 9fbcb14 by Miss Islington (bot) in branch '3.7':
[3.7] bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506) (GH-10522)
9fbcb14

[gpshead]

New changeset b6f4472 by Gregory P. Smith in branch '2.7':
[2.7] bpo-35214: Fix OOB memory access in unicode escape parser (GH-10506) (GH-10538)
b6f4472

[serhiy-storchaka]

Is this issue completely fixed?

[gpshead]

I believe there are still some issues to deal with. I don't want to close the issue until I've got my buildbot running.

[gpshead]

New changeset b474e67 by Gregory P. Smith in branch 'master':
bpo-35214: MSan workarounds for socket, time, and test_faulthandler. (GH-11375)
b474e67

[gpshead]

New changeset 01b9664 by Gregory P. Smith (Miss Islington (bot)) in branch '3.7':
bpo-35214: MSan workarounds for socket, time, and test_faulthandler. (GH-11375) (GH-11378)
01b9664

[gpshead]

New changeset e5796c4 by Gregory P. Smith in branch 'master':
bpo-35214: Skip test_io tests that'd cause a huge malloc under msan (bpo-11385)
e5796c4

[gpshead]

Status on my upcoming buildbot host after today's changes:

== Tests result: FAILURE ==

375 tests OK.

11 tests failed:
test_asyncio test_builtin test_code test_ctypes test_ioctl
test_openpty test_os test_posix test_pty test_shutil test_uuid

32 tests skipped:
test_bz2 test_curses test_dbm_gnu test_dbm_ndbm test_devpoll
test_gzip test_idle test_kqueue test_lzma test_msilib
test_ossaudiodev test_readline test_smtpnet test_socketserver
test_sqlite test_ssl test_startfile test_tcl test_timeout test_tix
test_tk test_ttk_guionly test_ttk_textonly test_turtle
test_urllib2net test_urllibnet test_winconsoleio test_winreg
test_winsound test_xmlrpc_net test_zipfile64 test_zlib

Most of those are dying due to pty use (openpty, etc) which is not properly memory sanitizer traced. test_posix appears to have something
I can fix by annotating in the code.

after that, I'll decide how to tell my buildbot not to run those tests so we can have a green buildbot memory sanitizing everything else.

[miss-islington]

New changeset 5d2e4b1 by Miss Islington (bot) in branch '3.7':
bpo-35214: Skip test_io tests that'd cause a huge malloc under msan (GH-11385)
5d2e4b1

[gpshead]

New changeset 1d300ce by Gregory P. Smith in branch 'master':
bpo-35214: Annotate posix calls for clang MSan. (bpo-11389)
1d300ce

[gpshead]

New changeset efcf08d by Gregory P. Smith in branch '3.7':
[3.7] bpo-35214: Annotate posix calls for clang MSan. (GH-11389) (GH-11391)
efcf08d

[epicfaace]

Can this be closed now?

[gpshead]

Nope, work remains to be done. I've got an msan buildbot system waiting but haven't had time to follow up on figuring out what remains in a while. (getting a functioning memory sanitizer build is... finnicky to say the least)

[vstinner]

@gpshead:

Nope, work remains to be done. I've got an msan buildbot system waiting but haven't had time to follow up on figuring out what remains in a while. (getting a functioning memory sanitizer build is... finnicky to say the least)

What's the status in 2023? Maybe it's time to close the issue. If needed, new a issue can be created.

[gpshead]

i mean while I think it'd be useful, i'm not working on it.

[vstinner]

Sadly, last time I tried MSAN, I got issues with stat() and OpenSSL:

• Python: ./Programs/_freeze_module fails with MSAN: Uninitialized value was created by an allocation of 'stat.i' #91043
• stat: msan: stat() result is seen as uninitialized (st_size) llvm/llvm-project#54131
• OpenSSL: msan: OBJ_txt2obj() fails with: MemorySanitizer: use-of-uninitialized-value openssl/openssl#17784

[vstinner]

Sometimes tried last March: #91043 (comment)