Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Occasional crashes when modifying effects in AC mode #1021

Closed
nkaminski opened this issue Dec 9, 2017 · 9 comments
Closed

Occasional crashes when modifying effects in AC mode #1021

nkaminski opened this issue Dec 9, 2017 · 9 comments
Labels
Bug - Minor

Comments

@nkaminski
Copy link
Contributor

nkaminski commented Dec 9, 2017 •
edited

Hello,
With xLights 2017.39 and 2017.40, I am experiencing occasional crashes when modifying effects in AC mode. All effects in question are single color on/off/ramp effects applied to line models.

I can reproduce this consistently via the following steps:
Enter AC mode, replace (individually) several ramp effects with intensity effects in rapid succession and this crash will occur.

I am running xLights on Arch Linux, fully updated as of 12/8/17.

Looking at the debug reports. is appears that the crash is consistently occurring within xLightsFrame::SetEffectControls.

Debug Reports:
xLights_dbgrpt-31273-20171208T004825.zip
xLights_dbgrpt-1152-20171208T235047.zip
xLights_dbgrpt-24885-20171209T001609.zip
@nkaminski nkaminski changed the title Occasional crashes when deleting or modifying effects in AC mode Occasional crashes when modifying effects in AC mode Dec 9, 2017
@keithsw1111
Copy link
Collaborator

Yeah. I think I will have this fixed in .41. I just can't be sure.

@nkaminski
Copy link
Contributor Author

@keithsw1111 Any insights as to where/how it happens? It seems like a use-after-free case, possibly where member functions of an effect are called on an effect that has been deleted. I have a fairly strong C++ background and have been digging into this myself as well.

@nkaminski
Copy link
Contributor Author

Log from GCC7 + libasan memory safety analyser:
==24016==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000075b60 at pc 0x5591a185d581 bp 0x7fff5e281010 sp 0x7fff5e281000
READ of size 8 at 0x613000075b60 thread T0
#0 0x5591a185d580 in Effect::GetEffectNameabi:cxx11 const sequencer/Effect.cpp:278
#1 0x5591a1ca37c1 in xLightsFrame::SelectedEffectChanged(SelectedEffectChangedEvent&) sequencer/tabSequencer.cpp:802
#2 0x5591a29e322a in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x298722a)
#3 0x5591a29e37ae in wxEvtHandler::SearchDynamicEventTable(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x29877ae)
#4 0x5591a29e3a6e in wxEvtHandler::TryHereOnly(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x2987a6e)
#5 0x5591a29e34d2 in wxEvtHandler::DoTryChain(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x29874d2)
#6 0x5591a29e3b4c in wxEvtHandler::ProcessEvent(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x2987b4c)
#7 0x5591a2b6b00a in wxWindowBase::TryAfter(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x2b0f00a)
#8 0x5591a2b6b00a in wxWindowBase::TryAfter(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x2b0f00a)
#9 0x5591a2b6b00a in wxWindowBase::TryAfter(wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x2b0f00a)
#10 0x5591a29e5701 in wxEvtHandler::ProcessPendingEvents() (/home/nashkaminski/xLights/bin/xLights+0x2989701)
#11 0x5591a28bcc06 in wxAppConsoleBase::ProcessPendingEvents() (/home/nashkaminski/xLights/bin/xLights+0x2860c06)
#12 0x5591a2a1452c in wxApp::DoIdle() (/home/nashkaminski/xLights/bin/xLights+0x29b852c)
#13 0x5591a2a1464f in wxapp_idle_callback (/home/nashkaminski/xLights/bin/xLights+0x29b864f)
#14 0x7fc9f3c150bd in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6b0bd)
#15 0x7fc9f3c16f68 (/usr/lib/libglib-2.0.so.0+0x6cf68)
#16 0x7fc9f3c17f41 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6df41)
#17 0x7fc9f5d913a6 in gtk_main (/usr/lib/libgtk-x11-2.0.so.0+0x12e3a6)
#18 0x5591a2a27a94 in wxGUIEventLoop::DoRun() (/home/nashkaminski/xLights/bin/xLights+0x29cba94)
#19 0x5591a28ec4f3 in wxEventLoopBase::Run() (/home/nashkaminski/xLights/bin/xLights+0x28904f3)
#20 0x5591a28ba4bb in wxAppConsoleBase::MainLoop() (/home/nashkaminski/xLights/bin/xLights+0x285e4bb)
#21 0x5591a2931f20 in wxEntry(int&, wchar_t**) (/home/nashkaminski/xLights/bin/xLights+0x28d5f20)
#22 0x5591a1ec2af8 in main /home/nashkaminski/xLights/xLights/xLightsApp.cpp:228
#23 0x7fc9f17faf49 in __libc_start_main (/usr/lib/libc.so.6+0x20f49)
#24 0x5591a1791489 in _start (/home/nashkaminski/xLights/bin/xLights+0x1735489)

