| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Add bytearray_check_consistency() and bytearray_check_trailing_null_byte() functions in call them in most bytearray methods.
Reset the trailing byte before destroying the bytes/bytearray object.
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). |
Sorry, something went wrong.
|
The PR adds bytearray_check_consistency(). Python has similar functions for other types:
|
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
I just made a similar change to detect buffer overflow in PyBytesWriter: PR gh-156943. |
Sorry, something went wrong.
|
I am a bit unsure about the usefulness of these checks, see my comment on issue. |
Sorry, something went wrong.
|
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! |
Sorry, something went wrong.
|
I think this would be effective but unlikely to be remembered when adding new methods, prefer the smaller / simpler just checking at destruction time. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Add bytearray_check_consistency() and
bytearray_check_trailing_null_byte() functions in call them in most bytearray methods.