| | [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #2637
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 2 - Low, e.g. a documentation error or undocumented side-effect |
|---|
| Scope: | 2 - Specific to an operating system |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | Valgrind errors and their fixes for 1.3.0rc5 |
|---|
| Version: | 1.3.0 |
|---|
| Created By: | csaba |
|---|
| Assigned To: | AlbrechtS |
|---|
| Fix Version: | 1.3.0 (SVN: v8731) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | csaba
12:20 May 19, 2011 |
| The attached patch fixes two errors reported by Valgrind. These are on a 64 bit Linux system.
First error probably only comes up in certain dual monitor configurations. Observed on a system using the proprietary Nvidia driver.
Second error may be 64 bit specific?
==7833== Invalid read of size 4
==7833== at 0x510047: screen_init() (screen_xywh.cxx:155)
==7833== by 0x510249: Fl::screen_xywh(int&, int&, int&, int&, int, int) (screen_xywh.cxx:197)
==7833== by 0x4F8FFE: Fl_X::make_xid(Fl_Window*, XVisualInfo*, unsigned long) (Fl_x.cxx:1594)
==7833== by 0x4FA1D2: Fl_Window::show() (Fl_x.cxx:1876)
==7833== by 0x4C6A97: Fl_Double_Window::show() (Fl_Double_Window.cxx:68)
==7833== by 0x4D9E4F: Fl_Overlay_Window::show() (Fl_Overlay_Window.cxx:46)
==7833== by 0x4F3C8C: Fl_Window::show(int, char**) (Fl_arg.cxx:361)
==7833== by 0x47BC74: Fl_Utilities::showWindow(Fl_Window*, int, char**, char const* const*) (Fl_Utilities.cxx:142)
==7833== by 0x412945: DsViewerApp::run(int, char**) (DsViewerApp.cxx:72)
==7833== by 0x4113FB: main (DSviewer.cxx:52)
==7833== Address 0x7a9bf60 is not stack'd, malloc'd or (recently) free'd
==7833==
==7833== Invalid read of size 1
==7833== at 0x501FAD: xrgb_converter(unsigned char const*, unsigned char*, int, int) (fl_draw_image.cxx:323)
==7833== by 0x502A9F: innards(unsigned char const*, int, int, int, int, int, int, int, void (*)(void*, int, int, int, unsigned char*), v
==7833== by 0x502D14: Fl_Xlib_Graphics_Driver::draw_image(unsigned char const*, int, int, int, int, int, int) (fl_draw_image.cxx:547)
==7833== by 0x4C7117: fl_draw_image(unsigned char const*, int, int, int, int, int, int) (fl_draw.H:667)
==7833== by 0x4CACA9: alpha_blend(Fl_RGB_Image*, int, int, int, int, int, int) (Fl_Image.cxx:431)
==7833== by 0x4CB2BE: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (Fl_Image.cxx:573)
==7833== by 0x4CAD2A: Fl_RGB_Image::draw(int, int, int, int, int, int) (Fl_Image.cxx:438)
==7833== by 0x4BDE35: Fl_Image::draw(int, int) (Fl_Image.H:160)
==7833== by 0x5004D8: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (fl_d
==7833== by 0x500D0C: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (fl_draw.cxx:386)
==7833== by 0x5065F0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (fl_labeltype.cxx:46)
==7833== by 0x50674C: Fl_Label::draw(int, int, int, int, unsigned int) const (fl_labeltype.cxx:88)
==7833== Address 0x7d2bbc0 is 0 bytes after a block of size 720 alloc'd
==7833== at 0x4C283AD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==7833== by 0x52600C: operator new(unsigned long) (in /home/cbiegl/EDAS-sw/ds-devel-3.0/bin/Debug/Linux/x86_64/DSviewer)
==7833== by 0x5251A8: operator new[](unsigned long) (in /home/cbiegl/EDAS-sw/ds-devel-3.0/bin/Debug/Linux/x86_64/DSviewer)
==7833== by 0x4CA9F9: alpha_blend(Fl_RGB_Image*, int, int, int, int, int, int) (Fl_Image.cxx:387)
==7833== by 0x4CB2BE: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (Fl_Image.cxx:573)
==7833== by 0x4CAD2A: Fl_RGB_Image::draw(int, int, int, int, int, int) (Fl_Image.cxx:438)
==7833== by 0x4BDE35: Fl_Image::draw(int, int) (Fl_Image.H:160)
==7833== by 0x5004D8: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (fl_d
==7833== by 0x500D0C: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (fl_draw.cxx:386)
==7833== by 0x5065F0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (fl_labeltype.cxx:46)
==7833== by 0x50674C: Fl_Label::draw(int, int, int, int, unsigned int) const (fl_labeltype.cxx:88)
==7833== by 0x506A18: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (fl_labeltype.cxx:134) | |
| |
| #2 | matt
13:38 May 19, 2011 |
| **SIGH**. I am really really glad that someone's using Valgrid to fix issues. I think it's a magic tool ;-) - but why in the world do those results always come exactly 5 days *after* the Release Candidate?
I think we can put that in the final as well.
COnsidering though that this is the fourth change for RC5, we may be looking at an RC6 :-/ | |
| |
| #3 | greg.ercolano
14:32 May 19, 2011 |
| Nice!
When committed, please comment the "+15)&(~7)" magic numbers, which appear to be a way to allocate in 8 byte chunks, if I read that correctly..? | |
| |
| #4 | csaba
15:10 May 19, 2011 |
| Note that the patch for screen_xywh.cxx only addresses the line that gave an error message under Valgrind. Probably the same logic should apply to the vertical DPI calculation. This currently polls the SAME (= fl screen) screen in each iteration of the loop. It did not generate an error message, so I did not change it -- just in case there is some non-obvious reason for doing it this way...
As for why now: Because today was the day when I messed up something so badly in my own app that I needed Valgrind:-). | |
| |
| #5 | AlbrechtS
02:09 May 20, 2011 |
| The first part of the patch (padding the allocation) seems /wrong/ to me. This would only fix the (valgrind) symptoms, but don't take care of accessing undefined bytes. I couldn't look at the code yet, but if someone else does, this ought to be considered.
That said, it could have to do with a recent change in alpha_blend() that I did to fix STR #2606. Csaba, do you have a simple test case for this error? IMHO alpha_blend() wouldn't be called if you didn't use a label (?) with an image with transparency. Did you? If yes, please post a simple test code with the image in question (unless my assumption is completely wrong).
I can take a look at this later tonight or tomorrow, but would like to leave the screen_init() issue to someone else. | |
| |
| #6 | csaba
05:45 May 20, 2011 |
| To me it looks like it is the consequence of doing two pixels in each iteration. Take a look at the "innards" #defines in fl_draw_image.cxx. There seems to be some effort there to do two pixels at a time when U64 is defined. What if the image width is odd?
I don't think this gives much performance gain anyway. Consider 3 byte size reads, some shifts, one 32 bit write versus 6 byte size reads, more shifts, and one 64 bit write (which may not even be 64 bit on a 32 bit architecture that has U64 defined). Why anyone even bothered is beyond me...
Maybe that whole U64 dependent conditional part branch just be deactivated, and then re-checked with Valgrind. I'll take a look at it. | |
| |
| #7 | matt
06:51 May 20, 2011 |
| You can use "svn blame" to find out who did it and when. It may well be the remains from optimization effort from many years ago when 32 bit *was * slow and 64 bits was all the rage (and hope ;-) | |
| |
| #8 | csaba
07:10 May 20, 2011 |
| Albrecht, I posted a simple test file you requested. | |
| |
| #9 | AlbrechtS
08:00 May 20, 2011 |
| Thanks for the example program and your comments about what you found out so far. For others that want to test: I had to add these 3 lines to icontest.cxx to make it work:
#include <FL/Fl.H>
#include <FL/Fl_Double_Window.H>
#include <FL/Fl_Button.H>
I compiled and ran it on 32-bit Windows and 32-bit Linux (with valgrind) without error, as was probably to be expected. I'll try later (maybe in 3 or 4 hours from now) on a 64-bit Linux system to see if I can reproduce the bug. | |
| |
| #10 | greg.ercolano
09:21 May 20, 2011 |
| FWIW: I compiled icontest.cxx on fedora14 (64bit) with latest FLTK svn (r8697) and it ran ok. Ran it under valgrind with "0 errors" too (below).
I re-ran it with -v and still no errors (but lots of warnings about seemingly unrelated stuff; strcmp, rindex, etc)
Caveat: in my case I ran it on 64bit linux box, but it's currently headless, so I had it displaying on my 32bit linux workstation (using the DISPLAY variable).
Will be curious to see Albrecht's results on a machine with a graphics head.
* * *
[root@ozark examples]# valgrind ./foo
==9270== Memcheck, a memory error detector
==9270== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==9270== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==9270== Command: ./foo
==9270==
==9270==
==9270== HEAP SUMMARY:
==9270== in use at exit: 390,844 bytes in 2,169 blocks
==9270== total heap usage: 7,914 allocs, 5,745 frees, 2,019,398 bytes allocated
==9270==
==9270== LEAK SUMMARY:
==9270== definitely lost: 1,089 bytes in 3 blocks
==9270== indirectly lost: 1,360 bytes in 42 blocks
==9270== possibly lost: 7,673 bytes in 227 blocks
==9270== still reachable: 380,722 bytes in 1,897 blocks
==9270== suppressed: 0 bytes in 0 blocks
==9270== Rerun with --leak-check=full to see details of leaked memory
==9270==
==9270== For counts of detected and suppressed errors, rerun with: -v
==9270== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 9 from 7)
* * * | |
| |
| #11 | greg.ercolano
09:36 May 20, 2011 |
| OP, do you have any special FLTK 'configure' flags enabled other than the FLTK defaults?
Followup: I just booted a 64bit Centos 5.5 machine with a graphics head, and ran the icontest.cxx built against fltk 1.3 current with default configure; 0 errors w/valgrind. I'm assuming the icontest.cxx should be showing errors in valgrind, is that what you're getting on your system? | |
| |
| #12 | csaba
09:57 May 20, 2011 |
| I did some more digging. The test program I posted will not produce the valgrind error. An image with an odd width is needed. I also experimented with the "#ifdef U64" conditional compile branch in "fl_draw_image.cxx". The error ONLY occurs if the image width is odd AND if the "U64 defined" condition is true.
Did the checking under 64 bit Linux. Will retest under 32 bit and post a follow up shortly. | |
| |
| #13 | AlbrechtS
10:04 May 20, 2011 |
| You'll have to wait another 1 or 2 hours until I'm sitting at my 64-bit Linux box. Until then you /may/ try it with another image (icon) with an odd width (the embedded image is 32x32 px). See also Csaba's post from 05:45 on May 20. | |
| |
| #14 | csaba
10:45 May 20, 2011 |
| 1) Posted updated test program. Has two images under control of a tab widget, error will appear when you tab to the "Odd" page. Precisely three error messages will appear, at 0,1,2 bytes after the end of the allocated block. See log below.
2) Checked under 32 bit Linux. No error even for odd width images. "configure" generates a "config.h" that does not define U64.
3) "configure" (I use no special options) will generate a "config.h" with U64 defined under 64 bit Linux.
4) When U64 is defined "fl_draw_image.cxx" (around line 295) will select the 64 bit, two pixels at a time "innards" implementation. This "innards" implementation was clearly written with an assumption that odd numbers do not exist. ("int w1 = (w+1)/2;" -- yeah, right).
5) The best fix is probably to disable the U64 specific #ifdef branch from the image conversion code as the performance gains provided by it are dubious at best. I am actually more worried about the destination buffer of the conversion. That's the one which will be possibly written to beyond its end. In the usage scenario where I detected this issue the destination seems to be allocated with some extra padding (after all valgrind does not complain about writes), but is this true for every usage scenario???
==25119== Memcheck, a memory error detector
==25119== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==25119== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==25119== Command: test/icontest
==25119==
==25119== Invalid read of size 4
==25119== at 0x41E2A7: screen_init() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41E3F0: Fl::screen_xywh(int&, int&, int&, int&, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x410EE4: Fl_X::make_xid(Fl_Window*, XVisualInfo*, unsigned long) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x40D79D: Fl_Window::show(int, char**) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x404E7D: main (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== Address 0x78476e0 is not stack'd, malloc'd or (recently) free'd
==25119==
==25119== Invalid read of size 1
==25119== at 0x414875: xrgb_converter(unsigned char const*, unsigned char*, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41538D: innards(unsigned char const*, int, int, int, int, int, int, int, void (*)(void*, int, int, int, unsigned char*), void*) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x415550: Fl_Xlib_Graphics_Driver::draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x40745A: fl_draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x409358: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== Address 0x7a523a1 is 0 bytes after a block of size 5,265 alloc'd
==25119== at 0x4C27909: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25119== by 0x4091FA: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4085EC: Fl_Group::draw_children() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119==
==25119== Invalid read of size 1
==25119== at 0x41487A: xrgb_converter(unsigned char const*, unsigned char*, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41538D: innards(unsigned char const*, int, int, int, int, int, int, int, void (*)(void*, int, int, int, unsigned char*), void*) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x415550: Fl_Xlib_Graphics_Driver::draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x40745A: fl_draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x409358: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== Address 0x7a523a2 is 1 bytes after a block of size 5,265 alloc'd
==25119== at 0x4C27909: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25119== by 0x4091FA: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4085EC: Fl_Group::draw_children() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119==
==25119== Invalid read of size 1
==25119== at 0x414880: xrgb_converter(unsigned char const*, unsigned char*, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41538D: innards(unsigned char const*, int, int, int, int, int, int, int, void (*)(void*, int, int, int, unsigned char*), void*) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x415550: Fl_Xlib_Graphics_Driver::draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x40745A: fl_draw_image(unsigned char const*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x409358: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== Address 0x7a523a3 is 2 bytes after a block of size 5,265 alloc'd
==25119== at 0x4C27909: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25119== by 0x4091FA: Fl_Xlib_Graphics_Driver::draw(Fl_RGB_Image*, int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x408713: Fl_RGB_Image::draw(int, int, int, int, int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x41264A: Fl_Image::draw(int, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x413BFD: fl_draw(char const*, int, int, int, int, unsigned int, void (*)(char const*, int, int, int), Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x414064: fl_draw(char const*, int, int, int, int, unsigned int, Fl_Image*, int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4175C0: fl_normal_label(Fl_Label const*, int, int, int, int, unsigned int) (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4176AE: Fl_Widget::draw_label(int, int, int, int, unsigned int) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x406A23: Fl_Button::draw() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4085EC: Fl_Group::draw_children() (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119== by 0x4083E7: Fl_Group::draw_child(Fl_Widget&) const (in /tmp/fltk-1.3.x-r8695/test/icontest)
==25119==
==25119==
==25119== HEAP SUMMARY:
==25119== in use at exit: 492,398 bytes in 1,597 blocks
==25119== total heap usage: 9,378 allocs, 7,781 frees, 1,757,801 bytes allocated
==25119==
==25119== LEAK SUMMARY:
==25119== definitely lost: 1,148 bytes in 3 blocks
==25119== indirectly lost: 1,296 bytes in 40 blocks
==25119== possibly lost: 1,968 bytes in 38 blocks
==25119== still reachable: 487,986 bytes in 1,516 blocks
==25119== suppressed: 0 bytes in 0 blocks
==25119== Rerun with --leak-check=full to see details of leaked memory
==25119==
==25119== For counts of detected and suppressed errors, rerun with: -v
==25119== ERROR SUMMARY: 13 errors from 4 contexts (suppressed: 10 from 5) | |
| |
| #15 | AlbrechtS
14:37 May 20, 2011 |
| Sorry, it took a little longer ... but now I can confirm everything Csaba found out. Many thanks for reporting the bug and for your thorough testing. I get the same valgrind errors when running on 64-bit Linux except the screen_init() error (I have currently only one monitor configured).
I do also think what we see here is a questionable and maybe even counter-productive attempt to optimize something. I know for sure that reads and writes to not properly aligned data in IA64 architecture (Intel Itanium) are very expensive WRT performance, because this will be trapped, and then "repaired" by a system handler. This is true for at least one OS I'm working with on IA64 (OpenVMS), but I assume that it doesn't depend on the OS, since it's a hardware (processor) property. Is the data here aligned for 64-bit access? I don't know, but maybe not.
The other important point Csaba made is that the destination array's bounds will be overwritten as well, although this is not found by Valgrind in our tests. On a first look it seems however, that the destination buffer might be allocated with rounding to the correct sizes, but this needs to be checked further.
It's too late here now to do more testing, but I'll take another look ASAP this weekend. Maybe removing the U64 part could be a *simple* solution, but then we must make sure that U32 is always defined. Looking at configure.in this seems to be so, but I'm not sure about that. | |
| |
| #16 | AlbrechtS
11:01 May 23, 2011 |
| Working on a solution now... | |
| |
| #17 | AlbrechtS
12:21 May 23, 2011 |
| Part 1 (screen_init) is fixed now (svn r 8727).
I decided to fix the vertical resolution (dpi) as well, since this was obviously a cut'n'paste error. Please test whether this solves the problem (I can't test it since I don't have your screen configuration). | |
| |
| #18 | AlbrechtS
14:17 May 23, 2011 |
| Fixed in Subversion repository.
Part 2 (image converter) is fixed now (svn r 8731).
It turned out that this was not only an issue for transparent images, but also for all other images with odd width on 64-bit X systems. All image converters would pack two pixels into one (64-bit) long variable and write this to the X image format buffer. Doing this, one byte too much (per line) was accessed when packing the pixel values. This is fixed now.
FTR: a simple test case was to run test/pixmap_browser with valgrind and to open an odd-sized image (I used a 41x47 pixel png image).
Note: I could not test this on a 64-bit big endian system, but I suppose that this works as well. Critical looks at the changes and test results from 64-bit big endian machines welcome!
Leaving the STR open for feedback, but I believe it is solved, and this shouldn't be a show stopper for the next RC! | |
| |
| #19 | AlbrechtS
14:21 May 23, 2011 |
| Csaba, thanks again for the test cases and the patches. I modified both patches, but they were very helpful.
Can you confirm that svn r 8731 fixes the issues with your application? | |
| |
| #20 | csaba
17:20 May 23, 2011 |
| Albrecht, I checked out branch-1.3 version 8732. All valgrind warnings are gone. Thanks for the good work! Csaba | |
| |
| #21 | AlbrechtS
00:32 May 24, 2011 |
| You're welcome, and thanks for confirmation. | |
[ Return to Bugs & Features ]
|
| |
