geany / geany-plugins

The combined Geany Plugins collection
http://plugins.geany.org/
594 stars 267 forks source link

Splitting a snowman in half crashes Geany with Spell Check (inserting a space before Unicode VARIATION SELECTOR-16) #1041

Open hroncok opened 3 years ago

hroncok commented 3 years ago

Hello. I run geany 1.37.1 on Fedora 33 and Geany crashes when I try to put a space between an emoji and Unicode VARIATION SELECTOR-16.

Steps to reproduce:

  1. Enable the Spell Check plugin and enable spell checking. I use en_US in case it matters.
  2. Insert an emoji with VARIATION SELECTOR-16, e.g. ☃️ (select the entire line form the source of this comment or from here).
  3. Move the cursor between the two characters: _
  4. Type "space" (or any other character it seems).
  5. Geany crashes (aborts).
/usr/include/c++/10/bits/stl_vector.h:1063: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; std::vector<_Tp, _Alloc>::const_reference = const unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
hroncok commented 3 years ago

Thanks @elextr, I've only realized this is dependent on spell check when creating the screenshot.

b4n commented 3 years ago

Sounds like #1022 (and #1036). We didn't really figure out what's happening because on #1022 the user didn't see the problem after manually rebuilding the package, which doesn't make much sense -- unless Fedora has a broken patch?

hroncok commented 3 years ago

I see no patch that could possibly affect this. @dmaphy can confirm or deny.

hroncok commented 3 years ago
Reading symbols from geany...
Reading symbols from /usr/lib/debug/usr/bin/geany-1.37.1-1.fc33.x86_64.debug...
(gdb) run
Starting program: /usr/bin/geany 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff5822640 (LWP 2039064)]
[New Thread 0x7ffff4f8b640 (LWP 2039065)]
[New Thread 0x7fffe7fff640 (LWP 2039066)]
[New Thread 0x7fffe77fe640 (LWP 2039067)]
[Detaching after fork from child process 2039068]
[Thread 0x7fffe7fff640 (LWP 2039066) exited]
/usr/include/c++/10/bits/stl_vector.h:1063: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; std::vector<_Tp, _Alloc>::const_reference = const unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.

Thread 1 "geany" received signal SIGABRT, Aborted.
__GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:49
49    return ret;
(gdb) backtrace
#0  __GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007ffff7b248a4 in __GI_abort () at abort.c:79
#2  0x00007ffff7d8bcd8 in std::__replacement_assert (__file=<optimized out>, __line=<optimized out>, __function=<optimized out>, __condition=<optimized out>) at /usr/include/c++/10/x86_64-redhat-linux/bits/c++config.h:2560
#3  0x00007ffff7dfab7b in std::vector<unsigned char, std::allocator<unsigned char> >::operator[] (this=<optimized out>, __n=<optimized out>, this=<optimized out>, __n=<optimized out>)
    at /usr/include/c++/10/bits/stl_vector.h:1061
#4  std::vector<unsigned char, std::allocator<unsigned char> >::operator[] (__n=<optimized out>, this=<optimized out>, this=<optimized out>, __n=<optimized out>) at /usr/include/c++/10/bits/stl_vector.h:1061
#5  Scintilla::RGBAImage::Pixels (this=0x7fffffffb690) at ../scintilla/src/XPM.cxx:254
#6  Scintilla::Indicator::Draw (value=<optimized out>, state=Scintilla::Indicator::State::normal, rcCharacter=<synthetic pointer>..., rcLine=<synthetic pointer>..., rc=<synthetic pointer>..., surface=0x555556782750, 
    this=0x55555675d710) at ../scintilla/src/Indicator.cxx:81
#7  DrawIndicator (indicNum=<optimized out>, startPos=<optimized out>, endPos=<optimized out>, surface=0x555556782750, vsDraw=..., ll=<optimized out>, xStart=44, rcLine=..., secondCharacter=9, subLine=0, 
    state=Scintilla::Indicator::State::normal, value=1) at ../scintilla/src/EditView.cxx:1042
#8  0x00007ffff7dfaed7 in DrawIndicators (surface=0x555556782750, model=..., vsDraw=..., ll=0x555556928720, line=<optimized out>, xStart=44, rcLine=..., subLine=0, lineEnd=18, under=false, hoverIndicatorPos=-1)
    at ../scintilla/src/EditView.cxx:1066
#9  0x00007ffff7e01294 in Scintilla::EditView::DrawLine (this=0x555556276dd8, surface=0x555556782750, model=..., vsDraw=..., ll=<optimized out>, line=0, lineVisible=0, xStart=44, rcLine=..., subLine=0, phase=510)
    at ../scintilla/src/EditView.cxx:2025
