New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issues with BINUNICODE8 and BINBYTES8 opcodes in pickle #69449
Comments
There are issues with BINUNICODE8 and BINBYTES8 opcodes in pickle.
Proposed patch fixes these issues. |
To access an array item, the type of the index variable must be size_t (or a type with the same size), otherwise the compiler produce less efficient machine code: => please keep Py_ssize_t type for i (I didn't check for the specific case of Py_ssize_t: it's signed. Does GCC emit the most efficient machine code for it?) diff -r 16c8278c03f6 Modules/_pickle.c
--- a/Modules/_pickle.c
+++ b/Modules/_pickle.c
@@ -4606,10 +4606,17 @@ static Py_ssize_t
calc_binsize(char *bytes, int nbytes)
{
unsigned char *s = (unsigned char *)bytes;
- Py_ssize_t i;
+ int i;
size_t x = 0;
- for (i = 0; i < nbytes && (size_t)i < sizeof(size_t); i++) {
+ if (nbytes > (int)sizeof(size_t)) {
+ for (i = (int)sizeof(size_t); i < nbytes; i++) {
+ if (s[i])
+ return -1;
+ }
+ nbytes = (int)sizeof(size_t);
+ } Please add a comment here to explain that the first loop check for integer overflow, it's not obvious at the first read. Does the Python implementation of pickle produce BINBYTES8? If not: why not? Note: the patch is probably based on a private Mercurial revision, so it didn't get the [Review] button. |
i is small integer between 0 and 8. I don't think that this article is
Done.
Yes, it produces BINBYTES8 on 64-bit platform.
Yes, I had made some refactoring for unpickling tests and forgot to push they. |
New changeset d4f8316d0860 by Serhiy Storchaka in branch '3.4': New changeset da9ad20dd470 by Serhiy Storchaka in branch '3.5': New changeset 8de1967edfdb by Serhiy Storchaka in branch 'default': |
Thank you Victor and Antoine for your reviews. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: