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 778883 - Assertion fails in gspell-inline-checker-text-buffer.c, function check_subregion()
Assertion fails in gspell-inline-checker-text-buffer.c, function check_subreg...
Status: RESOLVED FIXED
Product: gspell
Classification: Other
Component: general
1.2.x
Other Linux
: High critical
: ---
Assigned To: gspell maintainers
gspell maintainers
https://retrace.fedoraproject.org/faf...
Depends on:
Blocks:
 
 
Reported: 2017-02-18 19:12 UTC by Sébastien Wilmet
Modified: 2017-04-02 15:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test word boundaries with RTL text (921 bytes, text/plain)
2017-02-22 13:44 UTC, Sébastien Wilmet
Details

Description Sébastien Wilmet 2017-02-18 19:12:13 UTC
See:
https://retrace.fedoraproject.org/faf/problems/bthash/?bth=bced18e7f3d216658e7cf505c90f37e74d9ad2dd&bth=fe415fbcc1b71283cdda2d7f8bb72ee23b460d9e

The backtrace for latexila is more complete (check_subregion() is inlined):
https://retrace.fedoraproject.org/faf/reports/1500137/

So the second g_assert_cmpint() in check_subregion() fails.

I think it's with RTL (right-to-left) text, or LTR mixed with RTL.
Comment 1 Sébastien Wilmet 2017-02-22 13:09:50 UTC
If the problem comes from RTL text, here is a related bug in GtkSourceView: bug #778928
Comment 2 Sébastien Wilmet 2017-02-22 13:44:04 UTC
Created attachment 346443 [details]
Test word boundaries with RTL text

Small test case with RTL text, the output is:

0 3 7 11 15
15 12 8 4 0

So it works fine, gtk_text_iter_forward_word_end() always moves forward in the buffer, and gtk_text_iter_backward_word_start() moves backward. gtk_text_iter_starts_word() and gtk_text_iter_inside_word() seems to work fine too.
Comment 3 Sébastien Wilmet 2017-02-22 14:11:49 UTC
So when reading the code of check_subregion() and adjust_iters(), we know that:

1) After adjust_iters(), 'start' doesn't start a word.
 1.a) It means that _gspell_text_iter_backward_word_start() has not been called in adjust_iters().
 1.b) Which means that 'start' is outside a word (in blank or punctuation) or ends a word.

2) _gspell_text_iter_forward_word_end (&word_start) has moved 'word_start' (and we suppose that is has moved it forward, not backward). Since 'word_start' was initially outside a word or at a word end, the next word start is necessarily forward, so the g_assert_cmpint() is correct.

So there is necessarily a bug in _gspell_text_iter_*() or gtk_text_iter_*() functions (related to word boundaries). I can re-read the _gspell_text_iter_*() implementation, to see if there is a bug.

In the meantime, it would be useful to add a g_assert() in adjust_iters(), to verify that if _gspell_text_iter_backward_word_start() is called, 'start' is now correctly at a word start. To check the point 1.a above.
Comment 4 Sébastien Wilmet 2017-02-22 14:54:10 UTC
I've pushed commit 50b0061885c77cb4cd8e0ac1fa2d1b30e70b0dec to add assertions in adjust_iters().

I've re-read the _gspell_text_iter_*() implementation, and I don't see any bugs. Maybe there is a bug in gtk_text_iter_*() or pango.
Comment 5 Sébastien Wilmet 2017-02-22 15:21:26 UTC
And pushed commit 043609960097f80d99ce4c166892b291df06aae3 to add more assertions in check_subregion(), to see if it fails earlier.

Worst case, if we still don't understand why it fails, we can replace the g_asserts by g_returns, to avoid the crash.
Comment 6 Sébastien Wilmet 2017-02-24 12:14:39 UTC
I've pushed commit a387b11dd402639dea5832a1a1139fa2885eb40f on the gspell-1-2 branch, to replace the g_assert_cmpint() by a g_return_if_fail() to avoid the crash for the 1.2 version.

I'll release the 1.2.3 version soon, so we will normally see that the crash doesn't occur anymore with the 1.2 version. And with the 1.3/1.4 versions we will have more information since there are now more assertions.

I keep this bug open to not forget about it.
Comment 7 Sébastien Wilmet 2017-04-02 15:27:45 UTC
I've pushed the
commit 79640e6673722d5cabd3acb9af9fccb85451ac17
on the gspell-1-4 branch, to replace the assertions by g_returns.

The check_subregion() function has been completely rewritten on the master branch, to improve the performances, see bug #776811. So on master I've pushed this change:
commit bc52e8383c47c2f295e91ef7f4d77cd9e4b98a6c

So now I consider the bug fixed.