deblk: avoid undefined left shift of negative chroma delta#136
deblk: avoid undefined left shift of negative chroma delta#136SABITHSAHEB wants to merge 1 commit into
Conversation
|
gentle ping |
|
In general issues of undefined behavior due to shifts are not prioritized. |
|
That's fair. I don't have a platform where it actually miscompiles without -fsanitize=undefined. Every compiler these targets use lowers (p - q) << 2 and (p - q) * 4 to the same instructions, so decoded output is identical everywhere today. It surfaced under UBSan on an ordinary decode, so for me it was about keeping the sanitizer baseline clean for fuzzing rather than a runtime bug on your supported platforms. If clean UBSan output isn't something you're tracking, I'm fine leaving the code as-is. Feel free to close this and #137. |
|
Thank you for the response above. |
2 participants
The chroma deblocking filter builds its sample correction from the difference of two neighbouring reconstructed samples and scales it with a left shift, (p - q) << 2. Those samples come straight out of the decoded picture, so the difference is a plain signed int that is negative whenever q is the larger of the two, and a left shift of a negative value is undefined in C. UBSan reports it on a normal decode, several times per frame, while the luma edge filter right beside it already forms the same quantity with multiplication and stays clean. The same shift was copied into every chroma edge routine, including the high bit depth and 4:2:2 builds in ihevc_hbd_deblk_edge_filter.c, so I changed them together. Swapping the shift for a multiply by 4 leaves the result unchanged for the cases that were already well defined and removes the undefined path; a before/after decode of the same stream produces byte-identical YUV. I confirmed the warnings disappear under -fsanitize=undefined using the bundled hevcenc/hevcdec.