gh-128137: Update PyASCIIObject to handle interned field with the atomic operation

[corona10]

• Issue: Race in PyUnicode_InternFromString under free-threading #128137

[corona10]

@colesbury I am not sure what you intended.
To maintain the original size, I squeeze the state field from 32 bits to 16 bits and reserve 16 bits for the interned field.

[corona10]

f30b355 breaks WASI test: https://github.com/python/cpython/actions/runs/12469477403/job/34802702354?pr=128196 (object size is increased).

[colesbury]

I think this remain in the state struct. It's okay for a struct to contain both non-bitfield and bitfield members:

• It avoids a potential unnecessary breakage from moving the field
• Keeping it in state will make it easier to keep state 32-bits due to alignment.

[colesbury]

I think this should have a NEWS entry given that we're changing the size of a semi-public field.

[corona10]

Since we touch the semi-public field, I am not sure about backporting this PR.
Let's just leave 3.13t as buggy status because it is just the experimental build.

[corona10]

@colesbury @methane

I 've updated test_str.py and test_sys.py for win32 and WASI, but there seems to be an alignment issue that we didn't care about before.
Do you think the current implementation is fine, or would you prefer to redesign the layout to be more consistent?
(Not sure about this currently)

[colesbury]

I'm not sure. It's too bad that compilers treat alignment of bitfields differently. The two options I'd consider are:

1. As in this PR. The downside is this uses an extra four bytes on 32-bit Windows.

        uint8_t interned;
        unsigned int kind:3;
        unsigned int compact:1;
        unsigned int ascii:1;
        unsigned int statically_allocated:1;
        unsigned int :18;

2. Using unsigned char for the bitfield. This avoids the extra four bytes on 32-bit Windows, but may not be as portable. (Additional implementation-defined types may be acceptable ¹). GCC, MSVC, Clang, IBM's XL compiler support this.

        uint8_t interned;
        unsigned char kind:3;
        unsigned char compact:1;
        unsigned char ascii:1;
        unsigned char statically_allocated:1;
        unsigned char :2;
        unsigned char :8;
        unsigned char :8;

Footnotes

1. https://en.cppreference.com/w/c/language/bit_field#:~:text=Additional%20implementation%2Ddefined%20types%20may%20be%20acceptable ↩

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 53147e7 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[corona10]

@colesbury

I use unsigned short, which is supported by specific compilers as the same as unsigned char, and I attach related links about it.
And I am fine with if we can cover PEP 11 platforms.

• gcc: https://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html
• clang: Normally follow gcc's behavior. (No official implemenatation detail manual exists about it document implementation-defined behavior llvm/llvm-project#11644)
• MSVC: https://learn.microsoft.com/en-us/cpp/c-language/c-bit-fields?view=msvc-170
• IBM XL: https://www.ibm.com/docs/en/xcfbg/121.141?topic=modes-alignment-bit-fields

But if we want to forget about 4 bytes increasement issue at 32 bit platforms, and just use portable bit field declaration could be one possible option. Because we can just add specific unit test handling code for specific platforms but I don't prefer this option because it is very difficult to track platform specific issue in the future.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 30c2eb2 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[corona10]

Ah, okay, I re-read https://bugs.python.org/issue19537; unsigned short is not proper, so I revert to using unsigned char instead. It will be correct. (But I need to investigate Windows issue again)

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 91907aa 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[corona10]

I think that it would be fine

• uint16_t: interned field = 16bits (Increase it to 16bits is little bit painful, but it would be nothing for modern CPUs)
• unsigned short: kind + compact + ascii + statically_allocated + padding = 16bits (for 32bits and 64bits system both)
• In total: 32 bits

[corona10]

In Linux

• sizeof PyASCIIObject is 40bytes
• sizeof PyASCIIObject.state is 4 bytes.

So no regression will be.

[colesbury]

This LGTM. @methane, would you please take a look at this as well?