0x613000075b60 is located 32 bytes inside of 336-byte region [0x613000075b40,0x613000075c90)
freed by thread T0 here:
#0 0x7fc9f9bd7b51 in operator delete(void*, unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cc:140
#1 0x5591a185bc24 in Effect::~Effect() sequencer/Effect.cpp:172
#2 0x5591a1877b21 in EffectLayer::RemoveEffect(int) sequencer/EffectLayer.cpp:80
#3 0x5591a1896d9d in EffectsGrid::ACDraw(ACTYPE, ACSTYLE, ACMODE, int, int, int, int, int, int, int) sequencer/EffectsGrid.cpp:1111
#4 0x5591a18964bb in EffectsGrid::ACDraw(ACTYPE, ACSTYLE, ACMODE, int, int, int, int, int, int, int) sequencer/EffectsGrid.cpp:1059
#5 0x5591a18ad8af in EffectsGrid::DoACDraw(bool, ACTYPE, ACSTYLE, ACTOOL, ACMODE) sequencer/EffectsGrid.cpp:2482
#6 0x5591a18add3c in EffectsGrid::mouseReleased(wxMouseEvent&) sequencer/EffectsGrid.cpp:2502
#7 0x5591a29e322a in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x298722a)

previously allocated by thread T0 here:
#0 0x7fc9f9bd6489 in operator new(unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cc:80
#1 0x5591a1878b47 in EffectLayer::AddEffect(int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, int, int, int, bool) sequencer/EffectLayer.cpp:134
#2 0x5591a1943cb1 in SequenceElements::LoadEffects(EffectLayer*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, wxXmlNode*, std::vector<std::__cxx11::basic_string<char, std::char_traits, std::allocator >, std::allocator<std::__cxx11::basic_string<char, std::char_traits, std::allocator > > > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits, std::allocator >, std::allocator<std::__cxx11::basic_string<char, std::char_traits, std::allocator > > > const&) sequencer/SequenceElements.cpp:678
#3 0x5591a1949d9f in SequenceElements::LoadSequencerFile(xLightsXmlFile&, wxString const&) sequencer/SequenceElements.cpp:848
#4 0x5591a1c9f1e9 in xLightsFrame::LoadSequencer(xLightsXmlFile&) sequencer/tabSequencer.cpp:580
#5 0x5591a220ced5 in xLightsFrame::SeqLoadXlightsFile(xLightsXmlFile&, bool) /home/nashkaminski/xLights/xLights/SeqFileUtilities.cpp:560
#6 0x5591a2208d4e in xLightsFrame::OpenSequence(wxString, ConvertLogDialog*) /home/nashkaminski/xLights/xLights/SeqFileUtilities.cpp:399
#7 0x5591a1dacbbb in xLightsFrame::OnMenuItem_File_Open_SequenceSelected(wxCommandEvent&) /home/nashkaminski/xLights/xLights/xLightsMain.cpp:2686
#8 0x5591a29e322a in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) (/home/nashkaminski/xLights/bin/xLights+0x298722a)

@nkaminski
Copy link
Contributor Author

Confirmed as a UAF bug; it looks like the order of processing of the mouse up and SelectedEffectChanged does not always occur in order:

RaiseSelectedEffectChanged (call to RaiseSelectedEffectChanged)
RaiseSelectedEffectChanged
Process SelectedEffectChanged (SelectedEffectChanged event handled)
Process SelectedEffectChanged
Mouse up (Mouse up event handled)
RaiseSelectedEffectChanged
RaiseSelectedEffectChanged
Mouse up
Process SelectedEffectChanged
--crash--

@nkaminski nkaminski mentioned this issue Dec 10, 2017
Closed
@nkaminski
Copy link
Contributor Author

Regarding testing with Dan's recent changes (discussed in the PR more extensively), as of release 2017.42, this crash still occurs:

xLights version 2017.42 64bit