#10 0x00007ffff7d9eba6 in Scintilla::EditView::PaintText (vsDraw=..., rcClient=..., rcArea=..., model=..., surfaceWindow=0x555556815a80, this=0x555556276dd8) at ../scintilla/src/EditView.cxx:2188
#11 Scintilla::Editor::Paint (rcArea=..., surfaceWindow=<optimized out>, this=0x555556276800) at ../scintilla/src/Editor.cxx:1794
#12 Scintilla::ScintillaGTK::DrawTextThis (this=0x555556276800, cr=<optimized out>) at ../scintilla/gtk/ScintillaGTK.cxx:2587
#13 0x00007ffff77410e8 in _gtk_marshal_BOOLEAN__BOXEDv (closure=0x55555676c200, return_value=0x7fffffffbf10, instance=<optimized out>, args=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, 
    param_types=0x5555555f1010) at gtkmarshalers.c:129
#14 0x00007ffff76e797d in gtk_widget_draw_marshallerv (closure=0x55555676c200, return_value=0x7fffffffbf10, instance=0x5555557439f0, args=0x7fffffffbfc0, marshal_data=0x0, n_params=1, param_types=0x5555555f1010)
    at gtkwidget.c:975
#15 0x00007ffff6dbb080 in _g_closure_invoke_va (param_types=0x5555555f1010, n_params=<optimized out>, args=0x7fffffffbfc0, instance=0x5555557439f0, return_value=0x7fffffffbf10, closure=0x55555676c200)
    at ../gobject/gclosure.c:873
#16 g_signal_emit_valist (instance=0x5555557439f0, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffffffbfc0) at ../gobject/gsignal.c:3403
#17 0x00007ffff6dbb1a3 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3550
#18 0x00007ffff76f687a in gtk_widget_draw_internal (widget=0x5555557439f0, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7073
#19 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x5555557439f0, cr=0x5555568e2150) at gtkcontainer.c:3853
#20 0x00007ffff7d9bb1c in Scintilla::ScintillaGTK::DrawThis (cr=0x5555568e2150, this=0x555556276800) at ../scintilla/gtk/ScintillaGTK.cxx:2642
#21 Scintilla::ScintillaGTK::DrawMain (widget=widget@entry=0x55555675c1b0, cr=0x5555568e2150) at ../scintilla/gtk/ScintillaGTK.cxx:2654
#22 0x00007ffff773e59b in _gtk_marshal_BOOLEAN__BOXED (closure=0x5555555f0fc0, return_value=0x7fffffffc370, n_param_values=<optimized out>, param_values=0x7fffffffc3d0, invocation_hint=<optimized out>, 
    marshal_data=<optimized out>) at gtkmarshalers.c:83
#23 0x00007ffff76e78cf in gtk_widget_draw_marshaller (closure=0x5555555f0fc0, return_value=0x7fffffffc370, n_param_values=2, param_values=0x7fffffffc3d0, invocation_hint=0x7fffffffc350, 
    marshal_data=0x7ffff7d9b960 <Scintilla::ScintillaGTK::DrawMain(_GtkWidget*, _cairo*)>) at gtkwidget.c:947
#24 0x00007ffff6d9de2a in g_closure_invoke (closure=0x5555555f0fc0, return_value=0x7fffffffc370, n_param_values=2, param_values=0x7fffffffc3d0, invocation_hint=0x7fffffffc350) at ../gobject/gclosure.c:810
#25 0x00007ffff6dc6cce in signal_emit_unlocked_R.isra.0 (node=<optimized out>, detail=detail@entry=0, instance=instance@entry=0x55555675c1b0, emission_return=emission_return@entry=0x7fffffffc4f0, 
    instance_and_params=instance_and_params@entry=0x7fffffffc3d0) at ../gobject/gsignal.c:3776
