syoyo / tinyexr

Tiny OpenEXR image loader/saver library
703 stars 138 forks source link

UBSan issue when loading an .exr #187

Open jdumas opened 1 year ago

jdumas commented 1 year ago

Describe the issue Loading a certain .exr image with UBSan enabled flags some issues in tinyexr.

To Reproduce Steps to reproduce the behavior:

  1. Compile TinyEXR with UBSan (-fsanitize=undefined)
  2. Load EXR Image with LoadEXRImageFromFile
  3. See error
    tinyexr/tinyexr.h:1999:12: runtime error: left shift of 585757245565577410 by 8 places cannot be represented in type 'long long'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tinyexr/tinyexr.h:1999:12 in
    tinyexr/tinyexr.h:2659:5: runtime error: left shift of 769045763242690055 by 8 places cannot be represented in type 'long long'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tinyexr/tinyexr.h:2659:5 in
    tinyexr/tinyexr.h:2735:33: runtime error: left shift of 115287495861464541 by 8 places cannot be represented in type 'long long'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior tinyexr/tinyexr.h:2735:33 in

Image: plastic_stripes_Height.exr.zip

Expected behavior I am not sure whether it's an issue with our .exr image, or a problem in tinyexr, but I'd expect tinyexr to not trigger any runtime UBSan error (this one seems like a shift integer overflow?).

Environment

syoyo commented 1 year ago

These UBSan reported line of codes points to codes ported from OpenEXR(not written by me) and somewhat difficult to understand such codes. The reason would be integer overflows. But I'm not sure simple overflow fix is sufficient. It may introduce some side-effects.

Proper solution would be replace OpenEXR-derived codes with our own https://github.com/syoyo/tinyexr/issues/186

For a while, we can simply ignore/suppress UBSan warning: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-undefined

jdumas commented 1 year ago

Ah the wuffs thing sounds nice. Guess I'll just ignore these issues for now, thanks!