gh-111942: Fix crashes in TextIOWrapper.reconfigure() #111976

serhiy-storchaka · 2023-11-10T20:14:49Z

Fix crash when encoding is not string or None.
Fix crash when both line_buffering and write_through raise exception when converted ti int.
Add a number of tests for constructor and reconfigure() method with invalid arguments.

Issue: TextIOWrapper.reconfigure() crashes if encoding is not string or None #111942

* Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments.

aisk · 2023-11-12T07:55:14Z

textiowrapper_change_encoding is only been called by _io_TextIOWrapper_reconfigure_impl, and it's a clinic function, so I think we can just change the signature of it from:

encoding: object = None

to:

encoding: str(accept={str, NoneType}) = None

Then clinic will do the type check. This makes the configure method haves more equal behavior with the constructor, and have better type infomations.

serhiy-storchaka · 2023-11-12T17:19:39Z

encoding: str(accept={str, NoneType}) = None

It is more complex change, some code uses encoding and errors as PyObject *. It also has side effects for strings with embedded surrogate or null characters.

I plan such changes, but I am not sure that they will be backported. On other hand, the fix for a crash should be backported.

miss-islington-app · 2023-11-14T15:38:00Z

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

…-111976) * Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments. (cherry picked from commit ee06fff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app · 2023-11-14T15:38:14Z

GH-112058 is a backport of this pull request to the 3.12 branch.

…-111976) * Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments. (cherry picked from commit ee06fff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app · 2023-11-14T15:38:18Z

GH-112059 is a backport of this pull request to the 3.11 branch.

bedevere-bot · 2023-11-14T16:11:18Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian PGO 3.x has failed when building commit ee06fff.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/249/builds/6946) and take a look at the build logs.
Check if the failure is related to this commit (ee06fff) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/249/builds/6946

Failed tests:

test_io

Failed subtests:

test_constructor - test.test_io.CTextIOWrapperTest.test_constructor

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/var/lib/buildbot/workers/enable-optimizations-bot/3.x.gps-debian-profile-opt.pgo/build/Lib/test/test_io.py", line 2727, in test_constructor
    t.__init__(b, encoding="utf-8", errors='\udcfe')
SystemError: <method 'readable' of '_io.BytesIO' objects> returned a result with an exception set

bedevere-bot · 2023-11-14T16:13:05Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 Debian Clang LTO + PGO 3.x has failed when building commit ee06fff.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1084/builds/2603) and take a look at the build logs.
Check if the failure is related to this commit (ee06fff) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1084/builds/2603

Failed tests:

test_io

Failed subtests:

test_constructor - test.test_io.CTextIOWrapperTest.test_constructor

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/var/lib/buildbot/workers/arm64-clang/3.x.gps-arm64-debian.clang.lto-pgo/build/Lib/test/test_io.py", line 2727, in test_constructor
    t.__init__(b, encoding="utf-8", errors='\udcfe')
SystemError: <method 'readable' of '_io.BytesIO' objects> returned a result with an exception set

…) (GH-112058) * Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments. (cherry picked from commit ee06fff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-bot · 2023-11-14T16:37:40Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM Raspbian 3.x has failed when building commit ee06fff.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/424/builds/5401) and take a look at the build logs.
Check if the failure is related to this commit (ee06fff) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/424/builds/5401

Failed tests:

test_io

Summary of the results of the build (if available):

==

Click to see traceback logs

remote: Enumerating objects: 24, done.        
remote: Counting objects:   4% (1/24)        
remote: Counting objects:   8% (2/24)        
remote: Counting objects:  12% (3/24)        
remote: Counting objects:  16% (4/24)        
remote: Counting objects:  20% (5/24)        
remote: Counting objects:  25% (6/24)        
remote: Counting objects:  29% (7/24)        
remote: Counting objects:  33% (8/24)        
remote: Counting objects:  37% (9/24)        
remote: Counting objects:  41% (10/24)        
remote: Counting objects:  45% (11/24)        
remote: Counting objects:  50% (12/24)        
remote: Counting objects:  54% (13/24)        
remote: Counting objects:  58% (14/24)        
remote: Counting objects:  62% (15/24)        
remote: Counting objects:  66% (16/24)        
remote: Counting objects:  70% (17/24)        
remote: Counting objects:  75% (18/24)        
remote: Counting objects:  79% (19/24)        
remote: Counting objects:  83% (20/24)        
remote: Counting objects:  87% (21/24)        
remote: Counting objects:  91% (22/24)        
remote: Counting objects:  95% (23/24)        
remote: Counting objects: 100% (24/24)        
remote: Counting objects: 100% (24/24), done.        
remote: Compressing objects:   7% (1/13)        
remote: Compressing objects:  15% (2/13)        
remote: Compressing objects:  23% (3/13)        
remote: Compressing objects:  30% (4/13)        
remote: Compressing objects:  38% (5/13)        
remote: Compressing objects:  46% (6/13)        
remote: Compressing objects:  53% (7/13)        
remote: Compressing objects:  61% (8/13)        
remote: Compressing objects:  69% (9/13)        
remote: Compressing objects:  76% (10/13)        
remote: Compressing objects:  84% (11/13)        
remote: Compressing objects:  92% (12/13)        
remote: Compressing objects: 100% (13/13)        
remote: Compressing objects: 100% (13/13), done.        
remote: Total 13 (delta 11), reused 1 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'ee06fffd38cb51ce1c045da9d8336d9ce13c318a'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at ee06fffd38 gh-111942: Fix crashes in TextIOWrapper.reconfigure() (GH-111976)
Switched to and reset branch 'main'

Objects/obmalloc.c: In function ‘py_mimalloc_print_stats’:
Objects/obmalloc.c:2733:45: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 3 has type ‘long unsigned int’ [-Wformat=]
 2733 |     fprintf(out, "Small block threshold = %zd, in %u size classes.\n",
      |                                           ~~^
      |                                             |
      |                                             int
      |                                           %ld
Objects/obmalloc.c:2735:46: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 3 has type ‘long unsigned int’ [-Wformat=]
 2735 |     fprintf(out, "Medium block threshold = %zd\n",
      |                                            ~~^
      |                                              |
      |                                              int
      |                                            %ld
Objects/obmalloc.c:2737:45: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 3 has type ‘long unsigned int’ [-Wformat=]
 2737 |     fprintf(out, "Large object max size = %zd\n",
      |                                           ~~^
      |                                             |
      |                                             int
      |                                           %ld
In file included from Python/ceval.c:1020:
Python/executor_cases.c.h: In function ‘_PyEval_EvalFrameDefault’:
Python/executor_cases.c.h:1734:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 1734 |             PyObject *descr = (PyObject *)operand;
      |                               ^
Python/executor_cases.c.h:2368:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 2368 |             PyObject *descr = (PyObject *)operand;
      |                               ^
Python/executor_cases.c.h:2387:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 2387 |             PyObject *descr = (PyObject *)operand;
      |                               ^
Python/executor_cases.c.h:2405:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 2405 |             PyObject *descr = (PyObject *)operand;
      |                               ^
Python/executor_cases.c.h:2419:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 2419 |             PyObject *descr = (PyObject *)operand;
      |                               ^
Python/executor_cases.c.h:2446:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 2446 |             PyObject *descr = (PyObject *)operand;
      |                               ^
./Modules/_testcapi/heaptype_relative.c: In function ‘make_sized_heaptypes’:
./Modules/_testcapi/heaptype_relative.c:67:28: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   67 |                            (unsigned long long)data_ptr,
      |                            ^

make: *** [Makefile:2073: buildbottest] Error 2

vstinner · 2023-11-14T18:09:17Z

@serhiy-storchaka: test_io now fails if Python is built in release mode.

ERROR: test_constructor (test.test_io.CTextIOWrapperTest.test_constructor)
----------------------------------------------------------------------
UnicodeEncodeError: 'utf-8' codec can't encode character '\udcfe' in position 0: surrogates not allowed

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/test/test_io.py", line 2727, in test_constructor
    t.__init__(b, encoding="utf-8", errors='\udcfe')
SystemError: <method 'readable' of '_io.BytesIO' objects> returned a result with an exception set

This issue now prevents to make any further change, since Address Sanitizer builds Python in release mode and fails.

vstinner · 2023-11-14T18:12:17Z

I wrote #112067 to fix the regression.

…) (GH-112059) * Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments. (cherry picked from commit ee06fff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…-111976) * Fix crash when encoding is not string or None. * Fix crash when both line_buffering and write_through raise exception when converted ti int. * Add a number of tests for constructor and reconfigure() method with invalid arguments.

serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Nov 10, 2023

serhiy-storchaka requested review from ncoghlan and pitrou November 10, 2023 20:14

bedevere-app bot added the awaiting core review label Nov 10, 2023

bedevere-app bot mentioned this pull request Nov 10, 2023

TextIOWrapper.reconfigure() crashes if encoding is not string or None #111942

Closed

Some checks are only in debug mode.

1beb787

serhiy-storchaka merged commit ee06fff into python:main Nov 14, 2023

bedevere-app bot removed the awaiting core review label Nov 14, 2023

serhiy-storchaka deleted the TextIOWrapper-reconfigure branch November 14, 2023 15:38

bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 14, 2023

bedevere-app bot removed the needs backport to 3.11 only security fixes label Nov 14, 2023

vstinner mentioned this pull request Nov 14, 2023

gh-111942: Fix SystemError in the TextIOWrapper constructor #112061

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-111942: Fix crashes in TextIOWrapper.reconfigure() #111976

gh-111942: Fix crashes in TextIOWrapper.reconfigure() #111976

serhiy-storchaka commented Nov 10, 2023 •

edited by bedevere-app bot

Loading

aisk commented Nov 12, 2023 •

edited

Loading

serhiy-storchaka commented Nov 12, 2023

miss-islington-app bot commented Nov 14, 2023

bedevere-app bot commented Nov 14, 2023

bedevere-app bot commented Nov 14, 2023

bedevere-bot commented Nov 14, 2023

bedevere-bot commented Nov 14, 2023

bedevere-bot commented Nov 14, 2023

vstinner commented Nov 14, 2023

vstinner commented Nov 14, 2023

gh-111942: Fix crashes in TextIOWrapper.reconfigure() #111976

gh-111942: Fix crashes in TextIOWrapper.reconfigure() #111976

Conversation

serhiy-storchaka commented Nov 10, 2023 • edited by bedevere-app bot Loading

aisk commented Nov 12, 2023 • edited Loading

serhiy-storchaka commented Nov 12, 2023

miss-islington-app bot commented Nov 14, 2023

bedevere-app bot commented Nov 14, 2023

bedevere-app bot commented Nov 14, 2023

bedevere-bot commented Nov 14, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

bedevere-bot commented Nov 14, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

bedevere-bot commented Nov 14, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

vstinner commented Nov 14, 2023

vstinner commented Nov 14, 2023

serhiy-storchaka commented Nov 10, 2023 •

edited by bedevere-app bot

Loading

aisk commented Nov 12, 2023 •

edited

Loading