#26 0x00007ffff6dbaaee in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffc5a0) at ../gobject/gsignal.c:3504
#27 0x00007ffff6dbb1a3 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3550
#28 0x00007ffff76f687a in gtk_widget_draw_internal (widget=0x55555675c1b0, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7073
#29 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x55555675c1b0, cr=0x5555568e2150) at gtkcontainer.c:3853
#30 0x00007ffff74d6f5d in gtk_container_draw (widget=0x555555c149f0, cr=0x5555568e2150) at gtkcontainer.c:3673
#31 0x00007ffff748210d in gtk_box_draw_contents (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>, unused=0x0) at gtkbox.c:453
#32 0x00007ffff74ce6fd in gtk_css_custom_gadget_draw (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkcsscustomgadget.c:159
#33 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x55555676d1a0, cr=0x5555568e2150) at gtkcssgadget.c:885
#34 0x00007ffff7482191 in gtk_box_draw (widget=<optimized out>, cr=<optimized out>) at gtkbox.c:462
#35 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555c149f0, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#36 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x555555c149f0, cr=0x5555568e2150) at gtkcontainer.c:3853
#37 0x00007ffff75ceb3e in gtk_notebook_draw_stack (gadget=<optimized out>, cr=0x5555568e2150, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>, unused=0x0) at gtknotebook.c:2544
#38 0x00007ffff74ce6fd in gtk_css_custom_gadget_draw (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkcsscustomgadget.c:159
#39 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x555555e91b90, cr=0x5555568e2150) at gtkcssgadget.c:885
#40 0x00007ffff7489cf8 in gtk_box_gadget_draw (gadget=<optimized out>, cr=0x5555568e2150, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkboxgadget.c:512
#41 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x555555e88550, cr=0x5555568e2150) at gtkcssgadget.c:885
#42 0x00007ffff75ca75c in gtk_notebook_draw (widget=<optimized out>, cr=0x5555568e2150) at gtknotebook.c:2559
--Type <RET> for more, q to quit, c to continue without paging--c
#43 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555695c90, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#44 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x555555695c90, cr=0x5555568e2150) at gtkcontainer.c:3853
#45 0x00007ffff75e1fac in gtk_paned_render (gadget=<optimized out>, cr=0x5555568e2150, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>, data=0x0) at gtkpaned.c:1832
#46 0x00007ffff74ce6fd in gtk_css_custom_gadget_draw (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkcsscustomgadget.c:159
#47 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x555555e71c60, cr=0x5555568e2150) at gtkcssgadget.c:885
#48 0x00007ffff75dd551 in gtk_paned_draw (widget=<optimized out>, cr=<optimized out>) at gtkpaned.c:1782
#49 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555e50750, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#50 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x555555e50750, cr=0x5555568e2150) at gtkcontainer.c:3853
#51 0x00007ffff75e1f14 in gtk_paned_render (gadget=<optimized out>, cr=0x5555568e2150, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>, data=0x0) at gtkpaned.c:1818
#52 0x00007ffff74ce6fd in gtk_css_custom_gadget_draw (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkcsscustomgadget.c:159
#53 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x555555e71a60, cr=0x5555568e2150) at gtkcssgadget.c:885
#54 0x00007ffff75dd551 in gtk_paned_draw (widget=<optimized out>, cr=<optimized out>) at gtkpaned.c:1782
#55 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555e50570, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#56 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x555555e50570, cr=0x5555568e2150) at gtkcontainer.c:3853
#57 0x00007ffff74d6f5d in gtk_container_draw (widget=0x555555d54c30, cr=0x5555568e2150) at gtkcontainer.c:3673
#58 0x00007ffff748210d in gtk_box_draw_contents (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>, unused=0x0) at gtkbox.c:453
#59 0x00007ffff74ce6fd in gtk_css_custom_gadget_draw (gadget=<optimized out>, cr=<optimized out>, x=<optimized out>, y=<optimized out>, width=<optimized out>, height=<optimized out>) at gtkcsscustomgadget.c:159
#60 0x00007ffff74e213b in gtk_css_gadget_draw (gadget=0x555555d7e320, cr=0x5555568e2150) at gtkcssgadget.c:885
#61 0x00007ffff7482191 in gtk_box_draw (widget=<optimized out>, cr=<optimized out>) at gtkbox.c:462
#62 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555d54c30, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#63 0x00007ffff74d6d65 in gtk_container_propagate_draw (container=<optimized out>, child=0x555555d54c30, cr=0x5555568e2150) at gtkcontainer.c:3853
#64 0x00007ffff74d6f5d in gtk_container_draw (widget=0x555555cd2500, cr=0x5555568e2150) at gtkcontainer.c:3673
#65 0x00007ffff7715236 in gtk_window_draw (widget=0x555555cd2500, cr=0x5555568e2150) at gtkwindow.c:10486
#66 0x00007ffff76f6664 in gtk_widget_draw_internal (widget=0x555555cd2500, cr=0x5555568e2150, clip_to_size=<optimized out>) at gtkwidget.c:7080
#67 0x00007ffff7703a00 in gtk_widget_render (widget=0x555555cd2500, window=0x5555567b6240, region=<optimized out>) at gtkwidget.c:17606
#68 0x00007ffff75a8899 in gtk_main_do_event (event=0x7fffffffd530) at gtkmain.c:1843
#69 gtk_main_do_event (event=<optimized out>) at gtkmain.c:1690
#70 0x00007ffff7297043 in _gdk_event_emit (event=0x7fffffffd530) at gdkevents.c:73
#71 _gdk_event_emit (event=0x7fffffffd530) at gdkevents.c:67
#72 0x00007ffff72a9b61 in _gdk_window_process_updates_recurse_helper (window=0x5555567b6240, expose_region=<optimized out>) at gdkwindow.c:3874
#73 0x00007ffff72ae701 in gdk_window_process_updates_internal (window=0x5555567b6240) at gdkwindow.c:4020
#74 0x00007ffff72ae8f8 in gdk_window_process_updates_with_mode (window=<optimized out>, recurse_mode=<optimized out>) at gdkwindow.c:4215
#75 0x00007ffff6dbb080 in _g_closure_invoke_va (param_types=0x0, n_params=<optimized out>, args=0x7fffffffd7e0, instance=0x555555cc3ac0, return_value=0x0, closure=0x5555568c8dc0) at ../gobject/gclosure.c:873
#76 g_signal_emit_valist (instance=0x555555cc3ac0, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffffffd7e0) at ../gobject/gsignal.c:3403
#77 0x00007ffff6dbb1a3 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3550
#78 0x00007ffff72a439f in _gdk_frame_clock_emit_paint (frame_clock=0x555555cc3ac0) at gdkframeclock.c:657
#79 gdk_frame_clock_paint_idle (data=0x555555cc3ac0) at gdkframeclockidle.c:597
#80 0x00007ffff72910c9 in gdk_threads_dispatch (data=data@entry=0x5555559a1c40) at gdk.c:769
#81 0x00007ffff6ca1ec1 in g_timeout_dispatch (source=source@entry=0x55555691b2d0, callback=0x7ffff72910a0 <gdk_threads_dispatch>, user_data=0x5555559a1c40) at ../glib/gmain.c:4877
#82 0x00007ffff6ca17ef in g_main_dispatch (context=0x5555555dce30) at ../glib/gmain.c:3325
#83 g_main_context_dispatch (context=0x5555555dce30) at ../glib/gmain.c:4043
#84 0x00007ffff6cf35d8 in g_main_context_iterate.constprop.0 (context=0x5555555dce30, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4119
#85 0x00007ffff6ca0eb3 in g_main_loop_run (loop=0x555555f723e0) at ../glib/gmain.c:4317
#86 0x00007ffff75a420d in gtk_main () at gtkmain.c:1328
#87 0x00007ffff7d59f9d in main_lib (argc=<optimized out>, argv=<optimized out>) at libmain.c:1302
#88 0x00007ffff7b261e2 in __libc_start_main (main=0x555555555070 <main>, argc=1, argv=0x7fffffffdc68, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdc58) at ../csu/libc-start.c:314
#89 0x00005555555550ae in _start ()
dmaphy commented 3 years ago

