strukturag / libheif

libheif is an HEIF and AVIF file format decoder and encoder.
Other
1.69k stars 296 forks source link

crash during HEIC encoding on MSYS2 platform #357

Open novomesk opened 3 years ago

novomesk commented 3 years ago

When libheif master is built on MSYS2 platform (Windows), heif-enc and GIMP plug-in crashes when encoding HEIC files. 8-bit AVIF encoding works.

(gdb) run
Starting program: C:\msys64\mingw64\bin\heif-enc.exe genessa_600_450.jpg x.heic

Program received signal SIGSEGV, Segmentation fault.
0x000007fefda85b80 in strlen () from C:\Windows\system32\msvcrt.dll
(gdb) bt
#0  0x000007fefda85b80 in strlen () from C:\Windows\system32\msvcrt.dll
#1  0x000000006ecc5cea in x265_plugin_name ()
    at C:/msys64/home/Daniel/libheif/libheif/heif_encoder_x265.cc:190
#2  0x000000006eca58e7 in heif::HeifContext::encode_image (this=0x19b9830,
    pixel_image=std::shared_ptr<heif::HeifPixelImage> (use count 2, weak count 1) = {...}, encoder=encoder@entry=0x19ba050, options=options@entry=0x1a244c0,
    input_class=input_class@entry=heif_image_input_class_normal,
    out_image=std::shared_ptr<heif::HeifContext::Image> (empty) = {...})
    at C:/msys64/mingw64/include/c++/10.2.0/ext/new_allocator.h:79
#3  0x000000006ec9950d in heif_context_encode_image (ctx=ctx@entry=0x19b97e0,
    input_image=0x19ba400, encoder=0x19ba050,
    options=options@entry=0x1a244c0,
    out_image_handle=out_image_handle@entry=0x22fc68)
    at C:/msys64/mingw64/include/c++/10.2.0/ext/atomicity.h:97
#4  0x0000000000404105 in main (argc=3, argv=0x1a8af20)
    at C:/msys64/mingw64/include/c++/10.2.0/bits/shared_ptr_base.h:1324

Other report that problems are since 1.8.0. 1.7.0 worked fine according the reports. Related issue: https://gitlab.gnome.org/GNOME/gimp/-/issues/5749

novomesk commented 3 years ago

This line: https://github.com/strukturag/libheif/blob/b6a90de9e4667ea4ff81a97a01b02ef9ed3307b4/libheif/heif_encoder_x265.cc#L190

lillolollo commented 3 years ago

Detailed bt

$ gdb heif-enc.exe
GNU gdb (GDB) 9.2
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-w64-mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from heif-enc.exe...
(gdb) run portrait_8rot.jpg out.heic
Starting program: C:\msys64\mingw64\bin\heif-enc.exe portrait_8rot.jpg out.heic
[New Thread 2040.0x998]
[New Thread 2040.0x1ed0]
[New Thread 2040.0xe34]

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffe519cd700 in strlen () from C:\Windows\System32\msvcrt.dll
(gdb) bt full
#0  0x00007ffe519cd700 in strlen () from C:\Windows\System32\msvcrt.dll
No symbol table info available.
#1  0x00007ffe1c286b0a in x265_plugin_name ()
    at ../../libheif-1.9.1/libheif/heif_encoder_x265.cc:190
No locals.
#2  0x00007ffe1c26d5f7 in heif::HeifContext::encode_image (
    this=0x18dee20a050,
    pixel_image=std::shared_ptr<heif::HeifPixelImage> (use count 2, weak count 1) = {...}, encoder=encoder@entry=0x18dee20a870,
    options=options@entry=0x18dee4feb80,
    input_class=input_class@entry=heif_image_input_class_normal,
    out_image=std::shared_ptr<heif::HeifContext::Image> (empty) = {...})
    at C:/msys64/mingw64/include/c++/10.2.0/ext/new_allocator.h:79
        error = {error_code = heif_error_Ok,
          sub_error_code = heif_suberror_Unspecified, message = "",
          static Ok = {error_code = heif_error_Ok,
            sub_error_code = heif_suberror_Unspecified, message = "",
            static Ok = <same as static member of an already seen type>,
            static kSuccess = 0x7ffe1c2aa4f0 <heif::Error::kSuccess> "Success"}, static kSuccess = <same as static member of an already seen type>}
#3  0x00007ffe1c265766 in heif_context_encode_image (ctx=0x18dee20a000,
    input_image=0x18dee20ac20, encoder=0x18dee20a870, options=0x18dee4feb80,
    out_image_handle=0x3967dff808)
    at C:/msys64/mingw64/include/c++/10.2.0/ext/atomicity.h:97
        default_options = {version = 0 '\000', save_alpha_channel = 0 '\000'}
        image = std::shared_ptr<heif::HeifContext::Image> (empty) = {
          get() = 0x0}
        error = {error_code = heif_error_Ok,
          sub_error_code = heif_suberror_Unspecified, message = "",
          static Ok = {error_code = heif_error_Ok,
            sub_error_code = heif_suberror_Unspecified, message = "",
            static Ok = <same as static member of an already seen type>,
            static kSuccess = 0x7ffe1c2aa4f0 <heif::Error::kSuccess> "Success"}, static kSuccess = <same as static member of an already seen type>}
