Crash with -f sixels

[chrisant996]

Repro steps:

• Using chafa v1.14.2
• Download the photo "911 Row - redacted.jpg" from below (a row of cars in a parking garage)
• Run chafa -f sixels "911 Row - redacted.jpg"
• Crash

I'm able to reproduce crashes with -f sixels on many different input .jpg files, but also many other input .jpg files do not crash.

The input photo:

[hpjansson]

Thanks for reporting this. I can't repro so far, and ASAN/Valgrind are coming up empty-handed. Could you please specify:

• Which platform you're on (e.g. Linux distro, MacOS or Windows version).
• CPU arch (especially if not Intel).
• How you obtained Chafa (e.g. package manager, official binary, built from source..).

[chrisant996]

Oh that's interesting...

Field	Value
Platform	Windows 11 Pro, 23H2, 22631.4037
CPU	Intel Core Ultra 9 185H 2.30 Ghz
Arch	x64
Obtained	winget install hpjansson.Chafa
Version	Chafa version 1.14.2

Here is a minidump without heap:
chafa_dump_no_heap.zip

I'm working on finding a good way to upload a dump with heap, but it's 26 MB when compressed with zip deflate level 9, and GitHub only allows <= 25 MB.

[chrisant996]

Rar compresses it to 19 MB, but GitHub doesn't accept .rar files.

So, here is a .rar file, which I've renamed to .zip to placate GitHub's filter.
[Chafa_sixel_crash___with_heap.zip] -- removed to reclaim space, and because it wasn't useful without symbols.

To extract it, download it and rename it to .rar -- I tried that and on my machine it was able to successfully download and extract the file.

[j4james]

You may want to double check that the test image you pasted here actually matches your local copy. I think it's quite possible that github recompresses images that it hosts, and if that's the case, the github version wouldn't necessarily reproduce the crash that you're seeing on your local copy.

[chrisant996]

You may want to double check that the test image you pasted here actually matches your local copy. I think it's quite possible that github recompresses images that it hosts, and if that's the case, the github version wouldn't necessarily reproduce the crash that you're seeing on your local copy.

When I right-click on the image and download it, it comes down byte for byte identical to the source image.

And I'm able to repro the crash when using the downloaded copy.

The two dump files I uploaded should be useful in debugging, without needing a local repro.

[hpjansson]

Thanks, that helped a lot. I was able to take a peek with WinDbg and confirmed it's crashing on a vmovdqa instruction with bad data alignment (16 bytes where 32 is needed). I can repro this with the Windows binary, but not with any of the Linux builds.

Since I'm cross-compiling with gcc, I don't have a PDB, so no backtrace yet -- but there aren't too many candidates to check manually.

[chrisant996]

Anything further I can do to help?

I'm also happy to test a pre-release prototype fix.

[hpjansson]

Can't think of anything right now, but might take you up on the testing offer once I have something running here.

[chrisant996]

Is it a case of gcc bug 15795?
Might C++17 or -faligned-new help? (See comment 45 in bug 15795.)

[hpjansson]

I doubt it -- the code I'm suspicious of is just plain C, and heap allocations are aligned manually to 64 bytes. It's most likely just doing _mm256_store_si256() through an unaligned pointer somewhere. Probably in smolscale-avx2.c.

The asm looks familiar and could match in a couple of places. I just have to narrow it down.

[hpjansson]

Figured it out. It's actually gcc bug 54412. We have code that looks like this:

__m256i func_b (void) {
    __m256i v;
    ...
    return v;
}

void func_a (void) {
    __m256i u;
    u = func_b ();
    ...
}

func_b() tries to pass the return value on the stack by storing it with vmovdqa. This requires 32-byte alignment. However, the stack pointer is not necessarily aligned, even though the local variables are. Changing the code to this fixes the problem:

void func_b (__m256i *o) {
    __m256i v;
    ...
    *o = v;
}

void func_a (void) {
    __m256i u;
    func_b (&u);
    ...
}

I'm attaching a test binary wrapped in a zip file. It works for me -- @chrisant996, care to test?

chafa-test.zip

[chrisant996]

Bingo! 😎💪

Screenshot for visual confirmation, in Windows Terminal Canary 1.22.240815001-llm:

[veltza]

I tried chafa-test.zip on my Windows 10 machine with a nightly build of Windows Terminal Canary and noticed that the -s option doesn't work properly. It prints a sixel that's way too small.

[chrisant996]

I tried chafa-test.zip on my Windows 10 machine with a nightly build of Windows Terminal Canary and noticed that the -s option doesn't work properly. It prints a sixel that's way too small.

That doesn't seem related to the crash.

Have you considered opening a separate issue? That's probably better than appending to an unrelated issue.

[hpjansson]

Thanks @veltza -- I think it's a real issue, but I suspect it's not a Chafa bug, but rather the terminal misreporting its pixel dimensions. Please open a new issue so we can investigate :-)

[veltza]

Have you considered opening a separate issue? That's probably better than appending to an unrelated issue.

Thanks @veltza -- I think it's a real issue, but I suspect it's not a Chafa bug, but rather the terminal misreporting its pixel dimensions. Please open a new issue so we can investigate :-)

Sure, I'll do that. I apologize for disturbing you here.

[hpjansson]

Thanks for testing!