Bug 391251

Summary: Double free or corruption when creating a new asset account.
Product: [Applications] kmymoney Reporter: José Pekkarinen <koalinux>
Component: generalAssignee: KMyMoney Devel Mailing List <kmymoney-devel>
Status: RESOLVED FIXED    
Severity: normal CC: alexandref75, lukasz.wojnilowicz
Priority: NOR    
Version: git (master)   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In: 5.0.2
Sentry Crash Report:
Attachments: backtrace and memdump of the issue.
Example file to reproduce the issue
Crash from example.kmy
Backtrace of the crash
Corruption using latest gentoo ebuild.
Crash on -Og -g
gdb backtrace after crash
Check if institutionItemFromId returns nullptr

Description José Pekkarinen 2018-03-01 07:05:53 UTC
Created attachment 111103 [details]
backtrace and memdump of the issue.

Hi,

I hit a kmymoney crash when trying to add a new assets account to my file. Steps
to reproduce are the following:

1) Open kmymoney 5 with a file loaded which contains a couple of institutions with several accounts.
2) Follow the wizard to create a new assets account.

When the wizard is closing, the application will crash without saving the file.

Thanks!

José.
Comment 1 José Pekkarinen 2018-03-01 07:13:43 UTC
Created attachment 111104 [details]
Example file to reproduce the issue

This example file doesn't have any institution, but creating the institution
on the account wizard still makes my crash reproducible.
Comment 2 José Pekkarinen 2018-03-01 07:15:25 UTC
Created attachment 111105 [details]
Crash from example.kmy

This is based in the dumb example, so I mark the former as obsolete.
Comment 3 José Pekkarinen 2018-03-08 08:00:11 UTC
As of today:

