|
From: Daniel J S. <dan...@ie...> - 2006-03-15 23:12:20
Attachments:
bindata-djs-15mar2006.patch
|
Well, I've got to hand it to valgrind. It found an uninitialized variable in the image demo. The problem was actually not the instruction valgrind pointed out, but a step or two back from that in a different routine. One has to sort of trace where the memory came from. So, attached is a short patch to fix what valgrind found. Below are a few other things that valgrind has found for 'all.dem'. If anything looks familiar to you as code you may have worked with, please take a look to see if there is a problem with the code. If not, attempt a change to rid the valgrind complaint. Thanks, Dan PS: The free-ing of the memory in reset_palette() has lessened the rate at which memory accumulates. There still seems to be a leak. I'll hunt for that later. ======================== c13 -0.141 0.028 -0.086 1.000 phi0 0.114 -0.022 0.034 0.181 1.000 ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x8060155: update (fit.c:989) ==1563== by 0x80542BA: update_command (command.c:1711) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== by 0x80885BA: load_file (misc.c:277) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x8060146: update (fit.c:989) ==1563== by 0x80542BA: update_command (command.c:1711) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== by 0x80885BA: load_file (misc.c:277) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x806014A: update (fit.c:989) ==1563== by 0x80542BA: update_command (command.c:1711) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== by 0x80885BA: load_file (misc.c:277) ******************** file world.dem ******************** Hit return to continue Hit return to continue Hit return to continue Hit return to continue ==1563== ==1563== Use of uninitialised value of size 8 [[[[[ Is it possible here Ethan that v[3] (of color = v[3];) is not being set inside df_readascii() of datafile.c but the value of j (output in datafile.c) is being incremented? ]]]]] ==1563== at 0x8099BD9: eval_3dplots (plot3d.c:775) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== by 0x80885BA: load_file (misc.c:277) ==1563== by 0x8053092: load_command (command.c:978) Same plot with hidden line removal ==1563== ==1563== Use of uninitialised value of size 8 ==1563== at 0x8099BD9: eval_3dplots (plot3d.c:775) ==1563== by 0x80544FB: replotrequest (command.c:1843) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== by 0x80885BA: load_file (misc.c:277) Now create a x/y datafile for plotting with vectors and display vectors parallel to the electrostatic field Hit return to continue ==1563== ==1563== Use of uninitialised value of size 8 ==1563== at 0x807778E: plot_vectors (graphics.c:3272) ==1563== by 0x807A8EA: do_plot (graphics.c:1760) ==1563== by 0x80932F1: eval_plots (plot2d.c:2181) ==1563== by 0x80551B3: do_line (command.c:530) ==1563== ==1563== Use of uninitialised value of size 8 ==1563== at 0x8077792: plot_vectors (graphics.c:3273) ==1563== by 0x807A8EA: do_plot (graphics.c:1760) ==1563== by 0x80932F1: eval_plots (plot2d.c:2181) ==1563== by 0x80551B3: do_line (command.c:530) Hit return to continue********************** file tics.dem ********************* Now try histograms stacked by columns Next we do several sets of parallel histograms ==1563== ==1563== Use of uninitialised value of size 4 ==1563== at 0x80A8E34: parse_label_options (set.c:4664) ==1563== by 0x80A9578: set_xyzlabel (set.c:4127) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x80A8F2A: parse_label_options (set.c:4730) ==1563== by 0x80A9578: set_xyzlabel (set.c:4127) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x80A9237: parse_label_options (set.c:4744) ==1563== by 0x80A9578: set_xyzlabel (set.c:4127) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x80A90B1: parse_label_options (set.c:4832) ==1563== by 0x80A9578: set_xyzlabel (set.c:4127) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) ==1563== ==1563== Use of uninitialised value of size 4 ==1563== at 0x80A8E34: parse_label_options (set.c:4664) ==1563== by 0x80A9591: set_xyzlabel (set.c:4143) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) ==1563== ==1563== Conditional jump or move depends on uninitialised value(s) ==1563== at 0x80A90B1: parse_label_options (set.c:4832) ==1563== by 0x80A9591: set_xyzlabel (set.c:4143) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) Same plot using rowstacked histogram |
|
From: Ethan M. <merritt@u.washington.edu> - 2006-03-15 23:50:37
|
Looks like you're having fun with valgrind. I've applied your tuple[2] fix This one is a false alarm, though the code can be re-arranged to clarify what's going on: > at 0x8099BD9: eval_3dplots (plot3d.c:775) I recognize and will fix one of the errors below that ==1563== Use of uninitialised value of size 4 ==1563== at 0x80A8E34: parse_label_options (set.c:4664) ==1563== by 0x80A9578: set_xyzlabel (set.c:4127) ==1563== by 0x80AAA85: set_style (set.c:4879) ==1563== by 0x80ADE33: set_command (set.c:418) I don't have time to look at the others just now. Ethan -- Ethan A Merritt Biomolecular Structure Center University of Washington, Seattle WA |
|
From: Daniel J S. <dan...@ie...> - 2006-03-16 00:26:07
|
I've put some comments within to indicate what the line numbers correspond to in my version of the source code:
correlation matrix of the fit parameters:
c33 c11 c44 c13 phi0
c33 1.000
c11 -0.066 1.000
c44 -0.198 -0.278 1.000
c13 -0.141 0.028 -0.086 1.000
phi0 0.114 -0.022 0.034 0.181 1.000
==9857== Conditional jump or move depends on uninitialised value(s)
==9857== at 0x8060111: update (fit.c:989)
tmp = s + strlen(s) - 1;
while (*tmp != '\\' && *tmp != '/' && *tmp != ':' && tmp - s >= 0)
[989] tmp--;
[Not sure what valgrind is complaining about. Maybe the memory pointed to by "s" coming into the routine is not initialized.]
==9857== by 0x805425E: update_command (command.c:1711)
==9857== by 0x8055157: do_line (command.c:530)
==9857== by 0x8088576: load_file (misc.c:277)
==9857==
==9857== Conditional jump or move depends on uninitialised value(s)
==9857== at 0x8060102: update (fit.c:989)
==9857== by 0x805425E: update_command (command.c:1711)
==9857== by 0x8055157: do_line (command.c:530)
==9857== by 0x8088576: load_file (misc.c:277)
==9857==
==9857== Conditional jump or move depends on uninitialised value(s)
==9857== at 0x8060106: update (fit.c:989)
==9857== by 0x805425E: update_command (command.c:1711)
==9857== by 0x8055157: do_line (command.c:530)
==9857== by 0x8088576: load_file (misc.c:277)
******************** file world.dem ********************
Hit return to continue
Hit return to continue
Hit return to continue
Hit return to continue
==9857==
==9857== Use of uninitialised value of size 8
==9857== at 0x8099B95: eval_3dplots (plot3d.c:775)
if (j >= 4) {
[775] color = v[3];
color_from_column(TRUE);
[My guess would be that v[3] is not set correctly inside datafile.c.]
==9857== by 0x8055157: do_line (command.c:530)
==9857== by 0x8088576: load_file (misc.c:277)
==9857== by 0x8053036: load_command (command.c:978)
Same plot with hidden line removal
==9857==
==9857== Use of uninitialised value of size 8
==9857== at 0x8099B95: eval_3dplots (plot3d.c:775)
==9857== by 0x805449F: replotrequest (command.c:1843)
==9857== by 0x8055157: do_line (command.c:530)
==9857== by 0x8088576: load_file (misc.c:277)
Hit return to continue
******************** file prob.dem ********************
Now create a x/y datafile for plotting with vectors
and display vectors parallel to the electrostatic field
Hit return to continue
==9857==
==9857== Use of uninitialised value of size 8
==9857== at 0x807774A: plot_vectors (graphics.c:3272)
points[0] = plot->points[i];
[3272] points[1].x = plot->points[i].xhigh;
[3273] points[1].y = plot->points[i].yhigh;
[A quick search on xhigh and yhigh shows it isn't set very often. Probably these aren't being set properly when reading in some points.]
==9857== by 0x807A8A6: do_plot (graphics.c:1760)
==9857== by 0x80932AD: eval_plots (plot2d.c:2181)
==9857== by 0x8055157: do_line (command.c:530)
==9857==
==9857== Use of uninitialised value of size 8
==9857== at 0x807774E: plot_vectors (graphics.c:3273)
==9857== by 0x807A8A6: do_plot (graphics.c:1760)
==9857== by 0x80932AD: eval_plots (plot2d.c:2181)
==9857== by 0x8055157: do_line (command.c:530)
Hit return to continue********************** file tics.dem *********************H
|
|
From:
<br...@ph...> - 2006-03-17 16:27:23
|
Daniel J Sebald wrote: > ==9857== Conditional jump or move depends on uninitialised value(s) > ==9857== at 0x8060111: update (fit.c:989) > > tmp = s + strlen(s) - 1; > while (*tmp != '\\' && *tmp != '/' && *tmp != ':' && tmp - s >= 0) > [989] tmp--; That's the same line 989 that I was looking at when I said there's no conditional anywhere in it. One possible problem is that tmp could be moved to before 's'. A change of test sequence might be in order. Checked into CVS. |
|
From: James R. V. Z. <jr...@co...> - 2006-03-22 02:32:13
|
Daniel J Sebald <dan...@ie...> writes:
>Well, I've got to hand it to valgrind. It found an uninitialized
>variable in the image demo...
Thanks for reminding us about valgrind.
To extend coverage to more terminals, I modified demo/webify like this
open(GNUPLOT, "|valgrind --leak-check=full --log-file=$ARGV[0] ../src/gnuplot") or die "can't find gnuplot";
my $png="png";
if ($#ARGV>0) {$png=$ARGV[1];}
print GNUPLOT "set term $png\n";
print GNUPLOT "set output \"$ARGV[0].$plot.$png\"\n";
so I could call it like this
for x in *.dem; do ./webify.pl `echo $x|sed 's/.dem//'` postscript; done
This leakage turned up every time:
==17498== 156 (36 direct, 120 indirect) bytes in 1 blocks are
definitely lost in loss record 5 of 9
==17498== at 0x1B90459D: malloc (vg_replace_malloc.c:130)
==17498== by 0x1BC6C179: (within /lib/tls/libc-2.3.5.so)
==17498== by 0x1BC6C7C1: __nss_database_lookup (in /lib/tls/libc-2.3.5.so)
==17498== by 0x1B90F139: ???
==17498== by 0x1B9108F4: ???
==17498== by 0x1BC1A0B9: getpwnam_r (in /lib/tls/libc-2.3.5.so)
==17498== by 0x1BC19B41: getpwnam (in /lib/tls/libc-2.3.5.so)
==17498== by 0x80FA84A: getusername (util.c:1143)
==17498== by 0x80DD313: PS_common_init (post.trm:2278)
==17498== by 0x80DD841: PS_init (post.trm:2403)
==17498== by 0x80B62B9: term_init (term.c:531)
==17498== by 0x806E993: do_plot (graphics.c:1301)
getpwman returns a pointer to a struct. Apparently this
implementation allocates space for the struct from the heap. However,
we can't just free it because the man page says the struct may be in a
static area. We could use getpwnam_r instead, if it is available.
Possibly another configuration test?
This one:
==17774== 98 bytes in 2 blocks are definitely lost in loss record 2 of 8
==17774== at 0x1B90459D: malloc (vg_replace_malloc.c:130)
==17774== by 0x804A9CC: gp_alloc (alloc.c:268)
==17774== by 0x80DC8C3: PS_options (post.trm:1427)
==17774== by 0x80A7B13: set_terminal (set.c:3275)
==17774== by 0x8051E88: command (command.c:539)
==17774== by 0x8051938: do_line (command.c:391)
==17774== by 0x8051863: com_line (command.c:342)
==17774== by 0x808E8D8: main (plot.c:639)
show that this allocation in post.trm leaks memory:
ps_fontfile_char = gp_alloc (totlength+1,"ps_fontfile_char");
- Jim Van Zandt
|
|
From: Ethan A M. <merritt@u.washington.edu> - 2006-03-23 07:24:47
|
On Tuesday 21 March 2006 06:32 pm, James R. Van Zandt wrote: > Thanks for reminding us about valgrind. > This leakage turned up every time: > > ==17498== 156 (36 direct, 120 indirect) bytes in 1 blocks are > definitely lost in loss record 5 of 9 > ==17498== at 0x1B90459D: malloc (vg_replace_malloc.c:130) > ==17498== by 0x1BC6C179: (within /lib/tls/libc-2.3.5.so) > ==17498== by 0x1BC6C7C1: __nss_database_lookup (in /lib/tls/libc-2.3.5.so) > ==17498== by 0x1B90F139: ??? > ==17498== by 0x1B9108F4: ??? > ==17498== by 0x1BC1A0B9: getpwnam_r (in /lib/tls/libc-2.3.5.so) > ==17498== by 0x1BC19B41: getpwnam (in /lib/tls/libc-2.3.5.so) > ==17498== by 0x80FA84A: getusername (util.c:1143) > ==17498== by 0x80DD313: PS_common_init (post.trm:2278) > ==17498== by 0x80DD841: PS_init (post.trm:2403) > ==17498== by 0x80B62B9: term_init (term.c:531) > ==17498== by 0x806E993: do_plot (graphics.c:1301) I am not seeing that error here, or at least not for "setenv GNUTERM post; \ valgrind --leak-check=full --log-file=valgrind gnuplot all.dem </bin/true >foo" > show that this allocation in post.trm leaks memory: > > ps_fontfile_char = gp_alloc (totlength+1,"ps_fontfile_char"); I already knew about this leak, but have not found the proper place to free the pointers. They are copied to an array which is used in several places. IIRC this is Harald Harders' code. Perhaps he can help. -- Ethan A Merritt Biomolecular Structure Center University of Washington, Seattle 98195-7742 |
|
From:
<br...@ph...> - 2006-03-24 15:22:37
|
James R. Van Zandt wrote: > This leakage turned up every time: This is a leak-by-design of the C library. It's not really our job to worry about it. getpwnam() is defined to return a pointer to memory that we don't own. We *cannot* free it. We don't even know it was ever malloc()ed in the first place. > We could use getpwnam_r instead, if it is available. I seriously doubt that getting gnuplot 100% valgrind-clean is a viable goal for the foreseeable future. Until that changes, I'm against making such a change just to quiet valgrind. We've much bigger cats to skin than that one. |