-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spell Check plugin crashes with color-variant-selector emojis #1272
Comments
WFM latest git Geany with default backend and en_AU dictionary, perhaps its dependent on your backend and/or language. Just for the record they are Characters U+1F900 and U+1F982 here followed by VS16 colour variant selector not ZWJ. Here they show as grinning and smiling rather than the symbols shown in the Unicode chart, but they are recent Unicode IIUC so maybe my font has not caught up yet. |
Note: Occurs when "Toggle spell check" option is enabled. Disabling the option, no crash. Same problem with 27af15f with reverts to allow use with Geany 1.38 (5e13096 and 01b19d2). Unable to build for Geany git/1.39 because Lua appears to require another Scintilla update. Language doesn't seem to matter. I tried en_AU, en_GB, en_US. Font setting for editor and symbol list don't seem to matter. Tried Hack, Noto Sans, Courier Prime. Installed emoji font is noto-fonts-emoji 20220920. Don't know how to tell what backend is being used. Terminal output:
|
Ok, but still doesn't crash, maybe its been fixed.
Neither do I, but I think its probably chosen by
Can you run Geany under GDB and get a backtrace when it crashes (don't forget to continue if needed so you get it all before you paste). |
Just disable Geanylua |
I just noticed that your terminal output shows it aborts (not crashes) in the C++ library, so thats not Geany code, either the spell checker or Scintilla, but the backtrace will tell. |
Forgot to mention earlier, using GTK+ v3.24.38 and GLib v2.78.0 geany git/3787677de; without rebuilding plugins, same issue. Rebuilding geany-plugins 27af15f for geany-git (without reverts), same issue. Following obtained from this build. Configure output.
gdb backtrace
|
I am using the same git version of Geany and plugins as you. Hmmm, dunno why there are no symbols for libgeany in the backtrace, I thought it defaulted to a debug build that has symbols, are you absolutely sure you ran the git built version, not a system installed version (which usually don't have symbols)? The problem is in the C++ library called from libgeany, which means from Scintilla since Geany is C not C++, but without symbols its not possible to trace where and why it goes wrong. But its less likely to be anything specific about the spell checker except it triggers something in Scintilla. |
I built and installed geany as a package, but looking through the PKGBUILD, don't see anything turning off debug. Tried rebuilding with debug package. Is this what you need?
|
There is nothing turning it on either, who knows what weird distro package builds do ;-) I suggest that if building from git it is best to actually do that, there are far fewer moving parts:
Note:
I did try to have this incantation added to the readme or install as the simplest build method, but there were so many objections from people complaining it did not comply with their idea of a "proper" way to build and other variations and complications and whatifs that I gave up and so everyone is left on their own :-(. |
Ok, your backtrace came in while I was typing. The problem appears to be in Scintilla trying to draw an indicator, probably the squiggly underscore marking spelling. @nyamatongwe can you tell from that if Geany is passing invalid parameters to Scintilla or if its triggering a corner case in Scintilla (maybe applying the indicator between the base emoji and the vs16?) Edit: PlatGTK.cxx:623 indexing a vector with 0, which probably means the vector is empty. Edit edit: and that explains why it does not crash here, indexing an empty vector is unchecked and UB but @xiota is using a checked version of libc++ so it aborts instead. |
The Add a check for empty size at the top of |
It is undefined behaviour, so it can do anything, including aborting, which is what the version of stdlibc++ used by the OP decides to do, even if mine and yours and most others don't. @xiota as you are the only one with a standard library that conveniently aborts can you try adding a line after
and see if it still aborts? |
Geany no longer aborts with |
Committed fix with 652df8. |
Neat thanks, @xiota now you just need to add the patch to geany/geany#3551 |
Isn't that too convoluted? Creating a PR for a PR? |
I think you can create a PR on the fork @kugel- used to make the PR, not on the actual Geany repo1, and he can just merge it to his PR if ok. Otherwise it will wait for the next Scintilla release and "somebody" making a new PR for that, but since it is undefined behaviour it would be good to fix sooner. Additionally it could be added as a separate PR on current Scintilla in Geany, but it might still need to be added to the upgrade PR so it doesn't get removed or cause merge conflicts. Footnotes
|
When I tested, I put the conditional return statement before the
I noticed in your commit that you put it after. Could
|
The backtrace shows that the abort you are seeing is not from the PLATFORM_ASSERT. Anyway since the assert and the new if are testing different things they can be in either order. Also the assert is most likely compiled out in most builds. Best to put exactly what Neil did to avoid merge issues later. |
Spell Check plugin crashes whenever I paste text containing: ποΈ or ποΈ. I believe they are ZWJ emojis because searching for them in a search engine produces the following in the location bar: π%EF%B8%8F π%EF%B8%8F.
Pasting the non-ZWJ versions, π and π, does not cause the plugin to crash.
Using Geany/Geany plugins 1.38
Using GTK+ v3.24.38 and GLib v2.78.0
The text was updated successfully, but these errors were encountered: