OpenXcom Forum
OpenXcom Forks => OXCE Bugs => OpenXcom Extended (OXCE) => OXCE Bugs FIXED => Topic started by: Buscher on December 28, 2024, 11:36:10 am
-
Only affects Windows, doesn't happen on Linux (Xubuntu)
Running Extended-8.0.0-2e70fa97b-2024-12-27-win64 a multiline set up with (double) quotes will crash the game.
Our member Mercury reported it worked fine previously on Windows builds for the release candidate for rapidyaml builds (self-built). They were using a rapidyaml branch for testing 8.0.
STR_GENERAL_STORES_UFOPEDIA: "All equipment,
weapons systems, munitions, recovered material and Heavy Weapons Platforms are placed in stores, including equipment assigned to craft in hangars."
It works fine with a block scalar (both literal | and folded >)
STR_GENERAL_STORES_UFOPEDIA: > # or alternatively |
All equipment,
weapons systems, munitions, recovered material and Heavy Weapons Platforms are placed in stores, including equipment assigned to craft in hangars.
We (ROSIGMA) have changed from double quotes to block scalar for now as a stop gap measure.
-
It works fine when run from Visual Studio on Windows.
Only affects the MXE cross-build.
-
I can reproduce this on windows, if I run the 64bit release build.
However, I've gone through the code step by step, and failed to find a place in code where it could fail. It doesn't seem like there is any OS-dependent code while processing a quoted scalar.
Might have something to do with x64. Where can I get precompiled 64bit libs, so I could build x64 in VS? :x
-
I can reproduce this on windows, if I run the 64bit release build.
However, I've gone through the code step by step, and failed to find a place in code where it could fail. It doesn't seem like there is any OS-dependent code while processing a quoted scalar.
Same happens on the 32bit build for me.
[28-12-2024_14-13-29] [FATAL] Error loading file 'c:/Users/meridian/Downloads/oxce800/Extended-8.0.0-2e70fa97b-2024-12-27-win32/user/mods/multiline_crasher.zip/multiline_crasher/Language/en-US.yml'
[28-12-2024_14-13-29] [ERROR] Rapidyaml ERROR: reached end of file looking for closing quote
Might have something to do with x64. Where can I get precompiled 64bit libs, so I could build x64 in VS? :x
SupSuper has provided precompiled 64bit libs to Xilmi somewhere... ah, here: https://openxcom.org/forum/index.php?topic=11660.msg160324#msg160324
-
Thanks. Ok so, it does crash for me if I run the 32bit build from here (http://108.61.157.159/versions/win/Test-Extended-8.0.0-ab0bd4040-2024-12-27-win32.7z), or 64bit build from here (https://openxcom.org/oxce/release/oxce80/Extended-8.0.0-2e70fa97b-2024-12-27-win64.7z)
But doesn't crash if I compile it myself. Neither debug, nor release, neither 32bit nor 64bit if I build them myself in VS.
Whoever built these 32/64 bit versions, can they build a debug build?
Edit: Wait.. the "Test-Extended-8.0.0-ab0bd4040-2024-12-27-win32.7z" zip says "win32", but it's actually 64bit version inside lol. Where's the 32bit build that crashes?
-
if I would guess I would bets on `\r\n` vs `\n`, as some confusion what version should happens and what was available. Btw is legal for yaml to have real new lines in double quotes?
-
Yes, the standard says that new lines in double quoted scalars are fine.
Debugging assembler is hard, but I'm slowly making progress lol
-
I think what's going on is, the compiler optimized
this line of code (https://github.com/Delian0/OpenXcom/blob/aae66473b4bc1b3717cbda6c6b413cf0c51770f0/libs/rapidyaml/c4/yml/parse_engine.def.hpp#L1837)
in such a way that in subsequent loops it's not executing. However, it's critical that that line executes on every loop, because it contains the string view to the current line of text in document that's being processed.
I think removing the const (or making it const reference) would prevent this optimization from happening and fix the error.
Basically, that loop should be moving through document line by line until it finds the closing quote. But what I'm seeing in assembler is that the search keeps happening in one and the same document line (and somehow not crashing). So what I need is the MXE build with the const removed to see if it still crashes.
-
Commit: https://github.com/MeridianOXC/OpenXcom/commit/39501f356465010bfaa3052fbcc30b30cfba5e87
Build: http://108.61.157.159/versions/win/Test-Extended-8.0.0-39501f356-2024-12-27-win32.7z
Doesn't seem to be solved yet.
If I should try also const reference, please paste here exactly how the new code should look like.
-
This is wrong function, its handle `'` you need make similar fix in second function that handle `"`
-
Well, I tried with single quotes, and it still failed.
In ParseEngine<EventHandler>::_scan_scalar_squot()
and ParseEngine<EventHandler>::_scan_scalar_dquot()
try changing
const csubstr line = m_evt_handler->m_curr->line_contents.rem;
to
const csubstr& line = m_evt_handler->m_curr->line_contents.rem;
If that doesn't work, then... a .pdb would be helpful, so I wouldn't need to beat myself with assembler.
-
Well, I tried with single quotes, and it still failed.
In ParseEngine<EventHandler>::_scan_scalar_squot()
and ParseEngine<EventHandler>::_scan_scalar_dquot()
try changing
const csubstr line = m_evt_handler->m_curr->line_contents.rem;
to
const csubstr& line = m_evt_handler->m_curr->line_contents.rem;
Commit: https://github.com/MeridianOXC/OpenXcom/commit/f610f9f415b5a0d546088ac6dca7345f9fe2e8a4
Build: http://108.61.157.159/versions/win/Test-Extended-8.0.0-f610f9f41-2024-12-27-win32.7z
Unfortunately still no luck.
If that doesn't work, then... a .pdb would be helpful, so I wouldn't need to beat myself with assembler.
@Yankes: can you provide that?
-
Well, that variable is still being optimized away in a weird way...
Maybe the macro is causing the compiler to misbehave, so try removing the line that reads:
_c4dbgpf("scanning double quoted scalar @ line[{}]: line='{}'", m_evt_handler->m_curr->pos.line, line);
And if that doesn't work, then try changing:
const csubstr line = m_evt_handler->m_curr->line_contents.rem;
into
substr line{};
line.str = m_evt_handler->m_curr->line_contents.rem.str;
line.len = m_evt_handler->m_curr->line_contents.rem.len;
-
Commit: https://github.com/MeridianOXC/OpenXcom/commit/f610f9f415b5a0d546088ac6dca7345f9fe2e8a4
Build: http://108.61.157.159/versions/win/Test-Extended-8.0.0-f610f9f41-2024-12-27-win32.7z
Unfortunately still no luck.
@Yankes: can you provide that?
I can't provide `.pdb` as this is MSVC specific file, only thing I can add is symbols for DWARF. overall I can try check it under cygwin but what version of 40k exactly crash? I have some older version that do not have fix and fail on comments.
-
You don't need to run any mod. Just put the contents from OP's zip file
multiline_crasher.zip/.../en-US.yml
into
standard/anything/metadata.yml
Technically, you could also just run this code anywhere:
YAML::YamlString yaml(
"#test\n"
"\n"
" STR_GENERAL_STORES_UFOPEDIA: \"All equipment,\n"
" weapons systems, munitions, recovered material and Heavy Weapons Platforms are placed in stores, including equipment assigned to craft in hangars.\"");
YAML::YamlRootNodeReader reader(yaml, "");
-
Well, that variable is still being optimized away in a weird way...
Maybe the macro is causing the compiler to misbehave, so try removing the line that reads:
_c4dbgpf("scanning double quoted scalar @ line[{}]: line='{}'", m_evt_handler->m_curr->pos.line, line);
And if that doesn't work, then try changing:
const csubstr line = m_evt_handler->m_curr->line_contents.rem;
into
substr line{};
line.str = m_evt_handler->m_curr->line_contents.rem.str;
line.len = m_evt_handler->m_curr->line_contents.rem.len;
Macro probaby should not affect anything as it should be replaced by white space (until we set `RYML_DBG`)
Overall "disappearing" variable this is typical in release build as compiler could provide that he can get this value in other way.
question is if this is optimization is problem or it simply only hide bug some where else?
beside if we want hinder optimization one way is put `volatile` here, it force compiler to preserve all reads and writes to given variable.
[ps]
> You don't need to run any mod. Just put the contents from OP's zip file
aaa ok.
-
I try open it under cygwin and it run fine. I think we can try bump compiler version on build server, it could be we have bugged version and other will work.
-
one way is put `volatile` here
That's a good idea :D next thing to try!
Anyway, after the optimization cause the line variable to be optimized away, in this for loop
for(size_t i = 0; i < line.len; ++i)
const char curr = line.str;
the line.len and line.str become constants, so they're never updated during the whole function's execution.
-
I think one place that could be in fault is: `EventHandler *C4_RESTRICT m_evt_handler;` this `C4_RESTRICT` it mean that only using this pointer you can access this memory. If this value is updated in other function it could confuse compiler into assuming nobody touch this memory behind this pointer.
-
That's a good idea :D next thing to try!
Commit: https://github.com/MeridianOXC/OpenXcom/commit/417475bca6bed0e2ff1b02fc792b289eff33b532
Build: http://108.61.157.159/versions/win/Test-Extended-8.0.0-417475bca-2024-12-27-win32.7z
Didn't help :(
I think we can try bump compiler version on build server, it could be we have bugged version and other will work.
Now is probably a good time to update build environments anyway.
So it's definitely worth a try to update to newest MXE...
PS:
(Later I will try upgrading the Android build too)
(Santa Claus didn't send me a new Macbook this Xmas, but I'll try to get access to some new Apple hardware too, my macOS 10.15 builds are probably not used by anyone anymore)
(and lastly, I will also update native Linux builds from Kubuntu 18.04 to 24.04 if possible... or is there a more popular distro nowadays? I read/heard somewhere something about Kubuntu being 'evil'... as all distros eventually will become in the eyes of the Linux community)
-
Commit: https://github.com/MeridianOXC/OpenXcom/commit/417475bca6bed0e2ff1b02fc792b289eff33b532
Actually, this doesn't even compile.
How come the build farm created a build???
And how should I change the code so that it compiles?
-
To successfully compile with volatile, also need to change L1962
|| (_at_line_begin() && m_evt_handler->m_curr->pos.line, line.begins_with(' '));
to
|| (_at_line_begin() && m_evt_handler->m_curr->pos.line, m_evt_handler->m_curr->line_contents.rem.begins_with(' '));
-
Yes, that helped.
Now it both compiles and also works.
Commit: https://github.com/MeridianOXC/OpenXcom/commit/ad14a6fc29770f2b26ce41cb81f01d046c73b0f3
Build: http://108.61.157.159/versions/win/Test-Extended-8.0.0-ad14a6fc2-2024-12-27-win32.7z
-
Which specific compiler do MXE builds use? I'll need to know that to make an issue on rapidyaml git.
Now comes the real problem, which is the fact that this solution requires a custom library fix lol
I guess first we should see if a newer compiler version doesn't create the same optimization error.
And if that doesn't work... then it's probably best to tell modders to "avoid breaking line for double or single quoted strings".
-
$ /opt/mxe/usr/bin/x86_64-w64-mingw32.static-g++ --version
x86_64-w64-mingw32.static-g++ (GCC) 11.3.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
As I see new version have:
"gcc": {"version": "11.5.0", "website": "https://gcc.gnu.org/", "description": "GCC"},
I will update it but is small change that this fix it as only difference is "bug fixes".
> optimization error
we should try remove `restrict` in clean version too and see if it affect this optimization. if it dose this means is lib try be too clever for compiler.
-
PS:
(and lastly, I will also update native Linux builds from Kubuntu 18.04 to 24.04 if possible... or is there a more popular distro nowadays? I read/heard somewhere something about Kubuntu being 'evil'... as all distros eventually will become in the eyes of the Linux community)
Short Answer:
I think Kubuntu is a good choice to continue with.
Long Answer:
I am not that deep in what the "Linux community" thinks to be "evil". In general I can say that Canonical (Ubuntu) needs to make some money at some point to cover their costs, so they try to commercialize it a bit more. Be it the Ubuntu Software Center (a store), selling professional support (kinda like Red Hat) or becoming relevant for the IoT market (Ubuntu Core). Debian is also seemingly becoming more commercial with the extended LTS (10 years) but it's a service provided by a different project (Freexian). KDE (Kubuntu) got a bit bad reputation for eating a lot of resources and not managing to get a stable environment while communicating badly a few years ago. No clue what's it like now.
I personally like Xfce as a desktop environment and use Xubuntu for ease of use. I personally don't like Unity (desktop environment, not the game engine), that's why I don't use Ubuntu. There is also Linux Mint which is ok. It's very popular for people starting out with Linux. For one of my old Thinkpads I am using Zorin OS but that's becoming slow too. Fedora and OpenSUSE I tried ages ago.
Ubuntu is also used a lot for docker containers (f. ex. in github workflows). For a server VM I would probably use anything that has apt like Debian or some kind of Ubuntu LTS with your favorite desktop environment and its applications. I would definitely NOT use anything that is a rolling release (Arch, Manjaro). Manjaro managed to corrupt my boot partition a few times. You can probably use Kubuntu as it's always beneficial that you are familiar with it and the KDE devs seem to have managed to make it stable.
-
"gcc": {"version": "11.5.0", "website": "https://gcc.gnu.org/", "description": "GCC"},
we should try remove `restrict` in clean version too and see if it affect this optimization. if it dose this means is lib try be too clever for compiler.
If 11.5 doesn't work, could you also try 14.2?
There could be other code that might be hurt (become bugged or have worse performance) by removing "restrict". And even if removing restrict would work, it would still be a "custom library fix" which we're trying to avoid.
-
If 11.5 doesn't work, could you also try 14.2?
There could be other code that might be hurt (become bugged or have worse performance) by removing "restrict". And even if removing restrict would work, it would still be a "custom library fix" which we're trying to avoid.
I can't easy update compiler as it prepared by mxe devs. it could have some custom patches or other changes to make it work.
probably easier would be downgrade to older version.
beside removing `restrict` should not break any code, only thing hit could be performance as now compiler need to reload memory that previously could assume that should do not change. but yes, we should prefer fix from lib itself or work around that do not need mess with lib code.
I suggest removing `restrict` more to confirm my suspicion, as pure C++ do not have this keyword and problem we have is reason why C++ committee do not manage to make sense how it should work. Another workaround would be change optimization level as problem like this could disappeared when more or less functions get inlined.
[ps]
I now rebuilding new version of gcc from mxe, "nightly" builds should lag heavily or not be available for some time
-
Looks like you can use a newer gcc with plugins?
Quoted from random google result: https://github.com/mxe/mxe/issues/3083
You might want to try using a more recent gcc. For example, you could add this to your settings.mk file:
MXE_PLUGIN_DIRS := plugins/gcc14
Latest seems to be 14.2.0: https://github.com/mxe/mxe/commit/a513b17f32f715b5295b37f96bc53acc7907ea71
-
Btw, what optimization flags was the MXE build with?
-
Should use default set by cmake in `-DCMAKE_BUILD_TYPE=Release` build
-
I mean, the OpenXcomEx.exe that was built with MXE GCC 11.3, which was crashing, was it with -O3 or what?
-
I check my local cmake output and it run with -O3, looking on `CMakeLists.txt` I do not see any where explicit setting it.
Probably same should be on build server.
-
New Windows build with gcc14: http://108.61.157.159/versions/win/Extended-8.0.0-ae6ce0fb0-2024-12-30-win64.7z
Seems to work fine now.
-
So using gcc14, it works fine even without volatile?
Btw, did you test with 11.5? Asking so I can include the findings in the issue I made on ryml git.
-
on 11.5 OXCE do not compile at all :D it get some ICE crash
-
Short Answer:
I think Kubuntu is a good choice to continue with.
Thanks, I tried compiling on Kubuntu 24.04 and it worked almost without problems.
Then I tried running it on Kubuntu 18.04 and it didn't even start, complaining about GLIBC_2.34 not found.
And 7 similar errors about GLIBC and GLIBCXX just with different version number each time.
Then I wanted to install newer GLIBC, so I asked Google how to do it and the first 3 posts warned me that my computer may explode if I try to do that.
So much for user friendliness of Linux.
They are exactly the same as MacOS, every version being incompatible with the previous and the next.
/rant over
I'll keep compiling under Kubuntu 18.04.