Skip to content

Conversation

@r-barnes
Copy link
Contributor

@r-barnes r-barnes commented Feb 27, 2024

Fixes

libImaging/Convert.c:513:25: error: signed shift result (0xFF000000) sets the sign bit of the shift expression's type ('int') and becomes negative [-Werror,-Wshift-sign-overflow]
    UINT32 trns = (0xff << 24) | ((b & 0xff) << 16) | ((g & 0xff) << 8) | (r & 0xff);
                   ~~~~ ^  ~~

Fixes
```
libImaging/Convert.c:513:25: error: signed shift result (0xFF000000) sets the sign bit of the shift expression's type ('int') and becomes negative [-Werror,-Wshift-sign-overflow]
    UINT32 trns = (0xff << 24) | ((b & 0xff) << 16) | ((g & 0xff) << 8) | (r & 0xff);
                   ~~~~ ^  ~~
```
@radarhere
Copy link
Member

Hi. A quick glance through some of our CI jobs, and I don't see this problem on main. Could you provide some more information about the environment where you're seeing this?

@r-barnes
Copy link
Contributor Author

@radarhere - We compile pillow using a separate build system to get better LTO.

This build system has -Wshift-sign-overflow enabled for code safety reasons (it catches bugs!). It's therefore advantageous to have as much upstream code as possible be able to compile safely.

You could enable -Wshift-sign-overflow in pillow, though I don't know how to do this myself, but, in the meantime, godbolt demonstrates the failure indicated above.

@radarhere radarhere merged commit 38cec87 into python-pillow:main Mar 9, 2024
nulano added a commit to nulano/Pillow that referenced this pull request Mar 9, 2024
@nulano
Copy link
Contributor

nulano commented Mar 9, 2024

You could enable -Wshift-sign-overflow in pillow, though I don't know how to do this myself,

Generally, compiler flags specific to CI are listed here

CFLAGS="-coverage -Werror=implicit-function-declaration" python3 -m pip -v install .

but it seems this warning is not supported by GCC, only Clang.

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

Successfully merging this pull request may close these issues.

3 participants