Skip to content
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

archive: switch offsets and lengths to unsigned 32-bit integers #5667

Merged
merged 1 commit into from Mar 25, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Mar 24, 2021

Switch the offsets and lengths in CTOC and CArchive python data structures from signed 32-bit integers to unsigned 32-bit integers. Adjust the field definitions in bootloader's CArchive reader code accordingly.

Increases the maximum allowed CArchive size from 2 GiB to 4 GiB.

Fixes #3939.

Switch the offsets and lengths in CTOC and CArchive python data
structures from signed 32-bit integers to unsigned 32-bit integers.
Adjust the field definitions in bootloader's CArchive reader code
accordingly.

Increases the maximum allowed CArchive size from 2 GiB to 4 GiB.
@rokm
Copy link
Member Author

rokm commented Mar 24, 2021

This is a "lite" branch, which does not include tests. Those were useful for development and local testing, but on the CI they would need to remain disabled anyway (due to storage requirements, which could be further exacerbated due to potential timeouts).

The full branch is here, if anyone's interested: https://github.com/rokm/pyinstaller/tree/4gb-archive.
(The large-data tests are disabled by default, and need to be explicitly enabled on command-line).

@bwoodsend
Copy link
Member

I'm guessing that, during the period between merging this and our next rebuilding the bootloaders, we aren't going to experience issues due to mismatch between Python using uint32 and C still using int32 simply because the two types look identical provided you stay in the range of 0 to 231?

@rokm
Copy link
Member Author

rokm commented Mar 24, 2021

I'm guessing that, during the period between merging this and our next rebuilding the bootloaders, we aren't going to experience issues due to mismatch between Python using uint32 and C still using int32 simply because the two types look identical provided you stay in the range of 0 to 231?

Until bootloaders are rebuilt, the difference in behavior will be that the python code will happily pack the large CArchive, but the old bootloader will misinterpret the large offsets/sizes as negative. Which means that the error will change from being built-time to being run-time. If that's unacceptable, we can delay merging this until we're ready for new release.

EDIT: for CArchives smaller than 2 GiB, the behavior should be unchanged, as the change in representation involves only the most-significant bit.

@bwoodsend
Copy link
Member

EDIT: for CArchives smaller than 2 GiB, the behavior should be unchanged, as the change in representation involves only the most-significant bit.

That's what I wanted to hear.

@bwoodsend bwoodsend merged commit 937d8a3 into pyinstaller:develop Mar 25, 2021
@rokm rokm deleted the 4gb-archive-lite branch May 5, 2021 09:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error for binaries larger than 2Gb
2 participants