Skip to content
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.

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

bpo-34602: Quadruple stack size on macOS when compiling with UBSAN #27309

Merged
merged 4 commits into from Sep 3, 2021

Conversation

@ambv
Copy link
Contributor

@ambv ambv commented Jul 23, 2021

When running ./configure with --with-undefined-behavior-sanitizer, the currently used stack size on macOS is insufficient to pass the test suite.

https://bugs.python.org/issue34602

@ambv
Copy link
Contributor Author

@ambv ambv commented Jul 23, 2021

Ideally we could only quadruple the stack size if --with-undefined-behavior-sanitizer is used but I'm not sure how to do that.

Copy link
Member

@ammaraskar ammaraskar left a comment

Ideally we could only quadruple the stack size if --with-undefined-behavior-sanitizer is used but I'm not sure how to do that.

I think you should just be able to do a conditional where LINKFORSHARED is set with

if test "$with_undefined_behavior_sanitizer" != "no"; then
configure Outdated Show resolved Hide resolved
@ned-deily ned-deily requested a review from ronaldoussoren Aug 1, 2021
@ambv ambv closed this Aug 10, 2021
@ambv ambv deleted the ambv:quadruple-stack-memory branch Aug 10, 2021
@ambv ambv restored the ambv:quadruple-stack-memory branch Aug 10, 2021
@ambv ambv reopened this Aug 10, 2021
@ambv
Copy link
Contributor Author

@ambv ambv commented Aug 10, 2021

Closed by mistake. I intend to add the quadrupling to pydebug builds only.

@ambv
Copy link
Contributor Author

@ambv ambv commented Aug 27, 2021

It works now only when --with-undefined-behavior-sanitizer is passed.

With:

checking LINKFORSHARED... -Wl,-stack_size,4000000  -framework CoreFoundation

Without:

checking LINKFORSHARED... -Wl,-stack_size,1000000  -framework CoreFoundation

Also, as @ammaraskar pointed out, this should be in sync with THREAD_STACK_SIZE so now it is.

configure Show resolved Hide resolved
configure.ac Show resolved Hide resolved
pyconfig.h.in Outdated Show resolved Hide resolved
@ambv ambv force-pushed the ambv:quadruple-stack-memory branch from 9f21de0 to 5d734c3 Aug 27, 2021
@ambv
Copy link
Contributor Author

@ambv ambv commented Aug 27, 2021

I rebased on current main and added Blurb.

To demonstrate why this is necessary:

Without this patch the test suite fails miserably with UBSAN

== Tests result: FAILURE ==

393 tests OK.

19 tests failed:
    test_exceptions test_fileio test_io test_isinstance test_json
    test_largefile test_lib2to3 test_logging test_os test_pickle
    test_pickletools test_plistlib test_richcmp test_runpy test_sys
    test_threading test_traceback test_typing test_xml_etree

Command terminated abnormally.
     1490.89 real      5134.98 user       222.52 sys

There's plenty of clear stack issues like:

(click to expand) Fatal Python error: Aborted
Current thread 0x000000010db75dc0 (most recent call first):
  File "/Volumes/RAMDisk/cpy2/Lib/test/libregrtest/setup.py", line 82 in _test_audit_hook
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  File "/Volumes/RAMDisk/cpy2/Lib/xml/etree/ElementTree.py", line 178 in __repr__
  ...

Extension modules: _testcapi (total: 1)

With the patch tests almost pass

411 tests OK.

1 test failed:
    test_threading

The threading failure is another thing we'll have to get to work for UBSAN on macOS but it's unrelated to this bug.

Copy link
Contributor

@ronaldoussoren ronaldoussoren left a comment

The patch looks good in general, as mentioned below I'd use AC_DEFINE_UNQUOTED(THREAD_STACK_SIZE, ...) even if non using sanitisers and remove setting THREAD_STACK_SIZE for macOS from thread_pthread.h.

configure.ac Outdated Show resolved Hide resolved
configure Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Aug 28, 2021

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@ambv
Copy link
Contributor Author

@ambv ambv commented Aug 28, 2021

I have made the requested changes; please review again.

Using --with-undefined-behavior-sanitizer:

>>> import sysconfig
>>> sysconfig.get_config_var('LINKFORSHARED')
'-Wl,-stack_size,4000000  -framework CoreFoundation'
>>> sysconfig.get_config_var('THREAD_STACK_SIZE')
'0x4000000'

No --with-undefined-behavior-sanitizer:

>>> import sysconfig
>>> sysconfig.get_config_var('LINKFORSHARED')
'-Wl,-stack_size,1000000  -framework CoreFoundation'
>>> sysconfig.get_config_var('THREAD_STACK_SIZE')
'0x1000000'

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Aug 28, 2021

Thanks for making the requested changes!

@ronaldoussoren: please review the changes made to this pull request.

@ambv ambv changed the title bpo-34602: Quadruple stack size to be able to run full test suite with sanitizers bpo-34602: Quadruple stack size on macOS when compiling with UBSAN Aug 28, 2021
@ambv
Copy link
Contributor Author

@ambv ambv commented Sep 3, 2021

Merging this since I have some UBSAN-related work for next week.

@ambv ambv merged commit be9de87 into python:main Sep 3, 2021
12 checks passed
12 checks passed
@github-actions
Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
@github-actions
Address sanitizer Address sanitizer
Details
Azure Pipelines PR #20210828.8 succeeded
Details
@bedevere-bot
bedevere/issue-number Issue number 34602 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 3, 2021

Thanks @ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒🤖

@ambv ambv deleted the ambv:quadruple-stack-memory branch Sep 3, 2021
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 3, 2021

Sorry, @ambv, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker be9de8721d63b9d8e032d508069daf88c06542c6 3.10

@ambv
Copy link
Contributor Author

@ambv ambv commented Sep 3, 2021

I'll deal with the conflict today.

@ambv
Copy link
Contributor Author

@ambv ambv commented Sep 3, 2021

The conflict is because the 3.10 branch used autoconf 2.71 for its configure file whereas main (like the other branches) uses 2.69.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment