GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across Interpreters by ericsnowcurrently · Pull Request #119331 · python/cpython

Conversation

ericsnowcurrently commented May 21, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

_PyArg_Parser holds static global data generated for modules by Argument Clinic. The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global. In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters. However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes. It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime. The solution here is to temporarily switch to the main interpreter. The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.

Issue: getargs.c is Breaking Interpeter Isolation #119213

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently added 6 commits

May 21, 2024 08:11


          Identify where the problem is.

f0f080b


          Add a TODO.

c288a71


          Fix a no-gil bug in _parser_init().

35abdad


          Add a test for the crash.

e179765


          Always make sure kwtuple is allocatoed against the main interpreter.

e6f78fb


          Add NEWS.

dbf113b

ericsnowcurrently requested a review from erlend-aasland as a code owner

May 21, 2024 17:43

bedevere-app Bot added the awaiting core review label

May 21, 2024

bedevere-app Bot mentioned this pull request

May 21, 2024

getargs.c is Breaking Interpeter Isolation #119213

Closed

1st1 reviewed

May 21, 2024

View reviewed changes

Comment thread

Python/getargs.c Outdated

		struct _PyArg_Parser tmp, s = _PyRuntime.getargs.static_parsers;
		while (s) {
		tmp = s->next;
		//assert(tmp != s);

1st1 May 21, 2024

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

//assert(tmp != s);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently reacted with thumbs up emoji

All reactions

👍 1 reaction

ericsnowcurrently May 21, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

1st1 reviewed

May 21, 2024

View reviewed changes

Comment thread

Python/getargs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

1st1 reviewed

May 21, 2024

View reviewed changes

Comment thread

Python/getargs.c

		save_tstate = PyThreadState_Swap(temp_tstate);
		}
		kwtuple = new_kwtuple(keywords, len, pos);
		if (temp_tstate != NULL) {

1st1 May 21, 2024

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is doing this before checking if (kwtuple == NULL) the right way?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently May 21, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, we want to clean up the temporary tstate regardless of the outcome of new_kwtuple().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently added 3 commits

May 21, 2024 14:13


          Drop a dead line.

9b73c3d


          Update global objects.

6b9c436


          Merge branch 'main' into fix-getargs-parser

9157ad5

1st1 approved these changes

May 21, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels

May 21, 2024

ericsnowcurrently merged commit 8186500 into python:main

May 22, 2024

bedevere-app Bot removed the awaiting merge label

May 22, 2024

ericsnowcurrently added needs backport to 3.12

only security fixes

needs backport to 3.13 only security fixes labels

May 22, 2024

miss-islington-app Bot commented May 22, 2024

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented May 22, 2024

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented May 22, 2024

Copy link

Copy Markdown

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

cherry_picker 81865002aee8eaaeb3c7e402f86183afa6de77bf 3.12

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot assigned ericsnowcurrently

May 22, 2024

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

May 22, 2024


          pythongh-119213: Be More Careful About _PyArg_Parser.kwtuple Across I…

05a00aa

…nterpreters (pythongh-119331)

_PyArg_Parser holds static global data generated for modules by Argument Clinic.  The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global.  In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters.  However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes.  It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime.  The solution here is to temporarily switch to the main interpreter.  The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.
(cherry picked from commit 8186500)

Co-authored-by: Eric Snow <[email protected]>

bedevere-app Bot commented May 22, 2024

Copy link

Copy Markdown

GH-119410 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

May 22, 2024

bedevere-bot commented May 22, 2024

Copy link

Copy Markdown

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

Hi! The buildbot wasm32-wasi 3.x has failed when building commit 8186500.

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/1046/builds/5256) and take a look at the build logs.
Check if the failure is related to this commit (8186500) 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/1046/builds/5256

Failed tests:

test_math

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

Click to see traceback logs

remote: Enumerating objects: 27, done.        
remote: Counting objects:   3% (1/26)        
remote: Counting objects:   7% (2/26)        
remote: Counting objects:  11% (3/26)        
remote: Counting objects:  15% (4/26)        
remote: Counting objects:  19% (5/26)        
remote: Counting objects:  23% (6/26)        
remote: Counting objects:  26% (7/26)        
remote: Counting objects:  30% (8/26)        
remote: Counting objects:  34% (9/26)        
remote: Counting objects:  38% (10/26)        
remote: Counting objects:  42% (11/26)        
remote: Counting objects:  46% (12/26)        
remote: Counting objects:  50% (13/26)        
remote: Counting objects:  53% (14/26)        
remote: Counting objects:  57% (15/26)        
remote: Counting objects:  61% (16/26)        
remote: Counting objects:  65% (17/26)        
remote: Counting objects:  69% (18/26)        
remote: Counting objects:  73% (19/26)        
remote: Counting objects:  76% (20/26)        
remote: Counting objects:  80% (21/26)        
remote: Counting objects:  84% (22/26)        
remote: Counting objects:  88% (23/26)        
remote: Counting objects:  92% (24/26)        
remote: Counting objects:  96% (25/26)        
remote: Counting objects: 100% (26/26)        
remote: Counting objects: 100% (26/26), done.        
remote: Compressing objects:   7% (1/14)        
remote: Compressing objects:  14% (2/14)        
remote: Compressing objects:  21% (3/14)        
remote: Compressing objects:  28% (4/14)        
remote: Compressing objects:  35% (5/14)        
remote: Compressing objects:  42% (6/14)        
remote: Compressing objects:  50% (7/14)        
remote: Compressing objects:  57% (8/14)        
remote: Compressing objects:  64% (9/14)        
remote: Compressing objects:  71% (10/14)        
remote: Compressing objects:  78% (11/14)        
remote: Compressing objects:  85% (12/14)        
remote: Compressing objects:  92% (13/14)        
remote: Compressing objects: 100% (14/14)        
remote: Compressing objects: 100% (14/14), done.        
remote: Total 27 (delta 12), reused 13 (delta 12), pack-reused 1        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '81865002aee8eaaeb3c7e402f86183afa6de77bf'.

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 81865002ae gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across Interpreters (gh-119331)
Switched to and reset branch 'main'

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)

configure: WARNING: using cross tools not prefixed with host triplet
configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)

In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:130:11: warning: 'htole32' macro redefined [-Wmacro-redefined]
  130 | #  define htole32(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:43:9: note: previous definition is here
   43 | #define htole32(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:131:11: warning: 'le32toh' macro redefined [-Wmacro-redefined]
  131 | #  define le32toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:44:9: note: previous definition is here
   44 | #define le32toh(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:132:11: warning: 'htobe32' macro redefined [-Wmacro-redefined]
  132 | #  define htobe32(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:37:9: note: previous definition is here
   37 | #define htobe32(x) __bswap32(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:138:11: warning: 'be32toh' macro redefined [-Wmacro-redefined]
  138 | #  define be32toh(x) (htobe32((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:38:9: note: previous definition is here
   38 | #define be32toh(x) __bswap32(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:140:11: warning: 'htole64' macro redefined [-Wmacro-redefined]
  140 | #  define htole64(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:45:9: note: previous definition is here
   45 | #define htole64(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:141:11: warning: 'le64toh' macro redefined [-Wmacro-redefined]
  141 | #  define le64toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:46:9: note: previous definition is here
   46 | #define le64toh(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:142:11: warning: 'htobe64' macro redefined [-Wmacro-redefined]
  142 | #  define htobe64(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:39:9: note: previous definition is here
   39 | #define htobe64(x) __bswap64(x)
      |         ^
In file included from ../../Modules/md5module.c:46:
In file included from ../../Modules/_hacl/Hacl_Hash_MD5.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:149:11: warning: 'be64toh' macro redefined [-Wmacro-redefined]
  149 | #  define be64toh(x) (htobe64((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:40:9: note: previous definition is here
   40 | #define be64toh(x) __bswap64(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:130:11: warning: 'htole32' macro redefined [-Wmacro-redefined]
  130 | #  define htole32(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:43:9: note: previous definition is here
   43 | #define htole32(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:131:11: warning: 'le32toh' macro redefined [-Wmacro-redefined]
  131 | #  define le32toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:44:9: note: previous definition is here
   44 | #define le32toh(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:132:11: warning: 'htobe32' macro redefined [-Wmacro-redefined]
  132 | #  define htobe32(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:37:9: note: previous definition is here
   37 | #define htobe32(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:138:11: warning: 'be32toh' macro redefined [-Wmacro-redefined]
  138 | #  define be32toh(x) (htobe32((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:38:9: note: previous definition is here
   38 | #define be32toh(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:140:11: warning: 'htole64' macro redefined [-Wmacro-redefined]
  140 | #  define htole64(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:45:9: note: previous definition is here
   45 | #define htole64(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:141:11: warning: 'le64toh' macro redefined [-Wmacro-redefined]
  141 | #  define le64toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:46:9: note: previous definition is here
   46 | #define le64toh(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:142:11: warning: 'htobe64' macro redefined [-Wmacro-redefined]
  142 | #  define htobe64(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:39:9: note: previous definition is here
   39 | #define htobe64(x) __bswap64(x)
      |         ^
In file included from ../../Modules/sha1module.c:47:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA1.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:149:11: warning: 'be64toh' macro redefined [-Wmacro-redefined]
  149 | #  define be64toh(x) (htobe64((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:40:9: note: previous definition is here
   40 | #define be64toh(x) __bswap64(x)
      |         ^
8 warnings generated.
8 warnings generated.
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:130:11: warning: 'htole32' macro redefined [-Wmacro-redefined]
  130 | #  define htole32(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:43:9: note: previous definition is here
   43 | #define htole32(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:131:11: warning: 'le32toh' macro redefined [-Wmacro-redefined]
  131 | #  define le32toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:44:9: note: previous definition is here
   44 | #define le32toh(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:132:11: warning: 'htobe32' macro redefined [-Wmacro-redefined]
  132 | #  define htobe32(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:37:9: note: previous definition is here
   37 | #define htobe32(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:138:11: warning: 'be32toh' macro redefined [-Wmacro-redefined]
  138 | #  define be32toh(x) (htobe32((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:38:9: note: previous definition is here
   38 | #define be32toh(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:140:11: warning: 'htole64' macro redefined [-Wmacro-redefined]
  140 | #  define htole64(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:45:9: note: previous definition is here
   45 | #define htole64(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:141:11: warning: 'le64toh' macro redefined [-Wmacro-redefined]
  141 | #  define le64toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:46:9: note: previous definition is here
   46 | #define le64toh(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:142:11: warning: 'htobe64' macro redefined [-Wmacro-redefined]
  142 | #  define htobe64(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:39:9: note: previous definition is here
   39 | #define htobe64(x) __bswap64(x)
      |         ^
In file included from ../../Modules/sha3module.c:59:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA3.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:149:11: warning: 'be64toh' macro redefined [-Wmacro-redefined]
  149 | #  define be64toh(x) (htobe64((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:40:9: note: previous definition is here
   40 | #define be64toh(x) __bswap64(x)
      |         ^
8 warnings generated.
../../Modules/expat/xmlparse.c:7839:11: warning: format specifies type 'int' but the argument has type 'ptrdiff_t' (aka 'long') [-Wformat]
 7838 |           " (+" EXPAT_FMT_PTRDIFF_T("6") " bytes %s|%d, xmlparse.c:%d) %*s\"",
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~
 7839 |           bytesMore, (account == XML_ACCOUNT_DIRECT) ? "DIR" : "EXP",
      |           ^~~~~~~~~
1 warning generated.
../../Modules/_testinternalcapi/test_critical_sections.c:142:1: warning: unused function 'thread_critical_sections' [-Wunused-function]
  142 | thread_critical_sections(void *arg)
      | ^~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
../../Modules/posixmodule.c:7826:1: warning: unused function 'warn_about_fork_with_threads' [-Wunused-function]
 7826 | warn_about_fork_with_threads(const char* name)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:130:11: warning: 'htole32' macro redefined [-Wmacro-redefined]
  130 | #  define htole32(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:43:9: note: previous definition is here
   43 | #define htole32(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:131:11: warning: 'le32toh' macro redefined [-Wmacro-redefined]
  131 | #  define le32toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:44:9: note: previous definition is here
   44 | #define le32toh(x) (uint32_t)(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:132:11: warning: 'htobe32' macro redefined [-Wmacro-redefined]
  132 | #  define htobe32(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:37:9: note: previous definition is here
   37 | #define htobe32(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:138:11: warning: 'be32toh' macro redefined [-Wmacro-redefined]
  138 | #  define be32toh(x) (htobe32((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:38:9: note: previous definition is here
   38 | #define be32toh(x) __bswap32(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:140:11: warning: 'htole64' macro redefined [-Wmacro-redefined]
  140 | #  define htole64(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:45:9: note: previous definition is here
   45 | #define htole64(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:141:11: warning: 'le64toh' macro redefined [-Wmacro-redefined]
  141 | #  define le64toh(x) (x)
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:46:9: note: previous definition is here
   46 | #define le64toh(x) (uint64_t)(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:142:11: warning: 'htobe64' macro redefined [-Wmacro-redefined]
  142 | #  define htobe64(x)                                                           \
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:39:9: note: previous definition is here
   39 | #define htobe64(x) __bswap64(x)
      |         ^
In file included from ../../Modules/sha2module.c:48:
In file included from ../../Modules/_hacl/Hacl_Hash_SHA2.h:35:
In file included from ../../Modules/_hacl/include/krml/types.h:12:
../../Modules/_hacl/include/krml/lowstar_endianness.h:149:11: warning: 'be64toh' macro redefined [-Wmacro-redefined]
  149 | #  define be64toh(x) (htobe64((x)))
      |           ^
/opt/wasi-sdk-21.0/bin/../share/wasi-sysroot/include/endian.h:40:9: note: previous definition is here
   40 | #define be64toh(x) __bswap64(x)
      |         ^
8 warnings generated.

Kill <WorkerThread #1 running test=test_math pid=23541 time=25 min> process group
make: *** [Makefile:2208: buildbottest] Error 2

