libheif icon indicating copy to clipboard operation
libheif copied to clipboard

crash during HEIC encoding on MSYS2 platform

Open novomesk opened this issue 4 years ago • 10 comments

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 avatar Oct 11 '20 11:10 novomesk

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

novomesk avatar Oct 11 '20 11:10 novomesk

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)

lillolollo avatar Oct 11 '20 12:10 lillolollo

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.

kleisauke avatar Oct 11 '20 17:10 kleisauke

@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 avatar Oct 11 '20 18:10 lillolollo

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

lillolollo avatar Oct 11 '20 21:10 lillolollo

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.

farindk avatar Oct 12 '20 09:10 farindk

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.

jeremyd2019 avatar Oct 13 '20 03:10 jeremyd2019

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.

kleisauke avatar Oct 13 '20 06:10 kleisauke

@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.

farindk avatar Oct 13 '20 09:10 farindk

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.

jeremyd2019 avatar Oct 13 '20 19:10 jeremyd2019