After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 167604 - gimp_gradient_get_color_at() may return out-of-bounds values
gimp_gradient_get_color_at() may return out-of-bounds values
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Tools
2.2.x
Other All
: Normal minor
: 2.6
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2005-02-16 14:38 UTC by quazgar
Modified: 2009-07-08 19:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
output from gdb (1.73 KB, text/plain)
2005-02-16 17:31 UTC, quazgar
  Details
Proposed patch (1.15 KB, patch)
2009-07-04 09:16 UTC, Massimo
committed Details | Review

Description quazgar 2005-02-16 14:38:18 UTC
Distribution/Version: Gentoo stable, Kernel 2.6.10

Steps to reproduce the bug:

Open the Gimp, open the gradient tool, create a new gradient.
Select "Split Segment at Midpoint" from the 2nd Mouse Button menu.
On the left half, select "Right Endpoint's Color", the color chooser dialog will
open.
Move the transparency ruler down towards zero. If nothing happens, play around a
little with the other rulers.

Expected results:
The right endpoint will have the chosen color/alpha value.

Acutal result:
Gimp will segfault.

Reproducible:
Nearly always, often it is sufficient to move the alpha ruler to zero,
sometimes, the other values need to be changed as well.

Note:
I could not reproduce this for the initial left/right endpoints, maybe this is a
faulty initialization for the new points?
Comment 1 Raphaël Quinet 2005-02-16 15:27:14 UTC
It could be a Gentoo-specific issue, as I am not able to reproduce this.  Also,
you should be a bit more specific about "sometimes, the other values need to be
changed as well."  Which values?  How should they be changed?
Comment 2 quazgar 2005-02-16 17:31:07 UTC
Created attachment 37553 [details]
output from gdb

The output from gdb when running Gimp and reproducing the bug.
Comment 3 Raphaël Quinet 2005-02-16 17:39:04 UTC
Thanks for this stack trace.  Unfortunately, it seems that the source files
were not available when you ran gdb, so the information is a bit limited.  It
would be nice if you could keep a copy of the source files around so that gdb
could find them.  If you cannot prevent the gentoo build system from deleting
the files after installation, maybe there is at least a way to extract them
again?

If line 195 in gimpviewrendergradient.c is the same as the one I see, it would
be nice if you could ask gdb to print the values of "a" and "r" to see if "a | r"
could be out of the range of render_blend_dark_check[].  It would also be nice if
you could answer the questions in my previous comment.
Comment 4 quazgar 2005-02-17 00:18:03 UTC
@ #1: It doesn't matter how I change them, but I can give an example that works
for me :-):
First I change the alpha value to zero, and afterwards I increase the Blue ruler
by one click... Or at least try to.

@ #3: Yes, I'll do this as soon as I have some spare time... Didn't seem to work
until now, but I'll continue to try hard.
Comment 5 quazgar 2005-02-17 02:11:28 UTC
Ok, at long last:

according to gdb, the error occurs in:
195               *odd++ = render_blend_dark_check[a | r];

Values for a:
(gdb) p/d a
$1 = 70912
(gdb) p/o a
$3 = 0212400
(gdb) p a >> 8
$12 = 277

Values for r:
(gdb) p r
$15 = 128 '\200'
(gdb) p /t r
$6 = 10000000

And a|r:
(gdb) p a|r
$8 = 71040
(gdb) p /t a|r
$9 = 10001010110000000

But as "a << 8" evaluates to 277 which is bigger than 255 from
gimpdisplayshell-render.c and consequently the a|r = 71040 is bigger than the
allocated 65536 :-) Now I (or someone else) only has to find out, where the big
alpha value comes from...

(gdb) p color.a
$16 = 1.0887096774193543

instead of 1.000... 

Maybe something around line 526 in gimpgradient.c?
Well, good night, I'd just be happy if someone can confirm this bug...
Comment 6 Nathan Summers 2005-02-17 03:56:39 UTC
I ran into this problem on some early 1.3 versions, but haven't needed to use
the gradient editor since then.  When I tried to reproduce this on 2.2.3, I
couldn't get it to crash, but the color wouldn't change, either, so I can
confirm something is going wrong here.
Comment 7 Sven Neumann 2005-02-17 10:11:08 UTC
Looks like a valgrind session would most probably solve this easily.
Comment 8 Sven Neumann 2005-02-17 21:41:42 UTC
Following the description yields lots of these errors from valgrind:

