-
-
Notifications
You must be signed in to change notification settings - Fork 455
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.
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
Alpha version of Linux #282
Conversation
I'm getting a crash at the Crash report (Click to expand)
Valgrind Report (Click to expand)
GDB report (Click to expand)Program terminated with signal SIGTRAP, Trace/breakpoint trap.
#0 0x00007f94aff1fe09 in xrDebug::Fail(bool&, ErrorLocation const&, char const*, char const*, char const*, char const*) ()
from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrCore.so
[Current thread is 1 (Thread 0x7f94ad4f3040 (LWP 9334))]
(gdb) back
#0 0x00007f94aff1fe09 in xrDebug::Fail(bool&, ErrorLocation const&, char const*, char const*, char const*, char const*) ()
from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrCore.so
#1 0x00007f94aff201b7 in abort_handler(int) () from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrCore.so
#2 <signal handler called>
#3 0x00007f94aeb50d7f in raise () from /usr/lib/libc.so.6
#4 0x00007f94aeb3b672 in abort () from /usr/lib/libc.so.6
#5 0x00007f94aeb93878 in __libc_message () from /usr/lib/libc.so.6
#6 0x00007f94aeb9a18a in malloc_printerr () from /usr/lib/libc.so.6
#7 0x00007f94aeb9a6e4 in munmap_chunk () from /usr/lib/libc.so.6
#8 0x00007f94aef23517 in __gnu_cxx::new_allocator<char>::deallocate (this=0x7ffca776d090, __p=<optimized out>)
at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/ext/new_allocator.h:116
#9 std::allocator_traits<std::allocator<char> >::deallocate (__a=..., __n=<optimized out>, __p=<optimized out>)
at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/alloc_traits.h:462
#10 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy (__size=<optimized out>,
this=0x7ffca776d090) at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:226
#11 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose (this=0x7ffca776d090)
at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:221
#12 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string (this=0x7ffca776d090,
__in_chrg=<optimized out>) at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:657
#13 std::locale::locale (this=0x7ffca776d128, __s=<optimized out>) at /build/gcc/src/gcc/libstdc++-v3/src/c++98/localename.cc:94
#14 0x00007f94b0f0639a in CInput::get_dik_name (this=<optimized out>, dik=513, dest_str=0x7ffca776d1c0 "", dest_sz=128)
at /media2/xray-16/src/xrEngine/xr_input.cpp:198
#15 0x00007f94a04d6d5f in remap_keys() () from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrGame.so
#16 0x00007f94a04d8285 in CCC_Bind::Execute(char const*) ()
from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrGame.so
#17 0x00007f94b0ebcee6 in CConsole::ExecuteCommand (this=0x7f94a2081600, cmd_str=<optimized out>, record_cmd=<optimized out>)
at /media2/xray-16/src/xrEngine/XR_IOConsole.cpp:571
#18 0x00007f94b0f0813f in CCC_LoadCFG::Execute (this=0x7f94b0f51ea0 <CCC_Register()::xCCC_LoadCFG>, args=<optimized out>)
at /media2/xray-16/src/xrEngine/xr_ioc_cmd.cpp:267
#19 0x00007f94b0ebcee6 in CConsole::ExecuteCommand (this=0x7f94a2081600, cmd_str=<optimized out>, record_cmd=<optimized out>)
at /media2/xray-16/src/xrEngine/XR_IOConsole.cpp:571
#20 0x00007f94a04d7cde in CCC_DefControls::Execute(char const*) ()
from /media2/wine64/drive_c/GOG Games/S.T.A.L.K.E.R. Call of Pripyat/xrGame.so
#21 0x00007f94b0ebd028 in CConsole::ExecuteCommand (this=0x7f94a2081600, cmd_str=<optimized out>, record_cmd=<optimized out>)
at /media2/xray-16/src/xrEngine/XR_IOConsole.cpp:560 |
@@ -503,7 +503,7 @@ bool CScriptEngine::namespace_loaded(LPCSTR name, bool remove_from_stack) | |||
int start = lua_gettop(lua()); | |||
lua_pushstring(lua(), GlobalNamespace); | |||
lua_rawget(lua(), LUA_GLOBALSINDEX); | |||
string256 S2; | |||
string256 S2 = { 0 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C++ it's enough to do just = { }
to initialize arrays with nulls.
d4895db
to
12eb46f
Compare
@@ -2799,7 +2799,7 @@ extern "C" ODE_API void dJointSetFixedQuaternionPos (dxJointFixed *joint,dQuater | |||
// set joint->qrel to the transpose of the first body's q | |||
joint->qrel[0] = quaternion[0];//joint->node[0].body->q[0]; | |||
for (i=1; i<4; i++) joint->qrel[i] = -quaternion[i];//-joint->node[0].body->q[i]; | |||
for (i=0; i<4; i++) joint->offset[i] = pos[i];//joint->node[0].body->posr.pos[i]; | |||
for (i=0; i<3; i++) joint->offset[i] = pos[i];//joint->node[0].body->posr.pos[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the magic number "3"? Can we use the expression like sizeof(array)/sizeof(array[0]) here? Or maybe define some constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not array, type of pos is a pointer to dReal, but it a pointer to the first component of the vector is passed to the function. Even if we pass the link to the Fvector itself there, we cannot calculate the size of the components, we need to know in advance how much to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sucks :( This 'style' should lead to a lot of typos. I believe we find a LOT of such bugs later.
@@ -45,7 +45,7 @@ if (WIN32) | |||
set(_TBB_DEFAULT_INSTALL_DIR "C:/Program Files/Intel/TBB" "C:/Program Files (x86)/Intel/TBB") | |||
set(_TBB_LIB_NAME "tbb") | |||
set(_TBB_LIB_MALLOC_NAME "${_TBB_LIB_NAME}malloc") | |||
set(_TBB_LIB_MALLOC_PROXY_NAME "${_TBB_LIB_NAME}malloc_proxy") | |||
# set(_TBB_LIB_MALLOC_PROXY_NAME "${_TBB_LIB_NAME}malloc_proxy") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove this garbage instead of commenting it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that I will not be required to return it.
@@ -85,7 +85,7 @@ inline void _splitpath(const char* path, // Path Input | |||
) | |||
{ | |||
if(!path) | |||
return EINVAL; | |||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the NULL in path is expected behaviour? Seems like not - we don't report the error to the caller. So... Let's insert some type of assertion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case is defined in MSDN in the following way:
If path is NULL, the invalid parameter handler is invoked, as described in Parameter Validation. If execution is allowed to continue, errno is set to EINVAL and the function returns EINVAL.
We're trying to reproduce that second case. Notice that it doesn't make sense, because _splitpath is supposed to return void
. Maybe it should insert errno = EINVAL
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, reporting an error is an alternative solution. If this function tries to mimic the Windows implementation - I agree that it's the preferrable way.
@@ -85,7 +85,7 @@ inline void _splitpath(const char* path, // Path Input | |||
) | |||
{ | |||
if(!path) | |||
return EINVAL; | |||
return; | |||
|
|||
const char *p, *end; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's init the variables directly in this line
float f = 1.f; | ||
ZeroMemory(bufFrame, sizeof(bufFrame)); | ||
MemFill32(bufDepth, *LPDWORD(&f), size); | ||
MemFill32(bufDepth, *(u32 *)(&f), size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static_cast, please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static_cast will fill array integer numbers, but we need fill float value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reinterpret_cast then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reinterpret_cast doesn't seem to be a preferred solution - it's not the device driver's code to use such 'hard' types of casts. Maybe it's better to write a cycle here to avoid magic with pointers and the potential problems in the future? I believe the compiler is smart enough to optimize the cycle in the binary.
@@ -6,7 +6,7 @@ using namespace PAPI; | |||
void ParticleAction::Load(IReader& F) | |||
{ | |||
m_Flags.assign(F.r_u32()); | |||
type = (PActionEnum)F.r_u32(); | |||
type = PActionEnum(F.r_u32()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static_cast?
@@ -313,7 +313,7 @@ u32 CParticleManager::LoadActions(int alist_id, IReader& R) | |||
u32 cnt = R.r_u32(); | |||
for (u32 k = 0; k < cnt; k++) | |||
{ | |||
ParticleAction* act = CreateAction((PActionEnum)R.r_u32()); | |||
ParticleAction* act = CreateAction(PActionEnum(R.r_u32())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static_cast?
#ifdef LINUX | ||
__attribute__((constructor)) | ||
#endif | ||
static void load(int argc, char** argv, char** envp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const char** ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutable args match to the standard main signature.
https://stackoverflow.com/questions/2108192/what-are-the-valid-signatures-for-cs-main-function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But looks like we don't expect the modification of the arguments. So, making the arguments 'const' should help us to avoid occasional modifying data in the future. It makes our method safer. In addition, the 'const' variables allow the compiler to use advanced optimization techniques sometimes.
Possible calling free() for stack pointer in src/xr_3da/entry_point.cpp:
and then:
It should be:
and then
|
Думаю что
и
всё равно не совместимы. |
Присмотрись внимательней, там этой строчки давно уже нет. |
bit counter on GCC Built-in Function
add commented line for enable memory sanityzers
I think, now changes more complicated. |
This PR is fix of many various crashes at Linux. The game no longer crashes in random places.