After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756335 - When importing, date selection causes exit crash - strace included - GnuCash 2.6.9
When importing, date selection causes exit crash - strace included - GnuCash ...
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Import - QIF
2.6.9
Other Linux
: Normal critical
: ---
Assigned To: gnucash-import-maint
gnucash-import-maint
: 759889 759907 759984 760053 760111 760287 760501 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-10-10 09:24 UTC by A Cripps
Modified: 2018-06-29 23:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test qif file (86 bytes, text/plain)
2015-10-18 10:27 UTC, A Cripps
  Details
Convert qif-split:amount to numeric before using. (1.95 KB, patch)
2015-12-26 23:06 UTC, John Ralls
none Details | Review
Fixes double-to-gnc-numeric invocation error from previous patch. (2.18 KB, patch)
2015-12-27 01:32 UTC, John Ralls
rejected Details | Review

Description A Cripps 2015-10-10 09:24:28 UTC
When I try to select the date format, gnucash exits (this is not a full strace as it is hard to fit on the screen - full file available on request): 

recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"\1\1'6\0\0\0\0\311#\240\1\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 4096}], msg_controllen=0, msg_flags=0}, 0) = 32
recvmsg(4, 0x7fff2622e770, 0)           = -1 EAGAIN (Resource temporarily unavailable)
recvmsg(4, 0x7fff2622e770, 0)           = -1 EAGAIN (Resource temporarily unavailable)
write(2, "Backtrace:\n", 11Backtrace:
)            = 11
write(2, "In ", 3In )                      = 3
write(2, "ice-9/boot-9.scm", 16ice-9/boot-9.scm)        = 16
write(2, ":\n", 2:
)                      = 2
write(2, " ", 1 )                        = 1
write(2, "157", 3157)                      = 3
write(2, ": ", 2: )                       = 2
write(2, "4", 14)                        = 1
write(2, " ", 1 )                        = 1
stat("/usr/share/gnucash/scm/system/vm/frame.scm", 0x7fff2622d4c0) = -1 ENOENT (No such file or directory)
stat("/usr/share/gnucash/scm/system/vm/frame", 0x7fff2622d4c0) = -1 ENOENT (No such file or directory)
stat("/usr/share/guile/2.0/system/vm/frame.scm", {st_mode=S_IFREG|0644, st_size=6147, ...}) = 0
stat("/usr/lib/gnucash/scm/ccache/2.0/system/vm/frame.go", 0x7fff2622d550) = -1 ENOENT (No such file or directory)
stat("/usr/lib/guile/2.0/ccache/system/vm/frame.go", {st_mode=S_IFREG|0644, st_size=7045, ...}) = 0
open("/usr/lib/guile/2.0/ccache/system/vm/frame.go", O_RDONLY|O_CLOEXEC) = 14
fstat(14, {st_mode=S_IFREG|0644, st_size=7045, ...}) = 0
mmap(NULL, 7045, PROT_READ, MAP_PRIVATE, 14, 0) = 0x7f3bac675000
close(14)                               = 0
futex(0x7f3bdbe6a6f8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
write(2, "[catch #t #<catch-closure d58700"..., 38[catch #t #<catch-closure d58700> ...]) = 38
write(2, "\n", 1
)                       = 1
write(2, "In ", 3In )                      = 3
write(2, "unknown file", 12unknown file)            = 12
write(2, ":\n", 2:
)                      = 2
write(2, "   ?", 4   ?)                     = 4
write(2, ": ", 2: )                       = 2
write(2, "3", 13)                        = 1
write(2, " ", 1 )                        = 1
write(2, "[apply-smob/1 #<catch-closure d5"..., 38[apply-smob/1 #<catch-closure d58700>]) = 38
write(2, "\n", 1
)                       = 1
write(2, "In ", 3In )                      = 3
write(2, "qif-file.scm", 12qif-file.scm)            = 12
write(2, ":\n", 2:
)                      = 2
write(2, " ", 1 )                        = 1
write(2, "904", 3904)                      = 3
write(2, ": ", 2: )                       = 2
write(2, "2", 12)                        = 1
write(2, " ", 1 )                        = 1
write(2, "[check-and-parse-field #<procedu"..., 81[check-and-parse-field #<procedure 16fd030 at ice-9/boot-9.scm:1380:4 (obj)> ...]) = 81
write(2, "\n", 1
)                       = 1
write(2, "In ", 3In )                      = 3
write(2, "qif-parse.scm", 13qif-parse.scm)           = 13
write(2, ":\n", 2:
)                      = 2
write(2, " ", 1 )                        = 1


write(2, "regexp-exec", 11regexp-exec)             = 11
write(2, ":", 1:)                        = 1
write(2, " ", 1 )                        = 1
write(2, "W", 1W)                        = 1
write(2, "r", 1r)                        = 1
write(2, "o", 1o)                        = 1
write(2, "n", 1n)                        = 1
write(2, "g", 1g)                        = 1
write(2, " ", 1 )                        = 1
write(2, "t", 1t)                        = 1
write(2, "y", 1y)                        = 1
write(2, "p", 1p)                        = 1
write(2, "e", 1e)                        = 1
write(2, " ", 1 )                        = 1
write(2, "a", 1a)                        = 1
write(2, "r", 1r)                        = 1
write(2, "g", 1g)                        = 1
write(2, "u", 1u)                        = 1
write(2, "m", 1m)                        = 1
write(2, "e", 1e)                        = 1
write(2, "n", 1n)                        = 1
write(2, "t", 1t)                        = 1
write(2, " ", 1 )                        = 1
write(2, "i", 1i)                        = 1
write(2, "n", 1n)                        = 1
write(2, " ", 1 )                        = 1
write(2, "p", 1p)                        = 1
write(2, "o", 1o)                        = 1
write(2, "s", 1s)                        = 1
write(2, "i", 1i)                        = 1
write(2, "t", 1t)                        = 1
write(2, "i", 1i)                        = 1
write(2, "o", 1o)                        = 1
write(2, "n", 1n)                        = 1
write(2, " ", 1 )                        = 1
write(2, "2", 12)                        = 1
write(2, " ", 1 )                        = 1
write(2, "(", 1()                        = 1
write(2, "e", 1e)                        = 1
write(2, "x", 1x)                        = 1
write(2, "p", 1p)                        = 1
write(2, "e", 1e)                        = 1
write(2, "c", 1c)                        = 1
write(2, "t", 1t)                        = 1
write(2, "i", 1i)                        = 1
write(2, "n", 1n)                        = 1
write(2, "g", 1g)                        = 1
write(2, " ", 1 )                        = 1
write(2, "string", 6string)                   = 6
write(2, ")", 1))                        = 1
write(2, ":", 1:)                        = 1
write(2, " ", 1 )                        = 1
write(2, "(10 9 2015)", 11(10 9 2015))             = 11
write(2, "\n", 1
)                       = 1
open("/usr/lib/gnucash/libnvidia-egl-wayland.so.", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 14
fstat(14, {st_mode=S_IFREG|0644, st_size=280901, ...}) = 0
mmap(NULL, 280901, PROT_READ, MAP_PRIVATE, 14, 0) = 0x7f3bac41b000
close(14)                               = 0
open("/usr/lib/libnvidia-egl-wayland.so.", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
munmap(0x7f3bac41b000, 280901)          = 0
futex(0x7f3bc666bd88, FUTEX_WAKE_PRIVATE, 1) = 0
close(7)                                = 0
getpid()                                = 5113
munmap(0x7f3bc6387000, 3116056)         = 0
munmap(0x7f3bc6103000, 2636768)         = 0
exit_group(1)                           = ?
+++ exited with 1 +++
Comment 1 John Ralls 2015-10-10 13:47:49 UTC
The strace isn't very useful. The scheme debug that was printed to the terminal, and which can be sort-of discerned from the trace, would be better.

Please also explain exactly, step-by-step, how to induce the crash.
What distro are you using, and what version of Guile?
Comment 2 A Cripps 2015-10-10 14:33:38 UTC
Using Arch Linux - when you import a file, you are asked with a dialog which date format the file is following: d-m-y or m-d-y. When you change this drop down dialog the program will eventually crash with the regex problem. 

[adam@hark ~]$ gnucash 
libsqlite.so.0: cannot open shared object file: No such file or directory
libdbi: Failed to load driver: /usr/lib/dbd/libdbdsqlite.so
Backtrace:
In ice-9/boot-9.scm:
 157: 4 [catch #t #<catch-closure 157a8c0> ...]
In unknown file:
   ?: 3 [apply-smob/1 #<catch-closure 157a8c0>]
In qif-file.scm:
 904: 2 [check-and-parse-field #<procedure 1e75780 at ice-9/boot-9.scm:1380:4 (obj)> ...]
In qif-parse.scm:
 423: 1 [qif-parse:check-date-format (9 10 2015) (m-d-y)]
In unknown file:
   ?: 0 [regexp-exec #<regexp 19b4900> (9 10 2015) #<undefined> #<undefined>]

ERROR: In procedure regexp-exec:
ERROR: In procedure regexp-exec: Wrong type argument in position 2 (expecting string): (9 10 2015)
Comment 3 A Cripps 2015-10-10 14:34:19 UTC
[adam@hark ~]$ guile --version
guile (GNU Guile) 2.0.11
Copyright (C) 2014 Free Software Foundation, Inc.

License LGPLv3+: GNU LGPL 3 or later <http://gnu.org/licenses/lgpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Comment 4 John Ralls 2015-10-10 16:33:59 UTC
Perfect. I think I see the problem. Can you make the following change to 
/usr/share/gnucash/scm/qif-import/qif-parse.scm and test again?

@@ -400,8 +400,8 @@
   (let ((retval '()))
     (if (or (not (string? date-string))
             (not (> (string-length date-string) 0)))
-        (set! retval possible-formats))
-    (let ((match (regexp-exec qif-date-compiled-rexp date-string)))
+        (set! retval possible-formats)
+        (let ((match (regexp-exec qif-date-compiled-rexp date-string)))
       (if match
           (if (match:substring match 1)
               (set! retval (parse-check-date-format match possible-formats))
@@ -423,7 +423,8 @@
                         (memq 'm-d-y possible-formats))
                     (set! res2 (parse-check-date-format date-mdy possible-formats)))

-                (set! retval (append res1 res2))))))
+                (set! retval (append res1 res2)))))))
+
     retval))

 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

If you prefer to edit, it's just moving the last ')' from the first (set! retval... line to the second one.
Comment 5 John Ralls 2015-10-11 02:28:01 UTC
This one's pretty obvious, so committed in advance of your conversation. Fixed in stable, will turn up in the next release. Thanks for the report!
Comment 6 A Cripps 2015-10-11 07:37:33 UTC
No problem - the only thing that worries me slightly is that in that file I only have one set of (set! retval...

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;                                                                                                                                                   
;;  qif-parse:check-date-format                                                                                                                                                                                     
;;  given a list of possible date formats, return a pruned list                                                                                                                                                     
;;  of possibilities.                                                                                                                                                                                               
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;                                                                                                                                                   
(define (qif-parse:check-date-format date-string possible-formats)
  (let ((retval '()))
    (if (or (not (string? date-string))
            (not (> (string-length date-string) 0)))
        (set! retval possible-formats))
    (let ((match (regexp-exec qif-date-compiled-rexp date-string)))
      (if match
          (if (match:substring match 1)
              (set! retval (parse-check-date-format match possible-formats))

              ;; Uh oh -- this is a string XXXXXXXX; we don't know which                                                                                                                                            
              ;; way to test..  So test both YYYYxxxx and xxxxYYYY,                                                                                                                                                 
              ;; and let the parser verify the year is valid.                                                                                                                                                       
              (let* ((new-date-string (match:substring match 4))
                     (date-ymd (regexp-exec qif-date-ymd-compiled-rexp
                                            new-date-string))
                     (date-mdy (regexp-exec qif-date-mdy-compiled-rexp
                                               new-date-string))
                     (res1 '())
                     (res2 '()))
                (if (or (memq 'y-d-m possible-formats)
                        (memq 'y-m-d possible-formats))
                    (set! res1 (parse-check-date-format date-ymd possible-formats)))
                (if (or (memq 'd-m-y possible-formats)
                        (memq 'm-d-y possible-formats))
                    (set! res2 (parse-check-date-format date-mdy possible-formats)))

                (set! retval (append res1 res2))))))
    retval))
Comment 7 John Ralls 2015-10-11 14:25:42 UTC
Never seen a patch before, eh? See the + and - in the left margin? The - means "remove this line" and the + means "add this line". That's why there's a pair.
Comment 8 A Cripps 2015-10-17 09:01:39 UTC
I tried your fix, but still get an error: 

libsqlite.so.0: cannot open shared object file: No such file or directory
libdbi: Failed to load driver: /usr/lib/dbd/libdbdsqlite.so
;;; note: source file /usr/share/gnucash/scm/qif-import/qif-parse.scm
;;;       newer than compiled /usr/lib/gnucash/scm/ccache/2.0/qif-import/qif-parse.go
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling /usr/share/gnucash/scm/qif-import/qif-parse.scm
;;; compiled /home/adam/.cache/guile/ccache/2.0-LE-8-2.0/usr/share/gnucash/scm/qif-import/qif-parse.scm.go
openjdk version "1.8.0_60"
OpenJDK Runtime Environment (build 1.8.0_60-b24)
OpenJDK 64-Bit Server VM (build 25.60-b23, mixed mode)
Backtrace:
In ice-9/boot-9.scm:
 157: 6 [catch #t #<catch-closure 1b898c0> ...]
In unknown file:
   ?: 5 [apply-smob/1 #<catch-closure 1b898c0>]
In qif-file.scm:
 956: 4 [check-and-parse-field #<procedure 2302c30 at ice-9/boot-9.scm:1380:4 (obj)> ...]
In srfi/srfi-1.scm:
 619: 3 [for-each #<procedure 73f8310 at qif-file.scm:957:10 (current)> (# # # ...)]
In qif-file.scm:
 962: 2 [#<procedure 73f8310 at qif-file.scm:957:10 (current)> #]
In qif-import/qif-parse.scm:
 455: 1 [qif-parse:parse-date/format (16 10 2015) y-m-d]
In unknown file:
   ?: 0 [regexp-exec #<regexp 2691540> (16 10 2015) #<undefined> #<undefined>]

ERROR: In procedure regexp-exec:
ERROR: In procedure regexp-exec: Wrong type argument in position 2 (expecting string): (16 10 2015)
Comment 9 John Ralls 2015-10-17 18:27:02 UTC
OK, you can try this as well:
--- a/src/import-export/qif-imp/qif-file.scm
+++ b/src/import-export/qif-imp/qif-file.scm
@@ -898,7 +898,7 @@
         (let loop ((current (car objects))
                    (rest (cdr objects)))
           (let ((val (getter current)))
-            (if val
+            (if and (val (string? val))
                 (begin
                   (set! do-parsing #t)
                   (set! formats (checker val formats))))

That's adding a check for val being a string at the indicated line in qif-file.scm.

But this is kind of playing whack-a-mole because I don't have a QIF file with an already-formatted date. Given the age of this code it's apparently a pretty rare thing. Can you anonymize yours and attach it? A single transaction worth will do as long as it replicates the crash.
Comment 10 John Ralls 2015-10-17 20:25:07 UTC
I made a mistake in that patch: It should be 
--- a/src/import-export/qif-imp/qif-file.scm
+++ b/src/import-export/qif-imp/qif-file.scm
@@ -898,7 +898,7 @@
         (let loop ((current (car objects))
                    (rest (cdr objects)))
           (let ((val (getter current)))
-            (if val
+            (if (and val (string? val))
                 (begin
                   (set! do-parsing #t)
                   (set! formats (checker val formats))))
Comment 11 A Cripps 2015-10-18 10:27:10 UTC
Created attachment 313613 [details]
test qif file

anonymized (sort of) and imports in to gnucash. Causes crash when trying to import and switching date format (you may need to switch more than once to reproduce - try clicking the drop down date selector several times).
Comment 12 A Cripps 2015-10-18 10:37:48 UTC
OK, the crash stops now, at the date selector, but when you now go on to import, you get a different crash:

Backtrace:
In ice-9/boot-9.scm:
 157: 8 [catch #t #<catch-closure 137f8c0> ...]
In unknown file:
   ?: 7 [apply-smob/1 #<catch-closure 137f8c0>]
In qif-dialog-utils.scm:
 553: 6 [qif-dialog:make-memo-display (#) #<hash-table 1991dc0 2048/3517> ...]
In srfi/srfi-1.scm:
 619: 5 [for-each #<procedure 1a8c520 at qif-dialog-utils.scm:554:5 (file)> (#)]
 619: 4 [for-each #<procedure 1a8c500 at qif-dialog-utils.scm:556:8 (xtn)> (#)]
 619: 3 [for-each #<procedure 19b8990 at qif-dialog-utils.scm:566:17 (split)> (#)]
In qif-dialog-utils.scm:
 592: 2 [#<procedure 19b8990 at qif-dialog-utils.scm:566:17 (split)> #]
In unknown file:
   ?: 1 [gnc-numeric-positive-p "-10.00"]
In ice-9/boot-9.scm:
1381: 0 [#<procedure 1b263f0 at ice-9/boot-9.scm:1380:4 (obj)> "-10.00"]

ice-9/boot-9.scm:1381:15: In procedure #<procedure 1b263f0 at ice-9/boot-9.scm:1380:4 (obj)>:
ice-9/boot-9.scm:1381:15: In procedure struct_vtable: Wrong type argument in position 1 (expecting struct): "-10.00"
Comment 13 John Ralls 2015-12-10 23:33:34 UTC
Hmm, that implies that the split amount isn't getting converted into a numeric (i.e., it should be [gnc-numeric-positive-p ("-10", "100")] )
when it's inserted into the split. Unfortunately your test file imports for me without errors so I can't see where that's going wrong for you.
Comment 14 A Cripps 2015-12-19 08:00:04 UTC
@John Ralls - I can also import the file - the issue isn't that the file won't import. It's that the date selector which appears before import can occasionally crash if switched over a couple of times.
Comment 15 John Ralls 2015-12-19 15:11:48 UTC
(In reply to A Cripps from comment #12)
> OK, the crash stops now, at the date selector, but when you now go on to
> import, you get a different crash:

and

(In reply to A Cripps from comment #14)
> @John Ralls - I can also import the file - the issue isn't that the file
> won't import. It's that the date selector which appears before import can
> occasionally crash if switched over a couple of times.

Now I'm confused. With both patches in place, exactly what fails and when?
Comment 16 John Ralls 2015-12-26 23:04:52 UTC
*** Bug 759889 has been marked as a duplicate of this bug. ***
Comment 17 John Ralls 2015-12-26 23:06:43 UTC
Created attachment 317913 [details] [review]
Convert qif-split:amount to numeric before using.

A new bug confirms your last backtrace. Try the attached patch to see if it resolves the problem.
Comment 18 John Ralls 2015-12-27 01:32:50 UTC
Created attachment 317915 [details] [review]
Fixes double-to-gnc-numeric invocation error from previous patch.

This should fix the invocation error from the previous patch.
Comment 19 John Ralls 2015-12-27 01:38:38 UTC
*** Bug 759889 has been marked as a duplicate of this bug. ***
Comment 20 John Ralls 2015-12-28 00:29:41 UTC
*** Bug 759907 has been marked as a duplicate of this bug. ***
Comment 21 John Ralls 2015-12-28 00:41:57 UTC
I'm able to replicate the latest crash, and it's because the comment 10 patch was misguided, breaking all qif imports except A. Cripps's example.
Comment 22 John Ralls 2015-12-28 00:42:50 UTC
I should have added that reversing that patch allows the import to proceed normally.
Comment 23 David Carlson 2015-12-28 13:47:06 UTC
Is there a way for a windows user that is not able to compile the program to contribute to this debugging process?  Can I effectively reverse whichever patch is the culprit without recompiling the program?
Comment 24 John Ralls 2015-12-28 22:53:51 UTC
Yes, the code in question is in scheme, so you can edit it directly, even in Windows. The installed location for the file is c:\Program Files (x86)\gnucash\share\gnucash\scm\qif-import\qif-file.scm.

On most Linux installs it will be /usr/share/gnucash/scm/qif-impor/qif-file.scm
unless the user has deliberately installed it somewhere else.
Comment 25 David Carlson 2015-12-29 15:54:52 UTC
If I interpreted the patch correctly, to reverse it is as simple as changing that one line in one file.  I tried that, and GnuCash successfully imported my test file.  It also imported the file that had blank lines before the account type definition line.  This is not a complete test, but at least it works for these simple cases.
Comment 26 John Ralls 2015-12-29 16:10:57 UTC
You interpreted the patch correctly, and thanks for testing.
Comment 27 John Ralls 2015-12-29 23:08:21 UTC
So back to the original problem. The crash can be prevented by changing the line 
+        (set! retval possible-formats)
to
+        (set! retval #f)
in the patch from comment 4. That's an appropriate change, because it's returning all of the possible formats allows parsing to proceed and if the value isn't a string or is empty then the second crash results.

But it exposes a new problem: The assistant actually reparses the data based on the input from the first time you set the format in the combo box, which mutates the data. That's the real reason for it crashing if you change the setting in the combo. The fix keeps it from crashing, but it ignores changes after that so if you made a mistake you can't fix it.

I've fixed that problem too, but it's a change in C so a recompile and reinstall is required. 02ecb14 for those who want to build from git, and it will be in tomorrow's maint nightly for MSWin users.
Comment 28 John Ralls 2015-12-30 00:52:28 UTC
*** Bug 759984 has been marked as a duplicate of this bug. ***
Comment 29 John Ralls 2016-01-01 22:26:27 UTC
*** Bug 760053 has been marked as a duplicate of this bug. ***
Comment 30 John Ralls 2016-01-07 19:37:25 UTC
*** Bug 760287 has been marked as a duplicate of this bug. ***
Comment 31 John Ralls 2016-01-08 22:09:38 UTC
*** Bug 760287 has been marked as a duplicate of this bug. ***
Comment 32 John Ralls 2016-01-10 04:32:26 UTC
*** Bug 760111 has been marked as a duplicate of this bug. ***
Comment 33 John Ralls 2016-01-12 03:50:01 UTC
*** Bug 760501 has been marked as a duplicate of this bug. ***
Comment 34 John Ralls 2018-06-29 23:43:42 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=756335. Please update any external references or bookmarks.