aliens

Author Topic: [SOLVED] Yaml Multiline with (double) quotes crashes the game on Win  (Read 931 times)

Online Yankes

  • Global Moderator
  • Commander
  • *****
  • Posts: 3394
    • View Profile
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.

Online Yankes

  • Global Moderator
  • Commander
  • *****
  • Posts: 3394
    • View Profile
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.

Offline Delian

  • Commander
  • *****
  • Posts: 580
    • View Profile
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.

Online Yankes

  • Global Moderator
  • Commander
  • *****
  • Posts: 3394
    • View Profile
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.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9197
    • View Profile
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)

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9197
    • View Profile
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?
« Last Edit: December 29, 2024, 10:54:24 am by Meridian »

Offline Delian

  • Commander
  • *****
  • Posts: 580
    • View Profile
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(' '));
« Last Edit: December 29, 2024, 11:37:30 am by Delian »


Offline Delian

  • Commander
  • *****
  • Posts: 580
    • View Profile
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".
« Last Edit: December 30, 2024, 03:53:59 pm by Delian »

Online Yankes

  • Global Moderator
  • Commander
  • *****
  • Posts: 3394
    • View Profile
Code: [Select]
$ /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:

Code: [Select]
"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.

Offline Buscher

  • Colonel
  • ****
  • Posts: 181
    • View Profile
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.

Offline Delian

  • Commander
  • *****
  • Posts: 580
    • View Profile
"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.
« Last Edit: December 29, 2024, 07:01:35 pm by Delian »

Online Yankes

  • Global Moderator
  • Commander
  • *****
  • Posts: 3394
    • View Profile
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
« Last Edit: December 29, 2024, 04:42:16 pm by Yankes »

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9197
    • View Profile
Looks like you can use a newer gcc with plugins?

Quoted from random google result: https://github.com/mxe/mxe/issues/3083
Quote
You might want to try using a more recent gcc. For example, you could add this to your settings.mk file:
Code: [Select]
MXE_PLUGIN_DIRS := plugins/gcc14

Latest seems to be 14.2.0: https://github.com/mxe/mxe/commit/a513b17f32f715b5295b37f96bc53acc7907ea71

Offline Delian

  • Commander
  • *****
  • Posts: 580
    • View Profile
Btw, what optimization flags was the MXE build with?