tags 474313 patch
thanks
On Sun, Jul 13, 2008 at 07:31:57PM +0200, Georges Khaznadar wrote:
Hello, I tried to reproduce the bug about a 100 µF capacitor, the way
you described it:
To reproduce, start ktechlab, create a new circuit, drag a capacitor
onto it, select the capacitor, then change its value in the top bar.
and the bug was not triggered. I am using the version 0.3-9 of ktechlab,
with an amd64 platform.
Strange that it's so different on amd64 - after looking more closely, I was
able to identify two distinct bugs in the code.
I retried with 0.3-9 inside a sid chroot on my i386 machine. First of all,
this version *crashed* when I dragged any symbol onto a new circuit. I
recompiled with debugging symbols and -fno-inline using gcc 4.3.1, and got:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb5e7b700 (LWP 32442)]
0x082455fd in Map::reset (this=0x8941480) at matrix.cpp:308
308 (*m_map)[i][j] = 0;
(gdb) bt
#0 0x082455fd in Map::reset (this=0x8941480) at matrix.cpp:308
#1 0x082466ab in Map (this=0x8941480, size=2) at matrix.cpp:292
#2 0x08246776 in Matrix (this=0x893b958, n=2, m=0) at matrix.cpp:35
#3 0x0824220d in ElementSet (this=0x8941920, circuit=0x893ab88, n=2, m=0) at
elementset.cpp:32
#4 0x0823f77f in Circuit::init (this=0x893ab88) at circuit.cpp:203
...
It turned out that Map::Map() creates a new ETMap(m_size) and then calls
reset(), which assumes that *m_map is square. However, the new
ETMap(m_size) only creates a vector that contains two *empty* vectors, so
reset() would overwrite random memory. The attached patch fixes this by not
calling reset() and instead initializing the array properly.
Recommendation to upstream authors: Use valgrind!
OK, this allowed me to actually trigger the bug that I originally reported,
and which still existed. It turned out to be infinite recursion:
#135 0x0814ec5b in DoubleSpinBox::valueChanged (this=0xa34ccb0,
t0=9.9991e-05) at doublespinbox.moc:95
#136 0x0814f63a in DoubleSpinBox::checkIfChanged (this=0xbf7d4aac) at
doublespinbox.cpp:210
#137 0x0814f7c8 in DoubleSpinBox::qt_invoke (this=0xa34ccb0, _id=56,
_o=0xbf7d4bdc) at doublespinbox.moc:103
#138 0xb6582f6d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#139 0xb65839f0 in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#140 0xb68c7960 in QSpinBox::valueChanged () from /usr/lib/libqt-mt.so.3
#141 0xb669a4c6 in QSpinBox::valueChange () from /usr/lib/libqt-mt.so.3
#142 0xb668cd59 in QRangeControl::setValue () from /usr/lib/libqt-mt.so.3
#143 0xb6698b23 in QSpinBox::setValue () from /usr/lib/libqt-mt.so.3
#144 0xb669a02b in QSpinBox::interpretText () from /usr/lib/libqt-mt.so.3
#145 0xb6698385 in QSpinBox::value () from /usr/lib/libqt-mt.so.3
#146 0xb6699987 in QSpinBox::currentValueText () from /usr/lib/libqt-mt.so.3
#147 0xb6699bfa in QSpinBox::selectAll () from /usr/lib/libqt-mt.so.3
#148 0xb669a2fb in QSpinBox::updateDisplay () from /usr/lib/libqt-mt.so.3
#149 0xb669a4a8 in QSpinBox::valueChange () from /usr/lib/libqt-mt.so.3
#150 0xb668cd59 in QRangeControl::setValue () from /usr/lib/libqt-mt.so.3
#151 0xb6698b23 in QSpinBox::setValue () from /usr/lib/libqt-mt.so.3
#152 0x0814fb23 in DoubleSpinBox::setValue (this=0xa34ccb0,
value=9.9991e-05) at doublespinbox.cpp:78
#153 0x080ca642 in ItemInterface::slotSetData (this=0xa17dad0, [EMAIL
PROTECTED], value={d = 0xbf7d4fcc}) at iteminterface.cpp:570
#154 0x080cab29 in ItemInterface::tbDataChanged (this=0xa17dad0) at
iteminterface.cpp:480
#155 0x080cc668 in ItemInterface::qt_invoke (this=0xa17dad0, _id=4,
_o=0xbf7d50bc) at iteminterface.moc:107
#156 0xb6582f6d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#157 0xb658388d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#158 0x0814ec5b in DoubleSpinBox::valueChanged (this=0xa34ccb0,
t0=9.9991e-05) at doublespinbox.moc:95
The first and last line of the above are the same: The code recurses
forever updating the widget. This is because in
DoubleSpinBox::checkIfChanged(), the check m_lastEmittedValue == newValue
never becomes true due to a rounding error.
The bug is that the comparison takes place using a double value (e.g.
100.0e-6 for 100uF), whereas the widget stores two items, an integer 100
and a multiplier (like 1.0e-6 for micro). Converting the two back and forth
never gives an exact match.
(Incidentally, I find it strange that the widget needs to emit I have
changed in response to it receiving a you have changed. It smells wrong,
but I don't understand the code well enough to say whether it's a bug.)
My fix is to perform the above comparison on the two values (integer and
multiplier) rather than the combined double. The patch is attached. I've
only tested it lightly!
Cheers,
Richard
--
__ _
|_) /| Richard Atterer
| \/¯| http://atterer.net
¯ '` ¯
--- ./src/gui/doublespinbox.cpp.orig2008-07-13
