TTimo / GtkRadiant

The open source, cross platform level editor for idtech games
http://icculus.org/gtkradiant/
Other
586 stars 155 forks source link

NULL pointer dereference in PrefsDlg::LoadPrefs() #485

Open wtfbbqhax opened 7 years ago

wtfbbqhax commented 7 years ago
~/D/w/g/G/b/d/radiant ❯❯❯ lldb ./radiant.bin                                                                                                  V master
(lldb) target create "./radiant.bin"
Current executable set to './radiant.bin' (x86_64).
(lldb) r
Process 70456 launched: './radiant.bin' (x86_64)
Process 70456 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00000001000d4956 radiant.bin` PrefsDlg::LoadPrefs(this=0x00000001001615b0)  + 134 at preferences.cpp:2943
   2940     }
   2941
   2942     // load local.pref file
-> 2943     mLocalPrefs.ReadXMLFile( m_inipath->str );
   2944
   2945     mLocalPrefs.GetPref( PATCHSHOWBOUNDS_KEY,  &g_bPatchShowBounds,  FALSE );
   2946     mLocalPrefs.GetPref( MOUSE_KEY,            &m_nMouse,            MOUSE_DEF );
(lldb) p m_inipath
(GString *) $0 = 0x0000000000000000
wtfbbqhax commented 7 years ago

Backtrace

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001000d4956 radiant.bin` PrefsDlg::LoadPrefs(this=0x00000001001615b0)  + 134 at preferences.cpp:2943
    frame #1: 0x0000000100043187 radiant.bin` Error(error="No games setup, aborting\n")  + 855 at error.cpp:121
    frame #2: 0x00000001000cbb6b radiant.bin` CGameDialog::Init(this=0x0000000100161628)  + 283 at preferences.cpp:1294
    frame #3: 0x00000001000ccd33 radiant.bin` PrefsDlg::Init(this=0x00000001001615b0)  + 35 at preferences.cpp:1464
    frame #4: 0x0000000100066b0c radiant.bin` mainRadiant(argc=1, argv=0x00007fff5fbff190)  + 2316 at main.cpp:743
    frame #5: 0x0000000100067482 radiant.bin` main(argc=1, argv=0x00007fff5fbff190)  + 34 at main.cpp:992
    frame #6: 0x00007fffb0174235 libdyld.dylib` start  + 1
Pan7 commented 7 years ago

What Radiant version is that? can you try the latest?

ensiform commented 7 years ago

Does radiant actually support x86_64 fully? I Thought only the q3map2 portion was x86_64 compatible and working.

TTimo commented 7 years ago

We have never released an official x86_64 build for Windows. But on an x86_64 Linux system, you will compile and run an x86_64 version .. there have not been bugs reported that are specific to the 64 bit, there is less use and coverage on Linux but x86_64 on Windows should be mostly fine.

Unrelated to this bug though - iirc @Pan7 had said this crash happens when no game pack is installed or configured and you try to start the editor anyway.

wtfbbqhax commented 7 years ago

This was with the latest master code at the time.

The problem is not related to 64bit- it's a weird cyclic dependency on m_inipath, initialization that sets this variable is referencing it.

This only happens when Error() occurs prior to initialization- e.g., the startup game selection menu stuff. (Sorry, I don't currently have it installed to provide more specific details)

ensiform commented 7 years ago

Perhaps this might help, I'm not sure:

diff --git a/radiant/preferences.cpp b/radiant/preferences.cpp
index 30a6dbe2..2b445f28 100644
--- a/radiant/preferences.cpp
+++ b/radiant/preferences.cpp
@@ -1264,6 +1264,9 @@ void CGameDialog::InitGlobalPrefPath(){
 void CGameDialog::Reset(){
    if ( !g_PrefsDlg.m_global_rc_path ) {
        InitGlobalPrefPath();
+       if ( !g_PrefsDlg.m_global_rc_path ) {
+           Error( "Failed to initialize global settings path, aborting\n" );
+       }
    }
    CString strGlobalPref = g_PrefsDlg.m_global_rc_path->str;
    strGlobalPref += "global.pref";

Mind you I also fixed Error() so that it was a proper noreturn function as well.

pbtoast commented 4 years ago

Hey @wtfbbqhax, I just got a pull request merged that might fix this. If you're still in a position to check, can you give the latest master a try and close this issue if you're satisfied that it's resolved? Thanks!