Re: [help-GIFT] Re: Patches AMD 64 next try...

[risc]

On Tue, Feb 27, 2007 at 03:54:04PM +0100, Wolfgang Mueller wrote:
> Dear address@hidden,
> 
> On Tue, 27 Feb 2007, address@hidden wrote:
> 
> > On Tue, Feb 27, 2007 at 10:48:18AM +0100, Wolfgang Müller wrote:
> > > On Monday 26 February 2007 16:07, address@hidden wrote:
> > > > You and I are getting different results. You're using whats in CVS, and
> > > > the one line change i mentioned in the last mail, right?
> > > >
> > > > i used to run into something that sounds like your bug, but i dont 
> > > > remember
> > > > the details, its been months since i messed with it.
> > > >
> > > 
> > > I applied the patches you sent to this list relevant to FeatureExtraction 
> > > (the 
> > > other ones, too), and I found that most of them were already applied in 
> > > CVS. 
> > > So I had the impression of being pretty up-to-date :-) .
> > > 
> > > I will post my current gabor.c to this list to make sure that we speak 
> > > about 
> > > the same thing. It's at home behind a NAT so tomorrow earliest.
> > > 
> > > > personally, i break things, then valgrind them to death. i'll let you 
> > > > know
> > > > when i have progress with this.
> > > 
> > > The fact that you said that the output of your stuff was bit-compatible 
> > > to 
> > > David's suggests that David's version had the same bug (yielding some 
> > > calculation glitches), but that the optimisations make it become apparent 
> > > as 
> > > segfault on 64 bit.
> > > 
> > > Cheers,
> > > Wolfgang
> > > 
> > > -- 
> > > Dr. Wolfgang Müller
> > > LS Medieninformatik
> > > Universität Bamberg
> > > Check out the SIG MM web site http://www.sigmm.org
> > > 
> > 
> > When i say bit compatible, i mean bit compatible to the one on my 32bit box.
> 
> I did understand that. 
> 
> Let us look at the loop:
> 
>       for (x = 0; x < width; x++) {
>         for (y = 0; y < height; y++) {
>           int positive = (width*height-1);
>           int negative = (y*width+x+kernal_size[filter_scale]/2);
>          
>         ...
> 
> If the kernal size/2 > 0 then negative *can* get greater than positive
> for some values of x and y. Everything else are overflow effects.
> 
> ...or what am I getting wrong here?
> 
> Cheers,
> Wolfgang
> 
> --
> Dr. Wolfgang Mueller
> LS Medieninformatik
> Universitaet Bamberg

no, you're getting it right, lets try a thought-experiment with gdb.

technically, i create a pointer outside of memory.
the pointer is target_image, pointed to 
&image[(width*height-1)-(y*width+x+kernal_size[filter_scale]/2)].

but then i use that pointer, with a subscript that points it back into the 
array bounds.
target_image[k].

whats happening in reality is really spooky: using GDB, run the program until 
it crashes.

address@hidden:/disk1/test$ gdb 
~/src/merge/dev/gift/build/FeatureExtraction/gift-extract-features
GNU gdb 6.6-debian
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) set args tempsized.ppm
(gdb) run
Starting program: 
/home/gift/src/merge/dev/gift/build/FeatureExtraction/gift-extract-features 
tempsized.ppm
[Thread debugging using libthread_db enabled]
[New Thread 47833325162320 (LWP 23813)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 47833325162320 (LWP 23813)]
0x0000000000403064 in gabor_filter (image=0x2b810fe16010, width=256, 
height=256, filter_scale=0, orientation=0, kernelsxy=0x7fff9ad0c330, 
conv=0x2b811062e010, 
    conv2=0x2b81106ae010, output=0x2b81105ae010) at 
../../FeatureExtraction/gabor.c:112
112           conv[((width*height)-1)-(x*height+y)] +=
(gdb) list 
107       }
108     else
109       {
110     for (k=0; k < kernal_size[filter_scale]; k++) {
111       if ((x+kernal_size[filter_scale]/2 >= k) && 
(x+kernal_size[filter_scale]/2 < width+k)) {
112           conv[((width*height)-1)-(x*height+y)] +=
113            target_kernal[k]*target_image[k];
114       }
115     }
116       }
(gdb) list 107
102       {
103         for (k = 0; k < kernal_size[filter_scale]; k++)
104           temparray[k]= target_kernal[k]*target_image[k];
105         for (k = 0; k < kernal_size[filter_scale]; k++)
106           conv[((width*height)-1)-(x*height+y)] += temparray[k];
107       }
108     else
109       {
110     for (k=0; k < kernal_size[filter_scale]; k++) {
111       if ((x+kernal_size[filter_scale]/2 >= k) && 
(x+kernal_size[filter_scale]/2 < width+k)) {
(gdb) list 102
97  target_kernal=kernelsxy[filter_scale*num_gabors_per_scale+orientation];
98  for (x = 0; x < width; x++) {
99  for (y = 0; y < height; y++) {
100     
target_image=&image[(width*height-1)-(y*width+x+kernal_size[filter_scale]/2)];
101     if ((x>=kernal_size[filter_scale]/2) && 
((x+kernal_size[filter_scale]/2)<width))
102       {
103         for (k = 0; k < kernal_size[filter_scale]; k++)
104           temparray[k]= target_kernal[k]*target_image[k];
105         for (k = 0; k < kernal_size[filter_scale]; k++)
106           conv[((width*height)-1)-(x*height+y)] += temparray[k];
(gdb) print target_image 
$1 = (double *) 0x2b890fe16008
(gdb) print &image[(width*height-1)-(y*width+x+kernal_size[filter_scale]/2)] 
$2 = (double *) 0x2b810fe16008
(gdb) 

as you can see, 2b810 becomes 2b890 by magic!

this goes away if you change x or y into a uint64_t.

i'm scared. i'd love an answer to this one. other than "magic."

Julia Longtin <address@hidden>