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

x86 WebP DLL wrongly assumes stack alignment #145

Open
slipher opened this Issue Dec 25, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@slipher
Copy link
Contributor

slipher commented Dec 25, 2018

The libwebp-5.dll shipped in the msvc32-4 external_deps seems to be actually incompatible with MSVC. On the x86 Windows platform, only 4-byte stack alignment is guaranteed. But the code in this DLL apparently assumes that the stack will be 16-byte aligned when using SSE instructions on stack argument (causing it to crash).

I tried to find a compile switch or attribute to force 16-byte stack alignment, but no luck.

Where does this code come from anyway? Is it built by us or from a binary release?

@DolceTriade

This comment has been minimized.

Copy link
Contributor

DolceTriade commented Dec 25, 2018

@slipher

This comment has been minimized.

Copy link
Contributor

slipher commented Dec 26, 2018

We would like to imagine that C has a stable, interoperable between compilers ABI, but apparently that's not quite the case... mingw gcc assumes the caller aligns the stack to 16 bytes, so it may be unsafe to call from MSVC-compiled code into GCC-compiled code.

It seems like a mingw-specific bug that SSE instructions can be generated in the "i686" architecture. The [GCC manual] suggests that SSE should not be available, and GCC on Linux refuses to compile SSE intrinsics.

To fix this on the Webp DLL end, we could try to build it either with SSE instructions disabled, or with -mpreferred-stack-boundary=2 to change the assumed stack alignment to 4 bytes.

@slipher

This comment has been minimized.

Copy link
Contributor

slipher commented Jan 8, 2019

#151 should make the external_deps build script build compatible DLLs, so the issue will be fixed whenever a new version of the external_deps is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment