Vips.block seems to not work as expected

[ClearlyClaire]

Hi!

I'm not sure whether this is a bug, but Vips.block does not seem to work as intended.

Indeed, our codebase uses Vips.block('VipsForeign', true) then Vips.block('VipsForeignLoadJpeg', false) and others to re-enable a specific list of loaders, but a user reports this is broken on their deployment: mastodon/mastodon#33809 (comment)

According to them, they are unable to process JPEG files when Vips.block('VipsForeignLoadJpeg', false) is called, even when not calling Vips.block('VipsForeign', true).

I am not sure what is going there here.

[jcupitt]

Hi @ClearlyClaire, that's odd, I'll have a look.

[jcupitt]

I tried a test program:

#!/usr/bin/ruby

require "vips"

# block all loaders and savers
Vips.block("VipsForeign", true)

# reenable jpeg load
Vips.block("VipsForeignLoadJpeg", false)
    
image = Vips::Image.new_from_file(ARGV[0])
puts "image is #{image.width} x #{image.height} pixels"

I see:

john@orange ~/try $ ./block.rb ~/pics/k2.jpg
image is 1450 x 2048 pixels
john@orange ~/try $ ./block.rb ~/pics/k2.png
/home/john/packages/gems/gems/ruby-vips-2.2.2/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "/home/john/pics/k2.png" is not a known file format (Vips::Error)
	from ./block.rb:11:in `<main>'

So it seems to work for me (ruby 3.2, ruby-vips 2.2, libvips 8.16).

You'll need to find a way to reproduce this error. Is it possible to get more information from the user?

[ClearlyClaire]

I'll tag them here!

@17dec can you try running the above script, see how it behaves, and give us any relevant information?

[17dec]

Interesting, I'm getting unreliable results when running that script:

$ bundle exec ./test.ruby ./spec/fixtures/files/attachment.jpg
image is 600 x 400 pixels
$ bundle exec ./test.ruby ./spec/fixtures/files/attachment.jpg
$REDACTED/bundle/ruby/3.2.0/gems/ruby-vips-2.2.2/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "./spec/fixtures/files/attachment.jpg" is not a known file format (Vips::Error)
        from ./test.ruby:11:in `<main>'
