Hi Ola, Thanks for your detailed explanations.
Some logging: static int findnext(potrace_bitmap_t *bm, int *xp, int *yp) { FILE *fl = fopen("log", "w"); fprintf(fl, "Starting test\n"); fflush(fl); int x; int y; int x0; x0 = (*xp) & ~(BM_WORDBITS-1); for (y=*yp; y>=0; y--) { for (x=x0; x<bm->w && x>=0; x+=BM_WORDBITS) { //fprintf(fl, "x=%d\n", x); if (*bm_index(bm, x, y)) { fprintf(fl, "Executed *bm_index(bm, x=%d, y=%d)\n", x, y); fflush(fl); while (!BM_GET(bm, x, y)) { fprintf(fl, "Executed !BM_GET(bm, x=%d, y=%d)\n", x, y); fflush(fl); x++; } /* found */ *xp = x; *yp = y; fclose(fl); return 0; } } x0 = 0; } /* not found */ fprintf(fl, "Nothing found, exit with x=%d\n", x); fclose(fl); return 1; } (probably lots of useless fflush-es, but that doesn't matter) 1) When //fprintf(fl, "x=%d\n", x); is commented, potrace crashes and the log file only contains "Starting test". 2) If we uncomment it, potraces does not crash anymore and the log file contains Starting test x=0 x=64 x=128 ... x=2147483584 Nothing found, exit with x=-2147483648 which is the excepted behavior (right?). Case 1) is difficult to investigate because each time I try to log x or do anything with it, potrace does not crash anymore and everything works perfectly. Anyway, x seems to become negative which makes (x)/BM_WORDBITS negative and leads &bm_scanline(bm, y)[(x)/BM_WORDBITS]) to segfault. So you're right, it looks like the current issue comes from an erroneous optimization of the loop, like if the x>=0 statement would be ignored. It would probably be interesting to look at the assembly code, but I'm not sure it's a good way to spend my hours. :) > I think it would be better to have a check to validate if w is too large. You mean something like that (roughly) if( bm->w >= INT_MAX - BM_WORDBITS ) { return 0; } before the y loop ? > As a side-note. We probably have a similar issue if something is > matched at this very end. Then x++ is done a number of times, and that > could also lead to integer overflow. Maybe this is what you see in > wheezy? Let's say bm->w = INT_MAX, x = INT_MAX, y = 1: BM_GET(bm, x, y) = (bm_safe(bm, x, y) ? BM_UGET(bm, x, y) : 0)) = 0 as bm_safe(bm, x, y) = (bm_range(x, (bm)->w) && bm_range(y, (bm)->h)) = 0 as bm_range(x, (bm)->w) = ((int)(x) >= 0 && (int)(x) < (bm->w)) = 0 as x>=bm->w and bm_range(y, (bm)->h) = ((int)(y) >= 0 && (int)(y) < (bm->w)) = 1 so I guess while (!BM_GET(bm, x, y)) { x++; } would continue to loop, there would be an integer overflow, and it would continue to loop for a long long time, until x becomes positive again... But no segfault IMO because BM_UGET won't be called if x and y are not positive and there's no segfault-prone statement in BM_GET/the loop. > Addition of (unsigned): > --------------------------------- > > This is clearly superflous. First of all it will be converted back to > (int) as x is an integer. Secondly BM_WORDBITS is > 8*(int)sizeof(potrace_word) = 8*(int)sizeof(unsigned long) and that is > definitely a positive number always. > > I think this is definitely optimized away, if it ever had a meaning. +1 Cheers, Hugo -- Hugo Lefeuvre (hle) | www.owl.eu.com 4096/ ACB7 B67F 197F 9B32 1533 431C AC90 AC3E C524 065E
signature.asc
Description: PGP signature