Emulator Issues #9233
closed
Compiling outside a git tree
0%
Description
If Dolphin is compiled either without git installed or from a non-git source tree (eg downloaded through a link on GitHub), subtle errors occur due to the use of scm_rev_git_str. Without git scm_rev_git_str expands to an empty string whereas certain uses assume it to be a string with exactly 40 chars.
For example, compiling without git and with asan enabled immediately causes dolphin to crash on startup with:
==15199==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000188d4a1 at pc 0x7f5282980445 bp 0x7ffd69eaeb00 sp 0x7ffd69eae2b0
READ of size 40 at 0x00000188d4a1 thread T0
#0 0x7f5282980444 in __asan_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x88444)
#1 0xf0b74e in LinearDiskCache<OGL::SHADERUID, unsigned char>::Header::Header() /tmp/dol/Source/Core/Common/LinearDiskCache.h:189
#2 0xf09e1e in LinearDiskCache<OGL::SHADERUID, unsigned char>::LinearDiskCache() (/tmp/dol/build/Binaries/dolphin-emu-nogui+0xf09e1e)
#3 0xf08079 in __static_initialization_and_destruction_0 /tmp/dol/Source/Core/VideoBackends/OGL/ProgramShaderCache.cpp:35
#4 0xf0818f in _GLOBAL__sub_I__ZN3OGL18ProgramShaderCache17s_ubo_buffer_sizeE /tmp/dol/Source/Core/VideoBackends/OGL/ProgramShaderCache.cpp:644
#5 0x117d5ac in __libc_csu_init (/tmp/dol/build/Binaries/dolphin-emu-nogui+0x117d5ac)
#6 0x7f527b95c7fe in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x207fe)
#7 0x51c2d8 in _start (/tmp/dol/build/Binaries/dolphin-emu-nogui+0x51c2d8)
0x00000188d4a1 is located 0 bytes to the right of global variable '*.LC2' defined in '/tmp/dol/Source/Core/Common/Version.cpp' (0x188d4a0) of size 1
'*.LC2' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow ??:0 __asan_memcpy
Originally noticed after this PR:
https://github.com/dolphin-emu/dolphin/pull/3467
If I've read it right, the netplay version will always be the empty string for anyone compiling dolphin without git.
Either building dolphin within a git tree (and with git installed) need to be enforced, or each occurrence of scm_rev_git_str (and possibly other scm_ variables) needs to be fixed to work when they're empty.
Updated by delroth over 8 years ago
- Milestone set to Current
- Regression changed from No to Yes
Many distros will be using a tarball for 5.0 and not compiling from Git, let's not force them to patch our code :)
Updated by delroth about 8 years ago
- Assignee set to delroth
- Easy changed from No to Yes
My current plan for this: we are not going to support people making their own tarballs from our git repository and expecting it to work. For the tarballs we release, we are going to ship a premade scmrev.h which won't have this problem.
Updated by phire about 8 years ago
Sure, that works for the 5.0 release.
But the core issue of "it would be nice for tarballs from the git repo to build" will need to be tackled at some point, even if they have no idea what version of dolphin they are.
Just make sure they say "Unknown Version" instead of 5.0
Updated by BhaaL almost 8 years ago
What about simply going and adding a DEFINE (or bool or anything else) from the SCM script that says "hey, we're a valid working copy with all the infos, go ahead and use them" while changing the usage sites to check for it and say "Unversioned Build", "Unknown Version" or whatever in the undefined case.
It's easy to do, straight-forward and doesn't need other mumbo jumbo that would make it more painful. For official tarballs, we could just use a smudge-filter or post-checkout hook to populate scmrev.h
Updated by degasus almost 8 years ago
4.0-9289 fixes the handling of scm_ strings with different lenght. No idea if this issue shall be marked as fixed through...
Updated by mimimi almost 8 years ago
So Dolphin does compile and work now, if downloaded from github as .zip file. The title bar says "Dolphin []" in this case, tested with the release build.
Is that good enough? Or do we want something, where people can set manually what it says on non git builds? Wasn't there talk about this for the linux distributions, so they can set this to Dolphin 5.0 on the stable build?
Updated by mathieui almost 8 years ago
mimimi wrote:
Is that good enough? Or do we want something, where people can set manually what it says on non git builds? Wasn't there talk about this for the linux distributions, so they can set this to Dolphin 5.0 on the stable build?
We could include the 5.0 string in a source archive that distributions would use
Updated by delroth almost 8 years ago
- Status changed from Accepted to Fix pending
https://github.com/dolphin-emu/dolphin/pull/3869 should make this a bit better by having CMake fill the scmrev defaults with the version info we already hardcode in CMakeLists.
Updated by delroth almost 8 years ago
- Status changed from Fix pending to Fixed
We now don't crash and provide some dummy version number (basically, whatever the last release was).
We still heavily recommend building from a git tree, even for Linux distributions.