Cannot open file '/home/buildbot/buildarea/3.x.bcannon-wasi.wasi.debug/build/build/cross-build/wasm32-wasi/test-results.xml' for upload

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently added a commit that referenced this pull request

May 22, 2024


          [3.13] gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across …

bfd9c3e

…Interpreters (gh-119331) (gh-119410)

_PyArg_Parser holds static global data generated for modules by Argument Clinic.  The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global.  In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters.  However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes.  It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime.  The solution here is to temporarily switch to the main interpreter.  The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.

(cherry picked from commit 8186500)

Co-authored-by: Eric Snow <[email protected]>

ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request

May 22, 2024


          pythongh-119213: Be More Careful About _PyArg_Parser.kwtuple Across I…

c4bf08b

…nterpreters (pythongh-119331)

_PyArg_Parser holds static global data generated for modules by Argument Clinic.  The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global.  In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters.  However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes.  It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime.  The solution here is to temporarily switch to the main interpreter.  The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.

bedevere-app Bot commented May 22, 2024

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label

May 22, 2024

bedevere-bot commented May 22, 2024

Copy link

Copy Markdown

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

Hi! The buildbot AMD64 Ubuntu NoGIL 3.13 has failed when building commit bfd9c3e.

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/1422/builds/42) and take a look at the build logs.
Check if the failure is related to this commit (bfd9c3e) 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/1422/builds/42

Failed tests:

test_eintr

Failed subtests:

test_map_timeout - test.test_concurrent_futures.test_process_pool.ProcessPoolSpawnProcessPoolExecutorTest.test_map_timeout
test_lockf - main.FNTLEINTRTest.test_lockf
test_all - test.test_eintr.EINTRTests.test_all
test_flock - main.FNTLEINTRTest.test_flock
test_wait_integer - test.test_multiprocessing_spawn.test_misc.TestWait.test_wait_integer

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

Click to see traceback logs

Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 532, in test_lockf
    self._lock(fcntl.lockf, "lockf")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.3 sec


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 535, in test_flock
    self._lock(fcntl.flock, "flock")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.4 sec


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 535, in test_flock
    self._lock(fcntl.flock, "flock")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.3 sec


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_multiprocessing.py", line 5176, in test_wait_integer
    self.assertLess(delta, expected + 2)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
AssertionError: 5.200067439116538 not less than 5


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/test_eintr.py", line 17, in test_all
    script_helper.run_test_script(script)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/support/script_helper.py", line 316, in run_test_script
    raise AssertionError(f"{name} failed")
AssertionError: script _test_eintr.py failed


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/test_concurrent_futures/executor.py", line 71, in test_map_timeout
    self.assertEqual([None, None], results)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [None, None] != []


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 532, in test_lockf
    self._lock(fcntl.lockf, "lockf")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.4 sec

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently deleted the fix-getargs-parser branch

May 22, 2024 20:59

ericsnowcurrently mentioned this pull request

May 22, 2024

gh-119213: Fix getargs.c to store state in InterpreterState... #119195

Closed

ericsnowcurrently added a commit that referenced this pull request

May 22, 2024


          [3.12] gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across …

0d5fe2c

…Interpreters (gh-119331) (gh-119425)

_PyArg_Parser holds static global data generated for modules by Argument Clinic.  The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global.  In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters.  However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes.  It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime.  The solution here is to temporarily switch to the main interpreter.  The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.

(cherry picked from commit 8186500)

JelleZijlstra mentioned this pull request

May 28, 2024

backport a9a74da 3.13 #119642

Closed

estyxx pushed a commit to estyxx/cpython that referenced this pull request

Jul 17, 2024


          pythongh-119213: Be More Careful About _PyArg_Parser.kwtuple Across I…

712aaf2

…nterpreters (pythongh-119331)

_PyArg_Parser holds static global data generated for modules by Argument Clinic.  The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global.  In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters.  However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed.

This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes.  It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime.  The solution here is to temporarily switch to the main interpreter.  The alternative would be to always statically allocate the tuple.

This change also fixes a bug where only the most recent parser was added to the global linked list.

Reviewers

1st1

1st1 approved these changes

erlend-aasland

Awaiting requested review from erlend-aasland

erlend-aasland is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

ericsnowcurrently

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

ericsnowcurrently commented May 21, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1st1 May 21, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

1st1 May 21, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 21, 2024

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented May 22, 2024

Uh oh!

miss-islington-app Bot commented May 22, 2024

Uh oh!

miss-islington-app Bot commented May 22, 2024

Uh oh!

bedevere-app Bot commented May 22, 2024

Uh oh!

bedevere-bot commented May 22, 2024

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

Uh oh!

bedevere-app Bot commented May 22, 2024

Uh oh!

bedevere-bot commented May 22, 2024

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

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericsnowcurrently commented May 21, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.