I see no patch that could possibly affect this. @dmaphy can confirm or deny.

Confirmed. Or rather let me say: Currently the Geany package in Fedora does not apply any patches at all.

eht16 commented 3 years ago

Don't split snowmen!!! How dare you?

More seriously, the difference to #1022 and #1036 is that @hroncok seems to be using 1.37 and not 1.36. The backtrace shows the more or less obvious: the crash seems to happen while drawing the indicator in Scintilla. It might be there is a bug in the start to end calculation for the indicator.

Unfortunately, I cannot reproduce it :(.

elextr commented 3 years ago

From the various issues on spellcheck it seems that Scintilla has a problem drawing indicators on damaged or zero width characters.

The fact that @eht16 can't reproduce suggests its also font and/or Pango version dependent. @hroncok as I keep saying, post the version of GTK and Glib and OS and version (see top of help->debug messages) and also the font and size you use, and @eht16 too :) @hroncok can you try different fonts and sizes?

Maybe it needs to be pushed upstream, but since this is the no longer supported Scintilla 3 the answer is likely to be try Scintilla 4?

hroncok commented 3 years ago

post the version of GTK and Glib and OS and version (see top of help->debug messages)

Using GTK+ v3.24.23 and GLib v2.66.3 runtime libraries

02:58:18: Geany INFO        : Geany 1.37.1, cs_CZ.utf8
02:58:18: Geany INFO        : GTK 3.24.23, GLib 2.66.3
02:58:18: Geany INFO        : OS: Fedora 33 (Thirty Three) ()
02:58:18: Geany INFO        : System data dir: /usr/share/geany
02:58:18: Geany INFO        : User config dir: /home/churchyard/.config/geany
02:58:18: GLib-GIO DEBUG    : _g_io_module_get_default: Found default implementation gvfs (GDaemonVfs) for ‘gio-vfs’
02:58:19: Geany INFO        : Loaded GTK+ CSS theme '/usr/share/geany/geany.css'
02:58:19: Geany INFO        : Loaded GTK+ CSS theme '/usr/share/geany/geany-3.20.css'
02:58:19: Geany INFO        : System plugin path: /usr/lib64/geany
02:58:19: Geany INFO        : Added filetype Cython (63).
02:58:19: Geany INFO        : Added filetype Scala (64).
02:58:19: Geany INFO        : Added filetype Genie (65).
02:58:19: Geany INFO        : Added filetype Nim (66).
02:58:19: Geany INFO        : Added filetype Arduino (67).
02:58:19: Geany INFO        : Added filetype TypeScript (68).
02:58:19: Geany INFO        : Added filetype Graphviz (69).
02:58:19: Geany INFO        : Added filetype Groovy (70).
02:58:19: Geany INFO        : Added filetype Clojure (71).
02:58:19: Geany INFO        : Added filetype JSON (72).
02:58:19: Geany INFO        : Added filetype Swift (73).
02:58:19: Geany INFO        : Added filetype CUDA (74).
02:58:19: Geany INFO        : Added filetype Kotlin (75).
02:58:19: Geany INFO        : Added filetype SPEC (76).
02:58:19: Geany INFO        : Loaded libvte from libvte-2.91.so.0
02:58:19: Geany INFO        : Loaded:   /usr/lib64/geany/splitwindow.so (Rozdělení okna)
02:58:19: SpellCheck DEBUG  : Initializing Enchant library version 2.2.13
02:58:19: GLib DEBUG    : unsetenv() is not thread-safe and should not be used after threads are created
02:58:19: Geany INFO        : Loaded:   /usr/lib64/geany/spellcheck.so (Spell Check)
02:58:19: Geany INFO        : Loaded:   /usr/lib64/geany/classbuilder.so (Tvořič tříd)
02:58:19: Geany INFO        : unknown : None (UTF-8)

and also the font and size you use

Monospace Regular. Size doesn't matter (said the actress).

hroncok commented 3 years ago

Changing font to a random different font doesn't make a difference.

codebrainz commented 3 years ago

When I split a snowman I get this:

snowman

No crashes.

18:20:05: Geany INFO        : Geany 1.38 (git >= d2740f21f), en_CA.UTF-8
18:20:05: Geany INFO        : GTK 3.24.20, GLib 2.64.3
18:20:05: Geany INFO        : OS: Ubuntu 20.04.1 LTS (focal)

Both Geany and Geany-Plugins freshly built and installed from master branch.

elextr commented 3 years ago

Probably Scintilla (or Pango) has problems with ignoring variation sequences if the preceeding character isn't variable in some versions of Pango.

eht16 commented 3 years ago

I'm using DejaVu Sans Mono Book 8 with

23:24:25: Geany INFO        : Geany 1.38 (git >= 00ca7e7a), en_US.utf8
23:24:25: Geany INFO        : GTK 3.24.23, GLib 2.66.2
23:24:25: Geany INFO        : OS: Arch Linux

Pango: 1.48.0-1

elextr commented 3 years ago

Goodness @eht16 your Arch is 0.0.1 behind on Glib, I thought Arch was totally bleeding edge :grin:

Anyway thats unlikely to be the difference.

Its still possibly the font, @hroncok you said "monospace", but thats usually an alias for something else, can you find what or specify the other one you tried? @eht16 and @codebrainz which font are you using so we can just check if everybody has the same.

eht16 commented 3 years ago

Goodness @eht16 your Arch is 0.0.1 behind on Glib, I thought Arch was totally bleeding edge grin

Arch is totally bleeding egde as you'D expect it (https://www.archlinux.org/packages/core/x86_64/glib2/), just my local system isn't so much because of my personal lazyness :).

