Re: [lmi] Segfault in wxGrid

[Greg Chicares]

On 2016-06-21 23:21, Vadim Zeitlin wrote:
> On Tue, 21 Jun 2016 20:08:19 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-06-21 18:53, Vadim Zeitlin wrote:
> GC> [...]
> GC> >  I know how to fix the problem and will do it soon, after I think a bit
> GC> > more if there is no better way to do it, but what puzzles me is that I
> GC> > don't see any recent changes to wxGrid which could explain why you 
> hadn't
> GC> > seen this bug before. Moreover, I've retested with wxWidgets 3.1.0 
> (which I
> GC> > think was used before, please correct me if I'm wrong) and I see exactly
> GC> > the same crash. So either this is not a new crash, or it's due to 
> changes
> GC> > in lmi, rather in wx -- but then I don't see any changes that could 
> explain
> GC> > this in lmi neither. Should I continue to try to find which change broke
> GC> > this or maybe it's not a new bug, after all?
> GC> 
> GC> The same steps cause a segfault with earlier versions too, even with the
> GC> oldest version I have stored in my current VM, lmi-20150413T1546Z, which
> GC> is over a year old, and appears to use wx-3.0.0
> 
>  Thank you for confirming this, it's reassuring to know that I didn't miss
> some mysterious regression.
> 
>  Unfortunately I still don't see any really good way of fixing this bug, so
> for now I've committed the ugly fix which I originally had in mind:
> 
> https://github.com/wxWidgets/wxWidgets/commit/ff5981230a1adc46c4ab3d895e2ff5d0c4cf96f0
> 
> It does solve the crash for me, but it's really not great to have to do
> things like this manually, the code should ideally be written in a way to
> not allow using the destroyed objects organically, but this is just not the
> case in wxGrid and it's too dangerous/difficult to change it now...

Okay. It looks like the best that can reasonably be done.

>  Worse, while debugging this I realized that MultiDimGrid classes in lmi
> itself were using raw pointers too and my attempts to work around the bug
> with (minimal) changes in lmi itself failed because of this: trying to hide
> the in-place edit control from lmi code doesn't work because by the time we
> do it, it's already too late because the wxTreeCtrl in DatabaseView had
> been already destroyed and MultiDimGridTableAny used by MultiDimGridTable
> keeps (raw!) pointers to the item data stored in that already destroyed
> tree. I'm not sure at all what would be the right thing to do here, but I'm
> almost certain that the current approach is not good at all :-(

Sadly, the product editor is a red-headed stepchild of lmi. Trying to
improve it incrementally doesn't make much sense.

Replacing floating point with a currency class is simple compared to
rewriting old code that traffics in raw pointers.

>  Anyhow, once again, the most important thing right now is fixing the crash
> and the commit above does it, I hope you can still update your wxWidgets
> version for this month release. If not, please let me know and I'll
> continue with my attempts to work around the bug in lmi itself, but please
> be warned that it will require non-trivial changes (at least the wx commit
> above demonstrably can't add any new crashes...).

A segfault is always important, but I don't think this one is urgent:
it's been in production for quite a while, and few if any end users
these days use the product editor--so there's no need to try to work
around this issue by changing lmi. Our code-freeze date for this month's
release is Thursday. Upgrading wx always runs the risk of bringing in
new anomalies. I believe we should upgrade wx only after this month's
release, so that we have plenty of time for testing the upgrade.

> GC> BTW, that version gives a deeper trace of the call stack (below), which
> GC> makes me wonder whether we've removed some useful debugging option.
> 
>  I think this was a side effect of compiler upgrade, quoting
> https://gcc.gnu.org/onlinedocs/gcc-4.9.1/gcc/Optimize-Options.html
> 
> -fomit-frame-pointer
> 
>       ...
> 
>       Starting with GCC version 4.6, the default setting (when not
>       optimizing for size) for 32-bit GNU/Linux x86 and 32-bit Darwin x86
>       targets has been changed to -fomit-frame-pointer. The default can
>       be reverted to -fno-omit-frame-pointer by configuring GCC with the
>       --enable-frame-pointer configure option.
> 
>       Enabled at levels -O, -O2, -O3, -Os. 
> 
> Without the frame pointer (EBP) and without any extra auxiliary information
> (such as saved in the special FPO section of debug information files by
> Microsoft tools) it becomes impossible to reconstruct the stack, in
> general, and I'm afraid this is what might happen here.
> 
>  I believe we should add -fno-omit-frame-pointer to wx_{cc,cxx}_flags to
> have better stack traces with the current compiler. I'm not sure how
> exactly do you generate them,

Generate what? The flags? (See 'install_wx.make':
  # Pass gcc options in $CC and $CXX, not $*FLAGS--explanation here:
  #   http://lists.nongnu.org/archive/html/lmi/2013-07/msg00001.html
  wx_cc_flags    :=
  wx_cxx_flags   := -std=c++11
where I suppose I should add '-fno-omit-frame-pointer' to both, as
well as adding it to lmi's own flags--and wxPdfDoc's, too.)

Or...generate the stack traces that I've posted here? I've been using
the "Dr MinGW" JIT debugger for that. Would the wx stack walker be a
suitable replacement? If so, then perhaps we should add that next
month when we upgrade wx.

> however, so I didn't test it myself yet,
> please let me know if you'd like me to.

Let me try doing that myself first.