$ bundle exec ./test.ruby ./spec/fixtures/files/attachment.jpg
$REDACTED/bundle/ruby/3.2.0/gems/ruby-vips-2.2.2/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "./spec/fixtures/files/attachment.jpg" is not a known file format (Vips::Error)
        from ./test.ruby:11:in `<main>'
$ bundle exec ./test.ruby ./spec/fixtures/files/attachment.jpg
image is 600 x 400 pixels

Valgrind confirms there's an uninitialized read:

==14191== Conditional jump or move depends on uninitialised value(s)
==14191==    at 0xCBA9683: vips_operation_block_set_operation (in /usr/lib64/libvips.so.42.18.0)
==14191==    by 0xCB9AB4E: vips_class_map_all (in /usr/lib64/libvips.so.42.18.0)
==14191==    by 0xCB9AA8F: vips_type_map (in /usr/lib64/libvips.so.42.18.0)
==14191==    by 0xCBABCA4: vips_operation_block_set (in /usr/lib64/libvips.so.42.18.0)
==14191==    by 0x56B7ABD: ffi_call_unix64 (in /usr/lib64/libffi.so.8.1.4)
==14191==    by 0x56B6DBD: ffi_call_int (in /usr/lib64/libffi.so.8.1.4)
==14191==    by 0x56B75AA: ffi_call (in /usr/lib64/libffi.so.8.1.4)
==14191==    by 0xCA21028: rbffi_CallFunction (in $REDACTED/bundle/ruby/3.2.0/gems/ffi-1.16.3/lib/ffi_c.so)
==14191==    by 0xCA252F5: custom_trampoline (in $REDACTED/bundle/ruby/3.2.0/gems/ffi-1.16.3/lib/ffi_c.so)
==14191==    by 0x4AB655C: vm_call_cfunc_with_frame (in /usr/lib64/libruby32.so.3.2.5)
==14191==    by 0x4AC456F: vm_exec_core (in /usr/lib64/libruby32.so.3.2.5)
==14191==    by 0x4AC98CD: rb_vm_exec (in /usr/lib64/libruby32.so.3.2.5)

(Although I'm not sure how reliable that message is, it's also showing a lot of uninitialized reads within ruby itself. Those are all in the garbage collector, it looks like, so maybe that's to be expected)

Not really sure how that happens. FFI confusion over C 'bool' vs. 'gboolean'? Then again, I kind of expect both to expand to an 'int' in terms of ABI.

[17dec]

Your c version works reliably and valgrind output is nice and clean on my system as well.

$ ./block test.jpg
width = 512
$ ./block test.png
VipsForeignLoad: "test.png" is not a known file format

I only have a single libvips binary:

$ find / -xdev -name 'libvips*.so'
/usr/lib64/libvips-cpp.so
/usr/lib64/libvips.so

I rebuilt ruby with valgrind instrumentation (just found out Gentoo offers that option), and with that valgrind is almost completely silent on the ruby test script. The unconditional read in vips_operation_block_set_operation is the one thing that remains.

Some gentoo package versions & configuration:

media-libs/vips-8.16.0::gentoo was built with the following:
USE="heif highway jpeg jpeg2k jpegxl lcms png webp -archive -deprecated -doc -exif -fftw -fits -fontconfig -graphicsmagick -gtk-doc -imagemagick -imagequant -introspection -matio -openexr -orc -pango -pdf -python -svg -test -tiff -vala" ABI_X86="(64)" PYTHON_SINGLE_TARGET="python3_11 -python3_10 -python3_12 -python3_13"

dev-libs/libffi-3.4.6-r2::gentoo was built with the following:
USE="-debug -exec-static-trampoline -pax-kernel -static-libs -test" ABI_X86="32 (64) (-x32)"

dev-lang/ruby-3.2.5-r2::gentoo was built with the following:
USE="ssl valgrind -berkdb -debug -doc -examples -gdbm (-jemalloc) -jit -socks5 (-static-libs) -systemtap -tk -xemacs" ABI_X86="(64)"
CFLAGS="-O2 -pipe -march=native -mtune=native -fno-strict-aliasing"
CXXFLAGS="-O2 -pipe -march=native -mtune=native -fno-strict-aliasing"

sys-devel/gcc-14.2.1_p20241221::gentoo was built with the following:
USE="cet (cxx) (default-stack-clash-protection) (default-znow) fortran (multilib) openmp (pie) sanitize ssp zstd -ada (-custom-cflags) -d -debug -doc (-fixed-point) -go -graphite -hardened (-ieee-long-double) -jit (-libssp) -lto -modula2 -nls -objc -objc++ -objc-gc (-pch) -pgo -rust -systemtap -test (-time64) -valgrind -vanilla -vtv" ABI_X86="(64)"
CFLAGS="-pipe -march=native -mtune=native -O2"
CXXFLAGS="-pipe -march=native -mtune=native -O2"

sys-libs/glibc-2.40-r8::gentoo was built with the following:
USE="cet multiarch (multilib) ssp stack-realign (static-libs) -audit -caps -compile-locales (-custom-cflags) -doc -gd -hash-sysv-compat -headers-only -multilib-bootstrap -nscd -perl -profile (-selinux) -suid -systemd -systemtap -test (-vanilla)" ABI_X86="(64)"

I rebuilt ruby, vips and libffi just in case gcc/glibc/anything-else-important has changed since they were last built, but that didn't change anything.

Another observation: it seems the uninitialized read is only triggered when 'false' is passed as second argument. No problems are reported when the second argument is 'true'.

[kleisauke]

FFI confusion over C 'bool' vs. 'gboolean'? Then again, I kind of expect both to expand to an 'int' in terms of ABI.

ruby-vips uses int for other gboolean function definitions, since that is a typedef for int in GLib.

ruby-vips/lib/vips/gvalue.rb

Line 97 in 7a2b9ba

::GObject.g_value_set_boolean self, (value ? 1 : 0)

ruby-vips/lib/vips/gvalue.rb

Line 264 in 7a2b9ba

attach_function :g_value_set_boolean, [GValue.ptr, :int], :void

https://github.com/GNOME/glib/blob/2.83.3/glib/gtypes.h#L56

To rule things out, are you able to test commit kleisauke@ec69676?

[17dec]

To rule things out, are you able to test commit kleisauke@ec69676?

Can confirm that that commit fixes the problem. With that, the script works reliably and no uninitialized reads are reported.

[kleisauke]

To rule things out, are you able to test commit kleisauke@ec69676?

Can confirm that that commit fixes the problem. With that, the script works reliably and no uninitialized reads are reported.

Great! I just opened PR #411 for this with a polished commit message and added changelog note.

I'm not sure why I can't reproduce this locally, you would expect this to cause issues on other platforms too.

[jcupitt]

Well done Kleis! You'd think ruby-ffi would pass bools as ints, that's very unexpected.

[jcupitt]

So that's just bizarre. Let's do ruby-vips 2.2.3 with this fix ASAP.

[jcupitt]

OK, 2.2.3 with this fix is up. Thank you Kleis! And thanks for the report and feedback @ClearlyClaire and @17dec

[kleisauke]

No problem! Could you also push commit 9fb56ce to the master branch? I noticed v2.2.3 was based on that, but the commit itself is not (yet) present on the main branch.

[jcupitt]

Oops! Sorry. I always expect git push --tags to push code as well.