$ ./kmymoney
Using Wayland-EGL
WebConnect: Try to connect to WebConnect server
WebConnect: Connect to server failed
WebConnect: Running in server mode
Plugins: checkprinting loaded
Plugins: csvexporter loaded
Plugins: csvimporter loaded
Plugins: gncimporter loaded
Plugins: icalendarexporter loaded
Plugins: qifexporter loaded
Plugins: qifimporter loaded
Plugins: reconciliation report loaded
Plugins: weboob loaded
Online plugins found 1
Cost center model created with items 0
Payees model created with items 0
reading file
start parsing file
startDocument
reading securities
endDocument
*** Error in `./kmymoney': double free or corruption (!prev): 0x000056057011a520 ***
Comment 4 José Pekkarinen 2018-03-08 08:01:41 UTC
Created attachment 111249 [details]
Backtrace of the crash
Comment 5 wojnilowicz 2018-03-15 08:19:40 UTC
(In reply to José Pekkarinen from comment #4)
> Created attachment 111249 [details]
> Backtrace of the crash

I cannot reproduce your crash. Could you provide detailed list of steps to reproduce?

I suspect it might be related to some compiler optimizations.
Could you provide more information about how do you compile your KMM? I would like to know:
1) your compiler (gcc, clang)
2) your build mode (Debug, Release)
3) your compiler flags (mine are "-Og -g")
Comment 6 José Pekkarinen 2018-03-15 08:43:48 UTC
Created attachment 111410 [details]
Corruption using latest gentoo ebuild.

For me it's kinda easy:

1) Open kmymoney.
2) Go to account tabs.
3) Open the wizard to create an account.
4) Select institution.
5) Name the account, and select it as asset account.
6) Select a parent account(not selecting a parent account several times succeed)
7) Close the confirmation piece of the wizard.

That will produce the crash on the closure of the wizard.

José.
Comment 7 wojnilowicz 2018-03-15 08:49:12 UTC
(In reply to José Pekkarinen from comment #6)
> Created attachment 111410 [details]
> Corruption using latest gentoo ebuild.
> 
> For me it's kinda easy:
> 
> 1) Open kmymoney.
> 2) Go to account tabs.
> 3) Open the wizard to create an account.
> 4) Select institution.
> 5) Name the account, and select it as asset account.
> 6) Select a parent account(not selecting a parent account several times
> succeed)
> 7) Close the confirmation piece of the wizard.
> 
> That will produce the crash on the closure of the wizard.
> 
> José.

It still does not crash for me and there is no useful information for me in backtrace you posted.
Comment 8 José Pekkarinen 2018-03-15 12:02:40 UTC
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/6.4.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-6.4.0-r1/work/gcc-6.4.0/configure --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/6.4.0 --includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include --datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0 --mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/man --infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/info --with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/g++-v6 --with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/python --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 6.4.0-r1 p1.3' --disable-esp --enable-libstdcxx-time --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --enable-multilib --with-multilib-list=m32,m64 --disable-altivec --disable-fixed-point --enable-targets=all --disable-libgcj --enable-libgomp --disable-libmudflap --disable-libssp --disable-libcilkrts --disable-libmpx --enable-vtable-verify --enable-libvtv --enable-lto --without-isl --enable-libsanitizer --enable-default-pie --enable-default-ssp
Thread model: posix
gcc version 6.4.0 (Gentoo 6.4.0-r1 p1.3)

$ grep CFLAGS /etc/portage/make.conf
CFLAGS="-O2 -march=skylake -mtune=skylake -fomit-frame-pointer -pipe"
CXXFLAGS="${CFLAGS}"

I assume the ebuild is building for release, but I cannot tell, ebuild doesn't
light anything about it.
Comment 9 wojnilowicz 2018-03-15 12:29:18 UTC
(In reply to José Pekkarinen from comment #8)
> $ gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/6.4.0/lto-wrapper
> Target: x86_64-pc-linux-gnu
> Configured with:
> /var/tmp/portage/sys-devel/gcc-6.4.0-r1/work/gcc-6.4.0/configure
> --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
> --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/6.4.0
> --includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include
> --datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0
> --mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/man
> --infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/info
> --with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/g++-v6
> --with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/6.4.0/python
> --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt
> --disable-werror --with-system-zlib --enable-nls --without-included-gettext
> --enable-checking=release --with-bugurl=https://bugs.gentoo.org/
> --with-pkgversion='Gentoo 6.4.0-r1 p1.3' --disable-esp
> --enable-libstdcxx-time --enable-shared --enable-threads=posix
> --enable-__cxa_atexit --enable-clocale=gnu --enable-multilib
> --with-multilib-list=m32,m64 --disable-altivec --disable-fixed-point
> --enable-targets=all --disable-libgcj --enable-libgomp --disable-libmudflap
> --disable-libssp --disable-libcilkrts --disable-libmpx
> --enable-vtable-verify --enable-libvtv --enable-lto --without-isl
> --enable-libsanitizer --enable-default-pie --enable-default-ssp
> Thread model: posix
> gcc version 6.4.0 (Gentoo 6.4.0-r1 p1.3)
> 
> $ grep CFLAGS /etc/portage/make.conf
> CFLAGS="-O2 -march=skylake -mtune=skylake -fomit-frame-pointer -pipe"
> CXXFLAGS="${CFLAGS}"
> 
> I assume the ebuild is building for release, but I cannot tell, ebuild
> doesn't
> light anything about it.

I don't know anything about ebuild, but I believe your builds are heavily optimized by the use of "-mtune=skylake". I assume, it's the cause of your issues.
For the start, could you try to set CFLAGS and CXXFLAGS to "-Og -g" and see if you still can reproduce the crash?
Comment 10 Jack 2018-03-16 20:49:31 UTC
Jose - Exactly which ebuild are you using?  5.0.0-r1 from the main tree, or 5.0.9999 (git 5.0) or 9999 (git master) from the KDE overlay?  If you do "ebuild /path/to/ebuild configure" the build.log should have the output from cmake, which should confirm the build type.

As suggested in comment 9, it is possible that the skylake specification is causing a problem - can you try backing off to a more generic setting and see if that eliminates the crash?

(For those not familiar with Gentoo linux, it is a source based distro, and an ebuild is a package definition file, with all the information necessary to download, configure, compile, and install the package - either from tarball or git.)
Comment 11 José Pekkarinen 2018-03-17 21:02:33 UTC
(In reply to Jack from comment #10)
> Jose - Exactly which ebuild are you using?  5.0.0-r1 from the main tree, or
> 5.0.9999 (git 5.0) or 9999 (git master) from the KDE overlay?  If you do
> "ebuild /path/to/ebuild configure" the build.log should have the output from
> cmake, which should confirm the build type.

5.0.0-r1 version:

$ emerge -s kmymoney
  
[ Results for search key : kmymoney ]
Searching...

*  app-office/kmymoney
      Latest version available: 5.0.0-r1
      Latest version installed: 5.0.0-r1
      Size of files: 12,990 KiB
      Homepage:      https://kmymoney.org
      Description:   Personal finance manager based on KDE Frameworks
      License:       GPL-2
 
> As suggested in comment 9, it is possible that the skylake specification is
> causing a problem - can you try backing off to a more generic setting and
> see if that eliminates the crash?

I'll do, but bear with it because of me not having all the availability I'd want to have. I'll come back no later than one week from now though.

José.
Comment 12 José Pekkarinen 2018-03-20 07:26:32 UTC
Created attachment 111520 [details]
Crash on -Og -g

Seems it's fully reproducible if I reemerge using the following command line:

CFLAGS="-Og -g" CXXFLAGS=$CFLAGS emerge -1 kmymoney
Comment 13 Jack 2018-03-22 22:34:40 UTC
Created attachment 111567 [details]
gdb backtrace after crash

I can confirm the crash.  It is not just creating a new account but only if the new account is assigned to an institution.  

Separately, I cannot reproduce my original crash (on closng KMM if no file is currently open - with either 5.0.1 or git 5.0 head) so they are likely separate issues, even if optimization flags are involved in both.

I am attaching a backtrace from gdb.  Note in this case I created the institution as part of creating the account, and I took all default values except for the names.
Comment 14 wojnilowicz 2018-03-24 07:15:17 UTC
Created attachment 111589 [details]
Check if institutionItemFromId returns nullptr

Jack, your backtrace is helpful. Please test whether attached patch makes any difference.
Comment 15 Jack 2018-03-24 17:01:58 UTC
The patch does not apply to 5.0.1 or to 5.0 git head because they have different signature on 

 void InstitutionsModel::slotObjectModified(File::Object objType, const QString&

I'll try manually modifying the patch, but I don't know if other changes between 5.0 and master will cause issues.
Comment 16 Jack 2018-03-24 22:24:53 UTC
It also does not apply to git master.  The one chunk referred to in Comment 15 always fails.  Does the patch need to be rebased?  (I've tried to manually modify it, but somehow I always end up with bad syntax.)
Comment 17 wojnilowicz 2018-03-25 08:10:22 UTC
(In reply to Jack from comment #16)
> It also does not apply to git master.  The one chunk referred to in Comment
> 15 always fails.  Does the patch need to be rebased?  (I've tried to
> manually modify it, but somehow I always end up with bad syntax.)

The patch has been prepared for master branch, because the bug is reported against master branch.
I think you've got some internal issues with applying the patch, because even after recent changes the patch needs no change to apply cleanly.

Are you sure that you are on master branch and your HEAD is at 
"Show values right aligned in online job view" by Thomas?
Comment 18 Alexandre 2018-03-25 19:19:18 UTC
It applies cleanly for me but does not fix the problem. It still fail with an seg fault when creating an account.
Comment 19 Jack 2018-03-27 22:34:23 UTC
I still don't understand how badly my various build directories were messed up, but on a clean git head, the patch now applies (a few lines off for chunk 2) and I do not get the crash.  Hopefully José can also test.

Alexandre - what optimization options did you use, and are you compiling manually, or are you also using a Gentoo ebuild?
Comment 20 wojnilowicz 2018-03-28 08:07:20 UTC
(In reply to Jack from comment #19)
> I still don't understand how badly my various build directories were messed
> up, but on a clean git head, the patch now applies (a few lines off for
> chunk 2) and I do not get the crash.  Hopefully José can also test.
> 
> Alexandre - what optimization options did you use, and are you compiling
> manually, or are you also using a Gentoo ebuild?

Considering your messed build directories, could you confirm 100%, that my patch makes the difference? What I want to know is: do you still get crash when you compile on a clean git head without my patch.


As a side note, I compiled KMM on MS Windows and KMM doesn't crash for me there either. It leads me to conclusion that this may be hardware (mine is rather old) related issue or Gentoo specific issue.
Comment 21 wojnilowicz 2018-03-29 08:10:40 UTC
Please all try again now with latest git master.
Comment 22 Jack 2018-03-29 14:59:05 UTC
Yesterday, I was finally able to get two clean builds from git master.  Crash without the patch, and no crash with it.  These were direct builds, not using a Gentoo ebuild.  Will try again later with the new commits.
Comment 23 José Pekkarinen 2018-03-29 15:39:35 UTC
(In reply to Jack from comment #22)
> Yesterday, I was finally able to get two clean builds from git master. 
> Crash without the patch, and no crash with it.  These were direct builds,
> not using a Gentoo ebuild.  Will try again later with the new commits.

Hi there,

Sorry for the long wait, I got some cycles to test it and it does the
trick for me. Any chance to get it in 5.0 branch?

Thanks for the work!

José.
Comment 24 Jack 2018-03-29 15:45:29 UTC
Thanks for confirming.  We've just released 5.0.1, so this will likely be in 5.0.2 (or whatever number is used next) but it will probably be at least a month or two before we make the next release.

Question:  I have not tested - but is it possible to create the new account without an institution, and then later add the institution to the account?  If so, that would be a reasonable workaround for the interim.  Having the institution assigned is really only a convenience for now - I don't think the program actually uses it for anything right now.
Comment 25 wojnilowicz 2018-03-29 16:40:55 UTC
I hope Thomas will manage to get it into 5.0 branch in one of his merge windows.
I marked the commit which fixed the bug.

Thank you all for testing.
Comment 26 Thomas Baumgart 2018-03-30 06:17:08 UTC
Done. A slightly modified version of the patch is available in the 5.0 branch.
Comment 27 José Pekkarinen 2018-03-30 09:27:42 UTC
(In reply to Thomas Baumgart from comment #26)
> Done. A slightly modified version of the patch is available in the 5.0
> branch.

Awesome! Thanks!

José.
Comment 28 Alexandre 2018-03-30 14:04:51 UTC
The latest master works for me. No seg faults when creating an account Thanks,
Comment 29 Jack 2018-03-30 20:20:13 UTC
Minor note - it turns out the Gentoo ebuild for pulling from git master had a bogus line actually pulling from 5.0 instead.  It's been fixed.  That was why the patch wouldn't apply to that ebuild.