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

pngdetail -r … crashes #198

Open
chris0e3 opened this issue Mar 2, 2025 · 0 comments
Open

pngdetail -r … crashes #198

chris0e3 opened this issue Mar 2, 2025 · 0 comments

Comments

@chris0e3
Copy link

chris0e3 commented Mar 2, 2025

Hello,

Thanks for all your work on LodePNG. It’s appreciated.

On macOS, compiled with Clang, pngdetail -r ‹PNG-file› always crashes when given a 256×256 or 512×512 RGBA-8 PNG file.
[It probably crashes on any power-of-two multiples of these sizes & many other sizes too. And any pixel format.]

I’ve tracked it down to the rescale function in pngdetail.cpp.
The function appears to always access 1 element beyond the end of the std::vector.
[You were often getting away with it but for these images the vector’s data size is an exact (large-ish) power-of-two probably resulting in a full virtual memory page being allocated, so the array[size+1] access causes a page fault/illegal address exception.]

Changing:

lodepng/pngdetail.cpp

Lines 470 to 471 in 0b1d9cc

for(int x0 = xa; x0 <= xb; x0++) {
int index0 = x0 * numchannels + y * w0 * numchannels;

lodepng/pngdetail.cpp

Lines 491 to 492 in 0b1d9cc

for(int y0 = ya; y0 <= yb; y0++) {
int index0 = x * numchannels + y0 * w1 * numchannels;

to

          for(int x0 = xa; x0 <= xb; x0++) {
            int index0 = x0 * numchannels + y * w0 * numchannels;
            if (size_t(index0) >= in.size()) continue;
...
          for(int y0 = ya; y0 <= yb; y0++) {
            int index0 = x * numchannels + y0 * w1 * numchannels;
            if (size_t(index0) >= temp.size()) continue;

appears to fix the problem. [BTW index0 should probably have been declared size_t.]

Also the std::vectors passed to & returned by rescale appear to always contain RGBA-16 big-endian images. However, only the high bytes of the RGBA components are used/needed. So you could effectively double the performance by changing:

for (int c = 0; c < numchannels; c++) {

to

    for (int c = 0; c < numchannels; c += 2) {

Also

size_t countColors(std::vector<unsigned char> image, unsigned w, unsigned h,

should probably be:

size_t countColors(const std::vector<unsigned char>& image, unsigned w, unsigned h,

And

if(options.rendersize >= 1 && options.rendersize <= 4096) options.rendersize = size;

should probably be:

        if(size >= 1 && size <= 4096) options.rendersize = size;

Finally adding data.loadInspect(); as the first line in:

void showInfos(Data& data, const Options& options) {

encourages pngdetail -s ‹file› & pngdetail -c ‹file› to print some output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant