GNOME Bugzilla – Bug 648265
tile cache size larger than 2G causes core dump on opening preferences
Last modified: 2012-09-18 18:14:52 UTC
If you use edit->preferences to set the tile cache to a value larger than 2GBytes, there's a console warning, (gimp-2.7:17948): GLib-GObject-WARNING **: value "-2147483648" of type `gint' is invalid or out of range for property `cache-size' of type `gint' (I used 6G) If gimprc contains a value bigger than 2GBytes, e.g. I had (tile-cache-size 6G) there in mine which worked fine git master of a month ago, then when you start gimp you see the same warning, and edit->preferences dumps core.
app/gegl/gimp-gegl.c does this g_object_set (gegl_config (), "tile-width", TILE_WIDTH, "tile-height", TILE_HEIGHT, "cache-size", config->tile_cache_size, NULL); but cache-size in gegl is a gint, while tile_cache_size in gimp is a guint64. I just get the warning and no crash fwiw.
commit 220d06324d9870d2dc151279263eddb4474f9ab2 Author: Massimo Valentini <mvalentini@src.gnome.org> Date: Tue Jul 26 18:44:16 2011 +0200 Bug 648265 - tile cache size larger than 2G causes... limit gegl cache-size to its maximum value otherwise modulo arithmetic causes Warnings or small values
Ok to clarify: this was a bug in GIMP and a limitation in GEGL. The bug is that a guint64 was passed to a variadic function that was expecting a gint. This means that the most significant uint32_t was checked as the NULL terminator/name of the following property, and obviously when not 0 dereferencing it is likely to crash. But even when it is zero, that is a cache-size between 2 and 4 Gb, gegl's cache-size was silently and severely reduced. Reopening to remember that the cache-size set in preferences is correct only for GIMP's tile cache
So is the warning when opening preferences still there or not? And shouldn't gegl be fixed too?
I don't see neither warnings nor core dumps. Fixing GEGL means touching its public interface, that is simply changing GeglConfig:cache-size type to gint64 would silently break correct code like: g_object_set (config, "cache-size", 512 * 1024 * 1024, NULL); OTOH, prior to that, you need to ensure that in GEGL code all cache-file offset arithmetic is using 64 bits types and expressions. But GEGL code is little used, little exercised and full of bugs and inconsistencies. Looking at gegl/gegl-config.c shows that properties are declared with a default value that is not what is used as default. cache-size is installed to have a default of 512*1024*1024, but it is initialized with 256*1024*1024 chunk-size 256*300 -> 512*512 tile-width 64 -> 128 Looking at function 'read_block' in gegl/buffer/gegl-buffer-load.c you can see that it is completely missing whatever error-handling, the comment before the function suggests offset could be NULL, first thing it is dereferenced, second it is checked. If you follow whatever failure execution path, say !(block.flags == GEGL_FLAG_FREE_TILE || block.flags == GEGL_FLAG_TILE) -> own_size = 0 -> g_malloc returns NULL and memcpy will copy there sizeof (GeglBufferBlock) bytes ... Fixing GEGL is not easy.
Gegl could introduce a cache-size-64 property. Since GIMP is apparently fixed, i'm moving this bug to GEGL now. Thanks.
I cleaned the double init of the properties in gegl-config.
This should be fixed now, the last bit was broken in GIMP: commit 52af6e3f3f67d37aa72d06a5f60b1a3967d6c297 Author: Michael Natterer <mitch@gimp.org> Date: Tue Sep 18 20:07:13 2012 +0200 app: fix the code that sets the 64bit tile cache size on GeglConfig app/gegl/gimp-gegl.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
For reference, the GEGL bit was fixed in: commit ae2110b993db8b17749555734d39e3c5ff7c0762 Author: Michael Natterer <mitch@gimp.org> Date: Fri Jun 29 01:12:33 2012 +0200 gegl: switch tile cache size handling to 64 bit and add a new GeglConfig property "tile-cache-size" of type guint64, "cache-size" still exists but is deprecated and will go away in gegl-0.3.