search

erocarrera / pefile

pefile is a Python module to read and work with PE (Portable Executable) files
MIT License
1.86k stars 519 forks source link

256 MiB maximum offset limit is too low #396

Open randomascii opened 6 months ago

randomascii commented 6 months ago

The official dcheck enabled builds for Chrome are huge and have maximum section offsets of around 320 MB which is beyond the arbitrary hard-coded limit of 256 MiB in pefile.py. This causes symbol uploads to fail for these builds.

The limit should be increased to avoid this, and arguably the handling of the maximum should be changed so that an exception is thrown because the warning messages that are printed when invoking pefile.py are suppressed or otherwise hidden when using pefile.py as an import.

I have created a local change for Chrome that centralizes and increases the limit to 1 GiB.

randomascii commented 6 months ago

The change I made in Chromium's copy of pefile.py can be found here:

https://chromium-review.googlesource.com/c/chromium/src/+/5377931

It's pretty simple - I just centralized the maximum offset limit and then raised it. I could easily make a PR for this change for here, if that would be accepted.

Or, maybe the limit should just be removed - maybe it has served its purpose and no longer makes sense.

Or, maybe the limit should be a constructor parameter. I haven't checked to see how much work this would be but at a high level this would be a cleaner solution.

Finally, if the limit is reached then I think an exception should be thrown so that pefile.py doesn't fail in silent and mysterious ways.

Any thoughts on this before I create a PR?