#4  0x00007ff715e04bb0 in main (argc=3, argv=0x18dee209e80)
    at C:/msys64/mingw64/include/c++/10.2.0/bits/shared_ptr_base.h:1324
        input_filename = "portrait_8rot.jpg"
        suffix_pos = <optimized out>
        filetype = <optimized out>
        image = warning: RTTI symbol not found for class 'std::_Sp_counted_deleter<heif_image*, loadJPEG(char const*)::{lambda(heif_image*)#1}, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>'
warning: RTTI symbol not found for class 'std::_Sp_counted_deleter<heif_image*, loadJPEG(char const*)::{lambda(heif_image*)#1}, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>'
std::shared_ptr<heif_image> (use count 1, weak count 0) = {
          get() = 0x18dee20ac20}
        handle = 0x18d14070013
        suffix = "jpg"
        nclx = {version = 160 '▒',
          color_primaries = heif_color_primaries_unspecified,
          transfer_characteristics = heif_transfer_characteristic_unspecified, matrix_coefficients = heif_matrix_coefficients_ITU_R_BT_601_6,
          full_range_flag = 1 '\001', color_primary_red_x = 7.39612789e+31,
          color_primary_red_y = 7.49635208e+28,
          color_primary_green_x = 1.62550622e-43,
          color_primary_green_y = 1.40129846e-45, color_primary_blue_x = 0,
          color_primary_blue_y = 9.4503723e+10,
          color_primary_white_x = 4.59149455e-41,
          color_primary_white_y = -1.60214972e+28}
        options = 0x18dee4feb80
        quality = 50
        lossless = false
        output_filename = "portrait_8rot.heic"
        logging_level = 0
        option_show_parameters = false
        thumbnail_bbox_size = 0
        output_bit_depth = 10
        enc_av1f = false
        crop_to_even_size = false
        raw_params = std::vector of length 0, capacity 0
        context = warning: RTTI symbol not found for class 'std::_Sp_counted_deleter<heif_context*, main::{lambda(heif_context*)#1}, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>'
warning: RTTI symbol not found for class 'std::_Sp_counted_deleter<heif_context*, main::{lambda(heif_context*)#1}, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>'

std::shared_ptr<heif_context> (use count 1, weak count 0) = {
          get() = 0x18dee20a000}
        encoder = 0x18dee20a870
        encoder_descriptors = {0x18dee4f9c20, 0x14070013, 0x100,
          0x7ffe51b4875b <ntdll!RtlpNtMakeTemporaryKey+21227>, 0x18dee4f0000}
        count = <optimized out>
        error = <optimized out>
(gdb)
kleisauke commented 3 years ago

The problem is that it segfaults on strlen(NULL), which means that x265_version_str is incorrectly defined as NULL.

Assuming x265 is built as DLL; try to define X265_API_IMPORTS while building libheif to make sure this symbol is imported correctly. You could also add this to the cflags within the .pc file of x265 (see for example here).

If that doesn't work, you might also need to apply this patch to correctly annotate x265_version_str as data symbol within the module-definition (.def file). AFAIK this is only needed on llvm-mingw.

lillolollo commented 3 years ago

@kleisauke or maybe, since x265 seem actively developed, open e issue or propose a patch upstream to fix definitely this issue could be the best choice

lillolollo commented 3 years ago

Thanks @kleisauke fixed on mingw https://github.com/msys2/MINGW-packages/pull/7100/commits/31a11d8ce3c7420336c7be85cc20a2d1ba450a9a

farindk commented 3 years ago

Thanks for investigating this. If I'd want to add the X265_API_IMPORTS define to the libheif build files, for which systems does this have to be defined? Blindly adding it non-conditionally does not work as compilation on Linux fails when this is added.

jeremyd2019 commented 3 years ago

I think you may be on the wrong track thinking that x265_version_str is NULL. With the .def file missing the DATA annotation, and without dllimport, the linker was providing the symbol x265_version_str pointing to code that had a jmp to the address of x265_version_str in the dll.

As far as for which systems - Windows.

kleisauke commented 3 years ago

Ah, you're right. Sorry for giving the wrong conclusion. Is there a workaround to detect this bug in libheif (similar to https://github.com/strukturag/libheif/commit/443e2082920855f1e6d27d02413fe71ff9bb0709)?

Note that X265_API_IMPORTS should only be defined for shared x265 Windows builds. I don't think it will work when x265 is statically-linked. A better way would be to resolve this upstream in the package's .pc file.

farindk commented 3 years ago

@jeremyd2019 Yes, I know that 443e208 does not solve the root of the problem, but it's not bad to check for NULL anyway.

Is it for all Windows shared builds? I mean, there is gcc, clang, VC++, mingw cross-builds, MSYS, cygwin, ...

Best would definitely be that this define is included upstream in x265, but until this is available, we might need to add it ourselves.

jeremyd2019 commented 3 years ago

Is it for all Windows shared builds? I mean, there is gcc, clang, VC++, mingw cross-builds, MSYS, cygwin, ...

Yes, I think so. Not 100% sure about cygwin, but I don't think it would hurt there either.

Best would definitely be that this define is included upstream in x265, but until this is available, we might need to add it ourselves.

They can't provide it, because then the headers are different for shared or static libraries, and that is its own mess (that I just had to deal with with libgmp). Best would be if they added that patch for the .def file upstream. That would allow ld to get things right even without dllimport.