Its still possibly the font, @hroncok you said "monospace", but thats usually an alias for something else, can you find what or specify the other one you tried? @eht16 and @codebrainz which font are you using so we can just check if everybody has the same.

My font is: DejaVu Sans Mono Book 8

elextr commented 3 years ago

Note that I agree with @eht16 that its likely an error in the length calculation of the indicator, possibly due to rounding of Font data to integer (thats happened elsewhere IIRC, and a "fix" added) but quite why that manifests itself as accessing a vector at an illegal location I'm not sure. But without being able to reproduce it its hard to report upstream. Thats why I keep prompting people to provide more and more info on their situation too find whats different.

codebrainz commented 3 years ago

In my test I was using same font as @eht16 except size 10, but I believe emojies fallback to some of the Noto* fonts, one for the black and white and a different one for the colour emoji.

elextr commented 3 years ago

Got it!!! When creating the XPM image of the squiggle to underline the spelling error Scintilla allocates a vector to hold it whose size is width*height*4 and that doesn't allocate if the width or height is zero, so any index operation will likely crash. Here used here.

Unfortunately the backtrace @hroncok posted has too many values optimised out/not printed to know where it gets that zero from, and why it only applies if the variant is applied to a character without variants and only on @hroncok's system

hroncok commented 3 years ago
$ fc-match Monospace
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Regular"

The snowman seems like Noto, but I have no idea how to find out.

elextr commented 3 years ago

@hroncok I don't think the snowman matters, @eht16 will be pleased to know that you are not actually splitting the snowman :grin:. The snowman code point is followed by a variant selector code point that says make the preceding character a different style, in the case of the snowman it makes it coloured. Your "split" puts another code point between the snowman and the variant selector so it applies to this code point and no longer applies to the snowman, and so the olde black and white glyph is used. This is visible in the examples @codebrainz posted.

But the problem appears to be applying the variant to a code point that has no variants, and that code point is in normal font, ie Dejavu Sans Mono.

However since your fonts style is "regular" not "book" it is different to that used by @eht16 and @codebrainz so that may be the issue. Not sure where to go unless someone can install that font and reproduce the problem.