==12177==
==12177== Invalid read of size 1
==12177==    at 0x8174297: gimp_view_renderer_gradient_render
(gimpviewrenderergradient.c:196)
==12177==    by 0x817282C: gimp_view_renderer_real_draw
(gimpviewrenderer.c:671)==12177==    by 0x81729C4: gimp_view_renderer_draw
(gimpviewrenderer.c:601)
==12177==    by 0x1BAA7D9B: gtk_cell_renderer_render (gtkcellrenderer.c:606)
==12177==    by 0x1BC43396: ??? (gtktreeviewcolumn.c:2780)
==12177==    by 0x1BC434E6: _gtk_tree_view_column_cell_render
(gtktreeviewcolumn.c:3109)
==12177==    by 0x1BC2E7DF: ??? (gtktreeview.c:3723)
==12177==    by 0x1BC2EC23: ??? (gtktreeview.c:3913)
==12177==    by 0x1BB6480D: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:83)
==12177==    by 0x1BE83948: (within /usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE836B5: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE94924: (within /usr/lib/libgobject-2.0.so.0.600.2)
==12177==  Address 0x1CCAC308 is 8 bytes before a block of size 20 alloc'd
==12177==    at 0x1B906EDD: malloc (vg_replace_malloc.c:131)
==12177==    by 0x1BF9EA36: g_malloc (in /usr/lib/libglib-2.0.so.0.600.2)
==12177==    by 0x1BE8287A: g_closure_new_simple (in
/usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE83806: g_cclosure_new (in /usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE920FD: g_signal_connect_data (in
/usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x80D58CD: render_init (gimpdisplayshell-render.c:101)
==12177==    by 0x808E09F: gui_restore_callback (gui.c:395)
==12177==    by 0x1BE95D12: g_cclosure_marshal_VOID__POINTER (in
/usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE836B5: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE94EC7: (within /usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE93F4B: g_signal_emit_valist (in
/usr/lib/libgobject-2.0.so.0.600.2)
==12177==    by 0x1BE941E5: g_signal_emit (in /usr/lib/libgobject-2.0.so.0.600.2)

Comment 9 Sven Neumann 2005-02-17 22:05:16 UTC
Applied to both branches:

2005-02-17  Sven Neumann  <sven@gimp.org>

	* app/widgets/gimpviewrenderergradient.c
	(gimp_view_renderer_gradient_render): don't attempt to read beyond
	the pre-calculated render buffers, even if the gradient somehow
	has out-of-bounds values. Fixes the crash reported in bug #167604.

There is still the question why gimp_gradient_get_color_at() returns
out-of-range values for the gradient colors. But at least we got rid of the
crash. Changing the summary and lowering severity...
Comment 10 Sven Neumann 2007-04-19 13:37:20 UTC
Is this still reproducable with a recent GIMP 2.3?
Comment 11 Massimo 2009-07-04 09:16:38 UTC
Created attachment 137827 [details] [review]
Proposed patch

Reproducible with gimp-2.6.6.

Not a problem of gimp_gradient_get_color_at(), that only
acts as a garbage-in garbage-out black box, but a typo 
in file 'app/actions/gradient-editor-commands.c'.
Comment 12 Martin Nordholts 2009-07-04 09:23:52 UTC
You're pretty productive!

Did we get your name and email to use in git commits yet btw?
Comment 13 Sven Neumann 2009-07-08 19:23:08 UTC
Patch applied to master and merged to the gimp-2-6 branch. Closing as FIXED. Massimo, thanks a lot for your help with this bug.

commit c837f25b40b97bba09845336ea0a997149aa3c5e
Author: Massimo Valentini <sixtysix@inwind.it>
Date:   Tue Jul 7 17:02:32 2009 +0200

    Bug 167604 – gimp_gradient_get_color_at() may return out-of-bounds values
    
    This is not a problem of gimp_gradient_get_color_at(), but a typo
    in gradient_editor_right_color_update().

 app/actions/gradient-editor-commands.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)