search

sirjuddington / SLADE

It's a Doom editor
https://slade.mancubus.net
GNU General Public License v2.0
686 stars 104 forks source link

3D mode crashes on MacOS and Linux since the 3D floors PR #1648

Closed Pedro-Beirao closed 1 month ago

Pedro-Beirao commented 5 months ago

SLADE Version

3.2.5

OS

macOS

Editor

Map editor

Steps to Reproduce

Happens since #1358 Go into 3D mode, doesnt matter if 3D floors are ON or not

This seems to only happen on MacOS, both on x64 and arm64

Crash Information

slade(27786,0x203ce2240) malloc: Heap corruption detected, free list is damaged at 0x600000d3e8e0
*** Incorrect guard value: 105553167013504
slade(27786,0x203ce2240) malloc: *** set a breakpoint in malloc_error_break to debug

Screenshots

No response

Pedro-Beirao commented 5 months ago

Ive tried to find the problem and fix it, but I haven't been able to LLDB is no help

sirjuddington commented 5 months ago

That crash info doesn't really help much unfortunately, what map are you opening?

Pedro-Beirao commented 5 months ago

Happens in any map

Sometimes instantly after pressing the toggle 3d button, sometimes I can move the camera in 3d for less than a second before it happens

OrdinaryMagician commented 5 months ago

Crash also happens immediately on Linux upon trying to enter 3d view.

Here's a stack trace from a debug build:

#0  0x00007ffff524783c in  () at /usr/lib/libc.so.6
#1  0x00007ffff51f7668 in raise () at /usr/lib/libc.so.6
#2  0x00007ffff51df4b8 in abort () at /usr/lib/libc.so.6
#3  0x00007ffff51e0390 in  () at /usr/lib/libc.so.6
#4  0x00007ffff52517b7 in  () at /usr/lib/libc.so.6
#5  0x00007ffff5253ad4 in  () at /usr/lib/libc.so.6
#6  0x00007ffff5256353 in free () at /usr/lib/libc.so.6
#7  0x0000555555d009cd in slade::MapRenderer3D::checkVisibleFlats() (this=0x55555aa3c498) at /tmp/SLADE/src/MapEditor/Renderer/MapRenderer3D.cpp:3099
#8  0x0000555555cf15a4 in slade::MapRenderer3D::renderMap() (this=0x55555aa3c498) at /tmp/SLADE/src/MapEditor/Renderer/MapRenderer3D.cpp:673
#9  0x0000555555d4bca0 in slade::mapeditor::Renderer::drawMap3d() (this=0x55555aa3c258) at /tmp/SLADE/src/MapEditor/Renderer/Renderer.cpp:1210
#10 0x0000555555d4bfda in slade::mapeditor::Renderer::draw() (this=0x55555aa3c258) at /tmp/SLADE/src/MapEditor/Renderer/Renderer.cpp:1245
#11 0x0000555555da257a in slade::MapCanvas::draw() (this=0x55555a3665d0) at /tmp/SLADE/src/MapEditor/UI/MapCanvas.cpp:114
#12 0x0000555555f60747 in slade::OGLCanvas::onPaint(wxPaintEvent&) (this=0x55555a3665d0, e=...) at /tmp/SLADE/src/UI/Canvas/OGLCanvas.cpp:311
#13 0x00007ffff6cd3e62 in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#14 0x00007ffff6cd4a97 in wxEvtHandler::SearchDynamicEventTable(wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#15 0x00007ffff6cd4df5 in wxEvtHandler::TryHereOnly(wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#16 0x00007ffff6cd4e9f in wxEvtHandler::ProcessEventLocally(wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#17 0x00007ffff6cd4fba in wxEvtHandler::ProcessEvent(wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#18 0x00007ffff6cd77fb in wxEvtHandler::SafelyProcessEvent(wxEvent&) () at /usr/lib/libwx_baseu-3.2.so.0
#19 0x00007ffff742c637 in wxWindow::GTKSendPaintEvents(_cairo*) () at /usr/lib/libwx_gtk3u_core-3.2.so.0
#20 0x00007ffff7ea489e in  () at /usr/lib/libwx_gtk3u_gl-3.2.so.0
#21 0x00007ffff60496cd in  () at /usr/lib/libgtk-3.so.0
#22 0x00007ffff62f33c3 in  () at /usr/lib/libgtk-3.so.0
#23 0x00007ffff595e6c0 in g_closure_invoke () at /usr/lib/libgobject-2.0.so.0
#24 0x00007ffff598ca36 in  () at /usr/lib/libgobject-2.0.so.0
#25 0x00007ffff597d335 in  () at /usr/lib/libgobject-2.0.so.0
#26 0x00007ffff597dc77 in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#27 0x00007ffff597dd34 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#28 0x00007ffff6305ae3 in  () at /usr/lib/libgtk-3.so.0
#29 0x00007ffff6310862 in  () at /usr/lib/libgtk-3.so.0
#30 0x00007ffff61acc8b in gtk_main_do_event () at /usr/lib/libgtk-3.so.0
#31 0x00007ffff5ef6b77 in  () at /usr/lib/libgdk-3.so.0
#32 0x00007ffff5f08b02 in  () at /usr/lib/libgdk-3.so.0
#33 0x00007ffff5f0d158 in  () at /usr/lib/libgdk-3.so.0
#34 0x00007ffff5f0d375 in  () at /usr/lib/libgdk-3.so.0
#35 0x00007ffff597db73 in  () at /usr/lib/libgobject-2.0.so.0
#36 0x00007ffff597dc77 in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#37 0x00007ffff597dd34 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#38 0x00007ffff5f03fe9 in  () at /usr/lib/libgdk-3.so.0
#39 0x00007ffff5ef069e in  () at /usr/lib/libgdk-3.so.0
#40 0x00007ffff58593ee in  () at /usr/lib/libglib-2.0.so.0
#41 0x00007ffff5857f69 in  () at /usr/lib/libglib-2.0.so.0
#42 0x00007ffff58b6367 in  () at /usr/lib/libglib-2.0.so.0
#43 0x00007ffff5858b97 in g_main_loop_run () at /usr/lib/libglib-2.0.so.0
#44 0x00007ffff61aa2df in gtk_main () at /usr/lib/libgtk-3.so.0
#45 0x00007ffff73fbf76 in wxGUIEventLoop::DoRun() () at /usr/lib/libwx_gtk3u_core-3.2.so.0
#46 0x00007ffff6c25e52 in wxEventLoopBase::Run() () at /usr/lib/libwx_baseu-3.2.so.0
#47 0x00007ffff6c01f78 in wxAppConsoleBase::MainLoop() () at /usr/lib/libwx_baseu-3.2.so.0
#48 0x00007ffff6c63410 in wxEntry(int&, wchar_t**) () at /usr/lib/libwx_baseu-3.2.so.0
#49 0x00005555557387cb in main(int, char**) (argc=1, argv=0x7fffffffde78) at /tmp/SLADE/src/Application/SLADEWxApp.cpp:378