hroncok commented 3 years ago

The snowman code point is followed by a variant selector code point that says make the preceding character a different style, in the case of the snowman it makes it coloured. Your "split" puts another code point between the snowman and the variant selector so it applies to this code point and no longer applies to the snowman, and so the olde black and white glyph is used.

That's how I understand it, yes.

eht16 commented 3 years ago

I don't have the "DejaVu Sans Mono Regular" font variant :(. Also, my snowman is always in the uncolored variant, even when not "split":

geany_snowman

Not sure why this is. Still, no matter what I try, Geany doesn't crash :cry: :smile:

hroncok commented 3 years ago

I kinda think that having the colored version is a must for the crash to happen :/

codebrainz commented 3 years ago

I don't have the "DejaVu Sans Mono Regular" font variant

Me either

Also, my snowman is always in the uncolored variant

To copy into Geany, I had to "Edit" this issue description and copy/paste from there (and then cancel the edit). I think you also need a fallback font that provides it to be installed like Noto Color Emoji.

elextr commented 3 years ago

I kinda think that having the colored version is a must for the crash to happen :/

Perhaps, but not the sole requirement, @codebrainz has the coloured snowman, and no crash

codebrainz commented 3 years ago

This is what I get here:

$ fc-match -s monospace | grep -i emoji
NotoColorEmoji.ttf: "Noto Color Emoji" "Regular"
hroncok commented 3 years ago
$ fc-match -s monospace | grep -i emoji
OpenSansEmoji.ttf: "OpenSansEmoji" "Regular"
NotoColorEmoji.ttf: "Noto Color Emoji" "Regular"
eht16 commented 3 years ago

Also, my snowman is always in the uncolored variant

To copy into Geany, I had to "Edit" this issue description and copy/paste from there (and then cancel the edit). I think you also need a fallback font that provides it to be installed like Noto Color Emoji.

