gh-156939: Detect buffer overflow in bytearray

[vstinner]

Add bytearray_check_consistency() and
bytearray_check_trailing_null_byte() functions in call them in most bytearray methods.

• Issue: C API: Detect buffer overflow in PyBytesWriter #156939

[vstinner]

Tests / Sanitizers / TSan (pull_request): Failing after 13m

It's a legit bug:

WARNING: ThreadSanitizer: data race (pid=15518)
  Write of size 8 at 0x55b013af2f80 by main thread:
    #0 __tsan_memcpy <null> (python+0xfc452) (BuildId: 77d4ccbb4bf7bb80d66adcdbb8a43d9cd1f555bc)
    #1 set_allocator_unlocked /home/runner/work/cpython/cpython/Objects/obmalloc.c (python+0x38417f) (BuildId: 77d4ccbb4bf7bb80d66adcdbb8a43d9cd1f555bc)
    #2 PyMem_SetAllocator /home/runner/work/cpython/cpython/Objects/obmalloc.c:1143:5 (python+0x38417f)
    (...)

I reported the failure as #157415 and I proposed a fix (skip the test if TSAN is used).

[vstinner]

The PR adds bytearray_check_consistency(). Python has similar functions for other types:

• any Python object: _PyObject_CheckConsistency()
• type: _PyType_CheckConsistency()
• dict: _PyDict_CheckConsistency()
• str: _PyUnicode_CheckConsistency()
• PyWideStringList C API: _PyWideStringList_CheckConsistency()

[vstinner]

@cmaloney: Would you mind to review this change?

I didn't measure the overhead on runtime benchmark of a Python debug build. If bytearray_check_consistency() overhead is too high, the PR can be limited to add bytearray_check_buffer_overflow() assertions.

[vstinner]

I just made a similar change to detect buffer overflow in PyBytesWriter: PR gh-156943.

[kumaraditya303]

I am a bit unsure about the usefulness of these checks, see my comment on issue.

[vstinner]

This change is quite big. It adds checks (check for buffer overflow and/or check consistency) to basically every single bytearray method.

I wrote a way smaller change which only checks for buffer overflow in bytearray destructor: PR gh-157529. Bonus: I also added a similar check for bytes!

[cmaloney]

I think this would be effective but unlikely to be remembered when adding new methods, prefer the smaller / simpler just checking at destruction time.

[vstinner]

I wasn't sure if my change was worth it when I wrote it. It modifies a lot of code, and I'm not sure that buffer overflows are common enough to justify added code.

After reading @kumaraditya303 and @cmaloney comments, I'm now confident that no, it's not worth it. I abandon this large change to focus on the simpler and shorter PR gh-157529.