On 3 October 2017 at 21:18, Dirk Hohndel <[email protected]> wrote: > >> On Oct 3, 2017, at 11:02 AM, Lubomir I. Ivanov <[email protected]> wrote: >>>> yey, a compiler bug (stack corruption) in: >>>> gcc version 5.3.0 (i686-posix-dwarf-rev0, Built by MinGW-W64 project) >>>> >>>> in divelistview.cpp, for this: >>>> for (int col = DiveTripModel::NR; col < DiveTripModel::COLUMNS; ++col) { >>>> >>>> instead of incrementing col up to 16, it goes up to 2689. >>> >>> Sweet. If you look at the preprocessor output, is it the >>> DiveTripModel::COLUMNS >>> that is incorrectly expanded, or is it really the code generation? >>> >>> And given that Gaetan sees this on Arch... different compiler, right? >>> >>> WEIRD >>> >> >> apparently, the stack gets corrupted by out-of-bounds access...how? >> i'm not sure, but i won't disassemble this as there is an easy fix and >> we are doing something wrong. >> >> divelistview.cpp has this: >> static int defaultWidth[] = { 70, 140, 90, 50, 50, 50, 50, >> 70, 50, 50, 70, 50, 50, 5, 500}; >> >> the loop uses the 'col' iterator which is clamped up to >> DiveTripModel::COLUMNS, but DiveTripModel::COLUMNS is larger than the >> N of elements in defaultWidth. >> >> so the first out-of-bound access to: >> defaultWidth[col] >> >> modifies the loop's limit to something like 2689 and it proceeds with >> the out-of-bound access until a SIGSEGV. > > That makes more sense. > We need to figure out why we overwrite the stack! Can you set a memory > breakpoint to see where this happens? >
i'm pretty sure we overwrite it with the following, when 'col' is OOB: defaultWidth[col] = width but it's very hard to predict if the OOD override will harmful or not. >> fixes: >> 1) add another element to defaultWidth[] for 'country' and iterate col >> up to the size of defaultWIdth[]. > > can you submit a patch? yes, that's the trivial fix. please, confirm with option 2. > >> 2) move defaultWidth as a method in DiveTripModel, for safety > i was thinking about hardcoding the pixel values in DiveTripModel inside some method with error checking instead of a int array in divelistview.cpp. the idea is that once a new column is added the author should also modify the method in question to return this column's width and that would happen in the same source file. lubomir -- _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