Ha, thanks. After installing the Noto fonts it's also colored for me. It's even more fun now since you can set the crsor in the middle of the snowman (I'm still new to this fancy Unicode Emoji stuff :D).

geany_snowman_colored

$ fc-match -s monospace | grep -i emoji
NotoColorEmoji.ttf: "Noto Color Emoji" "Regular"

But Geany still doesn't crash.

So, the colored variant seems not relevant for the crash. The remaining least common denominator seems to be Fedora (also used in #1022 and #1036). Even if, as @b4n said, one user solved it by self-compiling.

At least the second theory could be easily verified / disproved. @hroncok could you build Geany manually (ala ./configure && make && make install)?

hroncok commented 3 years ago

Can I run it without make install? Do I need to do this with plugins and geany somehow combined?

codebrainz commented 3 years ago

So, the colored variant seems not relevant for the crash.

Agree, it seems unlikely that it's related to the font, but @hroncok does have a different emoji font than Noto.

Can I run it without make install? Do I need to do this with plugins and geany somehow combined?

You could configure it with a different prefix, like:

$ ./configure --prefix=/tmp/bleh
$ make
$ make install
$ /tmp/bleh/bin/geany -v

Once running you can set an Extra plugin path in Preferences->General to point to your existing plugins directory (ex. /usr/lib/geany or wherever your distro package puts them).

b4n commented 3 years ago

FWIW I just tried to compile Geany master with Clang as well as with what seem to be Fedora's hardening flags, yet I still can't reproduce. I also tried to run in Valgrind's memcheck in case it was a memory access error that was simply not caught by the system because of different build or runtime settings, but that didn't show anything either.

I also installed OpenSansEmoji, which didn't change behavior either.

elextr commented 3 years ago

@hroncok what happens if there is no snowman in sight, but you have a space character followed by the code point FE0F (which you can enter as <ctrl><shift>u all at once and you get a funny u then fe0f and <enter>).

hroncok commented 3 years ago

FWIW I just tried to compile Geany master with Clang as well as with what seem to be Fedora's hardening flags, yet I still can't reproduce.

FTR the cflags in Fedora are:

-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection

And ldflags:

-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld

And we don't use cgal but gcc.

@hroncok what happens if there is no snowman in sight, but you have a space character followed by the code point FE0F (which you can enter as <ctrl><shift>u all at once and you get a funny u then fe0f and <enter>).

A crash.

elextr commented 3 years ago

@hroncok what happens if there is no snowman in sight, but you have a space character followed by the code point FE0F (which you can enter as u all at once and you get a funny u then fe0f and ).

A crash.

Ok, so the snowman has melted out of the problem, it seems to be (at least) space followed by variant-16 in @hroncok's fonts.

b4n commented 3 years ago

FTR the cflags in Fedora are: […] And ldflags:

Ah-ah! We got a winner. If I build Geany with those flags (minus the -spec parameters that obviously don't work on my Debian):

$ export LDFLAGS="-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now"
$ export CFLAGS="-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong  -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection"
$ export CXXFLAGS="$CFLAGS"
$ ./autogen.sh --prefix=/tmp/_install
[…]
$ make -j4 install
[…]
$ /tmp/_install/bin/geany -c /tmp/_conf
/usr/include/c++/8/bits/stl_vector.h:950: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; std::vector<_Tp, _Alloc>::const_reference = const unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.

Without testing further, my money would go on -D_GLIBCXX_ASSERTIONS.

elextr commented 3 years ago

@b4n neat, so now why is the access to the vector out of range?

b4n commented 3 years ago

@elextr that's the next step, but I bet you can now debug it yourself as well :wink: :wink: :wink: I'll try and look at it maybe this evening if I find time.

elextr commented 3 years ago

@b4n hint

hroncok commented 3 years ago

Without testing further, my money would go on -D_GLIBCXX_ASSERTIONS.

I recall seeing this in other places as well, so I concur this is the flag.

elextr commented 3 years ago

@hroncok what that flag does is to make illegal indexing of vectors crash rather than be ignored, so Fedora using it is why your Geany crashes but others do not, but its not why the index was out of range in the first place.

b4n commented 3 years ago

@elextr for the moment your hint doesn't make much sense to me, you're just point out that a 0×H image will have 0 bytes of data, which makes sense. I don't see how it would account for accessing past the buffer, as the given size is 0 in one direction. Well, of course the can be a bug some other place, and likely is, but it makes perfect sense the data is 0 bytes.

Or maybe I'm just not C++-literate enough, but &zeroLengthMemory[0] should not cause any issue. Though, maybe the problem is that operator[0] actually checks the value can be dereferenced, in which case it cannot…

Meh, writing this makes me think that either:

I don't think there is an actual bug in accessing the memory as Valgrind's memcheck doesn't report errors; so I guess the calling code (at which I didn't look) does not access the memeory past its size. I guess we could special-case the code not to dereference, but that sounds weird to the C guy I am :)

hroncok commented 3 years ago

IIRC the other issue when -D_GLIBCXX_ASSERTIONS caused a crash, it was caused by accessing zero_length_vector[0]. Let me try to find it.

hroncok commented 3 years ago

https://github.com/openscad/openscad/issues/2965#issuecomment-500542956

elextr commented 3 years ago

@b4n, by default the C++ vector index is unchecked, but no index is legal on a zero length vector, its UB, as the spec says about operator[] Accessing a nonexistent element through this operator is undefined behavior..

The assertions flag makes it checked, as the message you posted above says Assertion '__builtin_expect(__n < this->size(), true)' failed, and if size() is 0 its impossible for __n to be < 0.

The reason its UB is that the vector would have to have at least one member to have memory allocated be able to access vector[0], but if size() is zero it need not have any memory allocated, remember std::vector is dynamic.

The reason nothing fails without the assertion is that Scintilla uses resize() as one of my links above points out, which does not de-allocate the vector memory, and its extremely likely that the vector has some memory left from previous operations, so its internal pointer is not nullptr.

@hroncok yeah, I'm sure lots of other programs have bugs found when the flag makes vector indexing checked :grin:

b4n commented 3 years ago

@elextr sure, but my point is that in C &pointer[0] does not dereference anything, pointercould be anything so long as you don't access it. So I guess if it's not the same with C++ vectors, Scintilla should use something else there.

elextr commented 3 years ago

@b4n, yes, but std::vector is a library type, not a builtin, which keeps track of its size and its capacity (which is why it is possible to insert the assert, its has size to compare to, but in C since nothing saves the size of array you malloced no automatic assert can be inserted in an index operation). Operator[] on a std::vector is not a builtin, its an overloaded operator member function that returns a reference to the indexed member. So your &vector[0] has to call vector.operator[](0) before taking the address of the referenced object when it returns. But of course you can't reference a non-existent object, which is why its UB to do so.

The std::vector has an alternative member function vector.at() that is always checked and which throws an exception if the index is out of range. But of course thats slower, whereas operator[] is as fast as C, so all premature optimisers (basically all programmers :) use it.

As I keep saying C++ is not C :grin:

b4n commented 3 years ago

As I keep saying C++ is not C :grin:

I know, and I battle for avoiding this confusion. I just see once more that C++ tries to be confusing for C programmers, reproducing syntax but with a whole other lot of constraint that only apply in corner cases :grin:

Anyhow, a bad patch could be as simple as:

diff --git a/scintilla/src/Indicator.cxx b/scintilla/src/Indicator.cxx
index f72102772..5c076290b 100644
--- a/scintilla/src/Indicator.cxx
+++ b/scintilla/src/Indicator.cxx
@@ -64,6 +64,8 @@ void Indicator::Draw(Surface *surface, const PRectangle &rc, const PRectangle &r
            const PRectangle rcSquiggle = PixelGridAlign(rc);

            const int width = std::min(4000, static_cast<int>(rcSquiggle.Width()));
+           if (width < 1)
+               break;
            RGBAImage image(width, 3, 1.0, nullptr);
            enum { alphaFull = 0xff, alphaSide = 0x2f, alphaSide2=0x5f };
            for (int x = 0; x < width; x++) {

It solves the very problem here, but doesn't fix it more generally than just for the squiggles.

Unfortunately just fixing the pixels retrieval is not enough, there's at least one other instance of an empty vector access down the same code path.

[edit] A probably better patch could be:

diff --git a/scintilla/gtk/PlatGTK.cxx b/scintilla/gtk/PlatGTK.cxx
index b999feeee..255a53f3b 100644
--- a/scintilla/gtk/PlatGTK.cxx
+++ b/scintilla/gtk/PlatGTK.cxx
@@ -575,6 +575,8 @@ void SurfaceImpl::GradientRectangle(PRectangle rc, const std::vector<ColourStop>

 void SurfaceImpl::DrawRGBAImage(PRectangle rc, int width, int height, const unsigned char *pixelsImage) {
    PLATFORM_ASSERT(context);
+   if (width < 1 || height < 1)
+       return;
    if (rc.Width() > width)
        rc.left += (rc.Width() - width) / 2;
    rc.right = rc.left + width;
diff --git a/scintilla/src/XPM.cxx b/scintilla/src/XPM.cxx
index 2e2fba986..1d18aefa0 100644
--- a/scintilla/src/XPM.cxx
+++ b/scintilla/src/XPM.cxx
@@ -251,7 +251,7 @@ int RGBAImage::CountBytes() const noexcept {
 }

 const unsigned char *RGBAImage::Pixels() const noexcept {
-   return &pixelBytes[0];
+   return pixelBytes.empty() ? nullptr : &pixelBytes[0];
 }

 void RGBAImage::SetPixel(int x, int y, ColourDesired colour, int alpha) noexcept {

I didn't check all instances of &vector[0] but at least it properly handles those -- or should.

codebrainz commented 3 years ago

But of course thats slower, whereas operator[] is as fast as C

Not true when distros enable debug assertions in release builds...

reproducing syntax but with a whole other lot of constraint that only apply in corner cases

Kind of like making the bitwise shift operators sometimes mean "read/write IO stream" :smile:

elextr commented 3 years ago

@b4n now that you are returning nullptr from pixels() but you did not put a check for that on the return value of every use of pixels() all you have done is shift the problem. :stuck_out_tongue_winking_eye:

Now we can explain it, I suspect that it would be best to pass it to Neil to decide the best solution for fixing drawing indicators on zero sized code points. He can best determine a solution that fits with his aspirations of noexcept everywhere and other performance issues.

@codebrainz yeah, well, everybody just says "Gee Fedora is slow" :grin:

@b4n @codebrainz just think of C++ as Sea++ a language which no longer has a connection to C, and that does its own thing, like Java or D, thats effectively what it has become :smile:

b4n commented 3 years ago

Kind of like making the bitwise shift operators sometimes mean "read/write IO stream" :smile:

Well, at least that one basically doesn't make any sense in C with the types involved. It's not mimicking the syntax for 99.8% identical behavior and .2% surprise :grin:

@b4n now that you are returning nullptr from pixels() but you did not put a check for that on the return value of every use of pixels() all you have done is shift the problem. :stuck_out_tongue_winking_eye:

Not really: at least to the C guy I am, reading 0 bytes from nullptr or 0 bytes from a valid pointer doesn't seem much different.

Now we can explain it, I suspect that it would be best to pass it to Neil to decide the best solution for fixing drawing indicators on zero sized code points. He can best determine a solution that fits with his aspirations of noexcept everywhere and other performance issues.

Agreed. I wanted to understand the issue and then provide a patch that Fedora could use to stop these repeated crashes. Now it'll be time to hand it out to Neil and let it live :)

@b4n @codebrainz just think of C++ as Sea++ a language which no longer has a connection to C, and that does its own thing, like Java or D, thats effectively what it has become smile

And they kept the C syntax weirdies like &array[index], very clever on their part :stuck_out_tongue_closed_eyes:

OK, enough trolling about C++ on my part for the moment :slightly_smiling_face:

elextr commented 3 years ago

@b4n now that you are returning nullptr from pixels() but you did not put a check for that on the return value of every use of pixels() all you have done is shift the problem. :stuck_out_tongue_winking_eye:

Not really: at least to the C guy I am, reading 0 bytes from nullptr or 0 bytes from a valid pointer doesn't seem much different.

What I meant is that you are assuming that wherever the return pointer from Pixels() is used its not de-referenced for any reason, otherwise its a segfault or a UB. I haven't traced all uses of the pointer Pixels() function returns to see, I just noticed your patch had no checks of the returned pointer.

Are you gonna post to Scintilla?