First of all thanks for the clear information, I agree that

static const OsPath tempCacheFileName = VisualReplay::GetDirectoryName() / L"replayCache_temp.json";

is wrong because it was intended to use the value after g_args was set in RunGameOrAtlas of main.cpp.

Using global g_args early at variable creation while it is not filled yet is causing segmentation fault in Slackware

However, CmdLineArgs g_args; is constructed with the default constructor already, its member OsPath m_Arg0;. The OsPath has a std::wstring path; and a wchar_t separator;`.
The crash occurs in:

#0  0xb6672805 in std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::basic_string(std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&) () from /usr/lib/libstdc++.so.6

which sounds like the std::wstring path; default constructor fails (before the separator is set to slash or backslash).

This call occurs in /usr/lib/libstdc++.so.6, so slackware must have some different libstdc++ version or configuration, probably related to __static_initialization_and_destruction_0:

#5  0x0851e645 in __static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535) at ../../../source/ps/VisualReplay.cpp:43

So to me it sounds like both the VisualReplay.cpp static init call should be fixed, and perhaps one can fix the path class(es) somewhere before removing the one way to reproduce this segfault.
(Quick search says all other static consts are primitves that don't call functions that are assigned constant values, so VisualReplay.cpp is the only place with this mistake).

Trying to make sense from the C++ specs, the member std::wstring path of the class Path is not default-initialized to indeterminate value, but zero-initialized to empty string, right?
So does it segfaults when the Path constructor tries to zero-initialize path to empty string, or does it happen in the constructor when it tries to read from path in path.find('\\') in DetectSeparator()?
Either the Path class or Slackware settings don't abide by the C++ specs if I understand correctly.

@vladislavbelov do you see undefined behavior for the std::wstring path member in the path.h constructor that could explain why Slackware segfaults but not the other distributions, see stacktrace at #4789?

So it sounds like it segfaults due to an attempt to dereference an uninitialized pointer p.

But why is Path(const char* p) called and not Path()? Is the stacktrace otherwise identical to the one in the ticket?

The crash happens here:

At ps/GameSetup/CmdLineArgs.cpp:

OsPath CmdLineArgs::GetArg0() const
{
	return m_Arg0; <-- Here
}

I've started;

gdb ../pyrogenesis

Then

  break CmdLineArgs.cpp:109
(this line -> OsPath CmdLineArgs::GetArg0() const)

Then issue 5 'step', at 6th 'step':

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff3fe4bdb in std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::basic_string(std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&) () from /usr/lib64/libstdc++.so.6

then issue:

thread apply all bt full

Output is:
(gdb) #0 Path::Path (this=0x7fffffffd6d0) at ../../../source/lib/path.h:77
(gdb) No locals.
(gdb) #1 CmdLineArgs::GetArg0 (this=this@entry=0xe312c0 <g_args>) at ../../../source/ps/GameSetup/CmdLineArgs.cpp:111
(gdb) No locals.
(gdb) #2 0x00000000006750db in Paths::Paths (this=0x7fffffffdae0, args=...) at ../../../source/ps/GameSetup/Paths.cpp:35
(gdb) subdirectoryName = <optimized out>
Undefined command: "subdirectoryName". Try "help".
(gdb) func = "Paths"
Undefined command: "func". Try "help".
(gdb) #3 0x00000000006f7bc7 in VisualReplay::GetDirectoryName () at ../../../source/ps/VisualReplay.cpp:47
(gdb) paths = {m_root = {path = {static npos = 18446744073709551615,
Undefined command: "paths". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_rdata = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_gameData = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_userData = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_config = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_cache = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}, m_logs = {path = {static npos = 18446744073709551615,
Undefined command: "separator". Try "help".
(gdb) _M_dataplus = {<std::allocator<wchar_t>> = {<gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>},
Undefined command: "_M_dataplus". Try "help".
(gdb) _M_p = 0x7ffff4290158 <std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_Rep::_S_empty_rep_storage+24> L""}},
Undefined command: "_M_p". Try "help".
(gdb) separator = 47 L'/'}}
Undefined command: "separator". Try "help".
(gdb) #4 0x0000000000433501 in static_initialization_and_destruction_0 (initialize_p=1, priority=65535) at ../../../source/ps/VisualReplay.cpp:42
(gdb) No locals.
(gdb) #5 _GLOBALsub_I_VisualReplay.cpp(void) () at ../../../source/ps/VisualReplay.cpp:514
(gdb) No locals.
(gdb) #6 0x0000000000afd356 in do_global_ctors_aux ()
(gdb) No symbol table info available.
(gdb) #7 0x0000000000000000 in ?? ()
(gdb) No symbol table info available.

This is the content of g_args

break Paths.cpp:35

( which is 'm_root = Root(args.GetArg0());')

(gdb) p args
$1 = (const CmdLineArgs &) @0xe312c0: {m_Args = {<std::_Vector_base<std::pair<CStr8, CStr8>, std::allocator<std::pair<CStr8, CStr8> > >> = {

    _M_impl = {<std::allocator<std::pair<CStr8, CStr8> >> = {<__gnu_cxx::new_allocator<std::pair<CStr8, CStr8> >> = {<No data fields>}, <No data fields>}, 
      _M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}}, <No data fields>}, m_Arg0 = {path = {static npos = 18446744073709551615, 
    _M_dataplus = {<std::allocator<wchar_t>> = {<__gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>}, _M_p = 0x0}}, separator = 0 L'\000'}, 
m_ArgsWithoutName = {<std::_Vector_base<CStr8, std::allocator<CStr8> >> = {
    _M_impl = {<std::allocator<CStr8>> = {<__gnu_cxx::new_allocator<CStr8>> = {<No data fields>}, <No data fields>}, _M_start = 0x0, _M_finish = 0x0, 
      _M_end_of_storage = 0x0}}, <No data fields>}}

After change, g_args at runtime:
$2 = {m_Args = {<std::_Vector_base<std::pair<CStr8, CStr8>, std::allocator<std::pair<CStr8, CStr8> > >> = {

    _M_impl = {<std::allocator<std::pair<CStr8, CStr8> >> = {<__gnu_cxx::new_allocator<std::pair<CStr8, CStr8> >> = {<No data fields>}, <No data fields>}, 
      _M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}}, <No data fields>}, m_Arg0 = {path = {static npos = 18446744073709551615, 
    _M_dataplus = {<std::allocator<wchar_t>> = {<__gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>}, 
      _M_p = 0xe700f8 L"/home/weberkai/0ad-0.0.23b-alpha/binaries/system/pyrogenesis"}}, separator = 47 L'/'}, 
m_ArgsWithoutName = {<std::_Vector_base<CStr8, std::allocator<CStr8> >> = {
    _M_impl = {<std::allocator<CStr8>> = {<__gnu_cxx::new_allocator<CStr8>> = {<No data fields>}, <No data fields>}, _M_start = 0x0, _M_finish = 0x0, 
      _M_end_of_storage = 0x0}}, <No data fields>}}

Sorry, if stupid question, but... The error can be in the initialization, if empty?

: path(p, p+wcslen(p)) <- Here?

At source/lib/os_path.h:

    Path(const wchar_t* p)
		: path(p, p+wcslen(p))
    {
		DetectSeparator();
    }

In rP19674#33106, @weberkai wrote:

Sorry, if stupid question, but... The error can be in the initialization, if empty?

: path(p, p+wcslen(p)) <- Here?

At source/lib/os_path.h:

    Path(const wchar_t* p)
		: path(p, p+wcslen(p))
    {
		DetectSeparator();
    }

If it's this function http://www.cplusplus.com/reference/cwchar/wcslen/

This is the number of wide characters between wcs and the first null wide character (without including it).

Then uhm, the Slackware empty strings might not be null-terminated while the ones on other platforms might be. Maybe.
One can move the init list to the code and see with gdb what's in the given string p.

Also wondering whether the constructor with 0 arguments wasn't supposed to be called.

I've tried this test:

break path.h:82
break path.h:95
run

Then the program breaked at

Breakpoint 2, Path::Path (this=0x7fffffffdbb0, p=0xb24ec0 L"replayCache_temp.json") at ../../../source/lib/path.h:95
95		Path(const wchar_t* p)

And then segfault. I think the variable creation is fine, even GetDirectoryName() is working with empty string.

I think creating "OsPath tempCacheFileName" after evaluating the right side with this value (L"replayCache_temp.json") is causing Path to segfault.

In rP19674#33108, @weberkai wrote:

I think creating "OsPath tempCacheFileName" after evaluating the right side with this value (L"replayCache_temp.json") is causing Path to segfault.

Then this would also segfault:

static const OsPath cacheFileName = OsPath() / L"replayCache.json";

static const OsPath cacheFileName = L"replayCache.json";

?

Also why is that empty string, when I run it, it's not empty, but the correct replay path (GameDataPaths).

g_args was created with the default constructor, so the the UserData() function still returns the correct replay folder - except for Slackware.

In rP19674#33099, @elexis wrote:

@vladislavbelov do you see undefined behavior for the std::wstring path member in the path.h constructor that could explain why Slackware segfaults but not the other distributions, see stacktrace at #4789?

There is no undefined behaviour in the path.h. The problem is in the commit. g_args isn't constructed yet.

By C++11 standard:

Variables with ordered initialization defined within a single translation unit shall be initialized in the order of their definitions in the translation units. Otherwise, the initialization of a variable is indeterminately sequenced with respect to the initialization of a variable defined in a different translation unit.

So you shouldn't add dependencies between different non-trivial constructable objects that are in different translation unit. Why it doesn't work only for the Slackware platform? Probably because of some specific linkage order.

The current problem can be solved by a static local variable:

const OsPath& GetTempCacheFileName()
{
     static const OsPath tempCacheFileName = VisualReplay::GetDirectoryName() / L"replayCache_temp.json";
     return tempCacheFileName;
}

const OsPath& GetCacheFileName()
{
    static const OsPath cacheFileName = VisualReplay::GetDirectoryName() / L"replayCache.json";
    return cacheFileName;
}

But still you should be sure that it won't be called on a static global variable constructor stack.

Maybe a solution like this could be possible. Fill an object once at runtime and then call this object when needed.
https://stackoverflow.com/questions/14131855/c-assign-a-const-value-at-run-time
But I think declaring them as static const will not be possible.

In rP19674#33117, @weberkai wrote:

But I think declaring them as static const will not be possible.

I don't see a problem for a local static const except wrong dependencies.

In rP19674#33116, @vladislavbelov wrote:

g_args isn't constructed yet.

Are you sure that g_args_ isn't constructed yet on Slackware or that it is only not guaranteed to be constructed yet?

Because I agree with the latter, but the stacktrace looked suspicious, I was wondering whether there is a second bug on top of the first bug.
Why is the path constructor with one argument called, I thought the path constructor with zero arguments was to be called for the path in the CmdLineArgs constructor.

@weberkai can you confirm that the CmdLineArgs constructor had not been called before the VisualReplay lines are called?

Don't worry about VisualReplay, Imarok and me should be able to fix this, and it doesn't have to be static to begin with I think.

In rP19674#33120, @elexis wrote:

Are you sure that g_args_ isn't constructed yet on Slackware or that it is only not guaranteed to be constructed yet?

Yes, it's not guaranteed, and it's not constructed. I just tested it on Slackware 14.2.

Why is the path constructor with one argument called, I thought the path constructor with zero arguments was to be called for the path in the CmdLineArgs constructor.

If you have UB then the following actions are not guaranteed to be correct. First of all you need to fix the usage of uninitialised memory and only then check which constructor is called.

In rP19674#33121, @vladislavbelov wrote:

it's not constructed. I just tested it on Slackware 14.2.

Well then this doesn't indicate an issue with path.h (there might still be one despite the lack of indication though :P)

In rP19674#33116, @vladislavbelov wrote:

The current problem can be solved by a static local variable:

const OsPath& GetTempCacheFileName()
{
     static const OsPath tempCacheFileName = VisualReplay::GetDirectoryName() / L"replayCache_temp.json";
     return tempCacheFileName;
}

const OsPath& GetCacheFileName()
{
    static const OsPath cacheFileName = VisualReplay::GetDirectoryName() / L"replayCache.json";
    return cacheFileName;
}

I would propose to put it into the namespace VisualReplay::GetTempCacheFileName().

Do you want to upload a patch for that that the author of this patch can review or should the author create the patch?

But still you should be sure that it won't be called on a static global variable constructor stack.

I checked all static globals in source/, no others have this problem unless I missed some.

In rP19674#33122, @elexis wrote:

Well then this doesn't indicate an issue with path.h (there might still be one despite the lack of indication though :P)

Yep.

I would propose to put it into the namespace VisualReplay::GetTempCacheFileName().
Do you want to upload a patch for that that the author of this patch can review or should the author create the patch?

I don't mind, but I'm a bit busy with other patches. So if it's needed ASAP then it should be done by someone else.

I checked all static globals in source/, no others have this problem unless I missed some.

Thank you!

@Imarok wanna fix this, have it tested by @weberkai and approved by vladislav and me?

In rP19674#33124, @elexis wrote:

@Imarok wanna fix this, have it tested by @weberkai and approved by vladislav and me?

Sure, I can. Though tbh until now I don't fully understand the cause of the issue. I'll reread the whole thread...

@elexis should GetDirectoryName really called that name? Isn't GetDirectoryPath more appropriate?

In rP19674#33126, @Imarok wrote:

@elexis should GetDirectoryName really called that name? Isn't GetDirectoryPath more appropriate?

Yes, name relates to the topmost folder in the chain, so name is inaccurate.

(Was wondering whether it makes sense to pass the filename GetAboslutePath(relativePath), but that would have the disadvantage that one can't get the root directory without passing an empty string.)

See D1852

Hi!

Good news!

The Differential D1852 fix the issue!

Awesome work guys! Congratulations!

Thank you very much!

In rP19674#33135, @weberkai wrote:

Hi!

Good news!

The Differential D1852 fix the issue!

Awesome work guys! Congratulations!

Thank you very much!

Thank your for your effort in looking into the issue and notifying us ?