xLights(_Z11handleCrashPv+0xc35) [0x55a824154675]
xLights(wxFatalSignalHandler+0x19) [0x55a8246d76c9]
/usr/lib/libpthread.so.0(+0x11da0) [0x7fab770c6da0]
/usr/lib/libc.so.6(+0x96496) [0x7fab72682496]
/usr/lib/libc.so.6(+0xaea32) [0x7fab7269aa32]
xLights(_Z7wxMB2WCPwPKcm+0x6e) [0x55a8246ac3de]
xLights(_ZNK8wxMBConv7ToWCharEPwmPKcm+0xc5) [0x55a82466e585]
xLights(_ZNK8wxMBConv6cMB2WCEPKcmPm+0x40) [0x55a82466eee0]
xLights(_ZN8wxString10ConvertStrEPKcmRK8wxMBConv+0x4d) [0x55a82467d70d]
xLights(_ZN8wxStringC2EPKc+0x42) [0x55a823e97232]
xLights(_ZN12xLightsFrame17SetEffectControlsERK11SettingsMap+0xa3) [0x55a8240a0003]
xLights(_ZN12xLightsFrame17SetEffectControlsERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES7_RK11SettingsMapSA_b+0x153) [0x55a8240a0443]
xLights(_ZN12xLightsFrame21SelectedEffectChangedER26SelectedEffectChangedEvent+0xb9) [0x55a8240a07d9]
xLights(_ZN12wxEvtHandler23ProcessEventIfMatchesIdERK21wxEventTableEntryBasePS_R7wxEvent+0x6b) [0x55a8246fa23b]
xLights(_ZN12wxEvtHandler23SearchDynamicEventTableER7wxEvent+0x1bf) [0x55a8246fa7bf]
xLights(_ZN12wxEvtHandler11TryHereOnlyER7wxEvent+0x1f) [0x55a8246faa7f]
xLights(_ZN12wxEvtHandler10DoTryChainER7wxEvent+0x43) [0x55a8246fa4e3]
xLights(_ZN12wxEvtHandler12ProcessEventER7wxEvent+0x3d) [0x55a8246fab5d]
xLights(_ZN12wxWindowBase8TryAfterER7wxEvent+0xab) [0x55a8248837db]
xLights(_ZN12wxWindowBase8TryAfterER7wxEvent+0xab) [0x55a8248837db]
xLights(_ZN12wxWindowBase8TryAfterER7wxEvent+0xab) [0x55a8248837db]
xLights(_ZN12wxEvtHandler20ProcessPendingEventsEv+0x152) [0x55a8246fc712]
xLights(_ZN16wxAppConsoleBase20ProcessPendingEventsEv+0x67) [0x55a8245d38a7]
xLights(_ZN5wxApp6DoIdleEv+0xad) [0x55a82472b8cd]
xLights(+0x10be9f0) [0x55a82472b9f0]
/usr/lib/libglib-2.0.so.0(g_main_context_dispatch+0x15e) [0x7fab74a270be]
/usr/lib/libglib-2.0.so.0(+0x6cf69) [0x7fab74a28f69]
/usr/lib/libglib-2.0.so.0(g_main_loop_run+0xd2) [0x7fab74a29f42]
/usr/lib/libgtk-x11-2.0.so.0(gtk_main+0xb7) [0x7fab76ba33a7]
xLights(_ZN14wxGUIEventLoop5DoRunEv+0x25) [0x55a82473ef95]
xLights(_ZN15wxEventLoopBase3RunEv+0xa4) [0x55a824603194]
xLights(_ZN16wxAppConsoleBase8MainLoopEv+0x5c) [0x55a8245d115c]
xLights(_Z7wxEntryRiPPw+0x41) [0x55a824648d71]
xLights(main+0xa6) [0x55a823e50d16]
/usr/lib/libc.so.6(__libc_start_main+0xea) [0x7fab7260cf4a]
xLights(_start+0x2a) [0x55a823e87c5a]

Crashed thread id: 0x7AABB9C0 or 2058074560

@keithsw1111
Copy link
Collaborator

To take this forward I need a reliable way to reproduce it.

@nkaminski
Copy link
Contributor Author

nkaminski commented Dec 31, 2017 •
edited

Replacing a several effects with a different effect in rapid succession with AC mode active will trigger this within at most 20 replace actions. The crash happens much sooner if the mouse button is held for a very short time while doing such, since it seems to greatly increase the probability of the mouse up event being handled before the SelectedEffectChanged is handled (which triggers the UAF and crash).

I will setup a VM with the exact same environment (fully updated Arch Linux) that I can reliably reproduce the issue in and make the image available if that helps.

@nkaminski
Copy link
Contributor Author

nkaminski commented Dec 31, 2017 •
edited

I also noticed you mentioned that a variant of #1022 was incorporated. Will retest to see if i can still reproduce on the latest code.

@keithsw1111
Copy link
Collaborator

Please reopen if it is not resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug - Minor
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nkaminski @keithsw1111 @stevegiron