Seems that this delete here may be the culprit. That array is likely not even yet allocated.

void MapRenderer3D::checkVisibleFlats()
{
    // Update flats array
    delete[] flats_;
    n_flats_ = 0;
    for (unsigned a = 0; a < sector_flats_.size(); a++)
    {
sirjuddington commented 5 months ago

Hmm that's weird since deleting a null pointer is supposed to do nothing, not crash. Unless the pointer is invalid somehow but I can't see how that would happen.

That being said, I don't like that it's using raw new/delete so will look at changing it to vector and that should hopefully fix it

Pedro-Beirao commented 5 months ago

Tried commit https://github.com/sirjuddington/SLADE/commit/4921aa96d51583f436ef2e2608aec3aa2d932ba7 and it now freezes Slade instead of crashing right away

Same stack trace

Stack Trace:
0: [unknown location] wxFatalSignalHandler(int)
1: [unknown location] _sigtramp
2: [unknown location] slade::MapRenderer3D::checkVisibleFlats()
3: [unknown location] slade::MapRenderer3D::renderMap()
4: [unknown location] slade::mapeditor::Renderer::drawMap3d()
5: [unknown location] slade::mapeditor::Renderer::draw()
6: [unknown location] slade::MapCanvas::draw()
...
Pedro-Beirao commented 1 month ago

Tried with the new release and this still happens. Slade freezes when I go to 3d mode

sirjuddington commented 1 month ago

Do you still get the same stack trace too? Also is it a debug or release build? Would be handy to get line numbers in the trace. Could maybe even add some extra logging to checkVisibleFlats if it's still happening there

Pedro-Beirao commented 1 month ago

Quick debug shows that the exception occurs in MapRenderer3D.cpp line 3147

flats_[flat_idx++] = &flat;

flats_ is empty (size=0) but flat_idx is 1

sirjuddington commented 1 month ago

I have no idea how that could happen unless all sectors are invisible (behind the camera), in which case it wouldn't get to that line at all. What about n_flats_, is that 0 as well?

Pedro-Beirao commented 1 month ago

Yes it is 0. Im testing on doom2 map01, so there are sectors in front of the camera. Screenshot 2024-05-29 at 15 49 52

sirjuddington commented 1 month ago

Weird, what about sector_flats_? I can see there it has size 59 but are they just 59 empty vectors? If not I'm completely stumped heh

Pedro-Beirao commented 1 month ago

All of them are empty except the first

Screenshot 2024-05-29 at 16 48 17

sirjuddington commented 1 month ago

Ah I think I see what it is - is your flats_use_vbo cvar set to false? If I set it to false I get the crash.

Pedro-Beirao commented 1 month ago

Yes it was set to false. Setting it to true does fix it.

sirjuddington commented 1 month ago

Ok that's good, the latest commit should also fix the crash when the cvar is false

Pedro-Beirao commented 1 month ago

Yep latest commit did fix it. Thanks for looking into this!