[BUG] Legacy of Rust (id1.wad) flamethrower crash with newer linux gcc libs

[cncz42]

GZDoom version

GZDoom 4.12.2

Which game are you running with GZDoom?

Doom 2

What Operating System are you using?

Linux x86_64

Please describe your specific OS version

Arch Linux Latest, Pop OS 22.04

Relevant hardware info

AMD ryzen 9 3900x, AMD Radeon 7900xt (mesa)

Have you checked that no other similar issue already exists?

• I have searched and not found similar issues.

A clear and concise description of what the bug is.

The latest doom release included an mbf21 compatible level set with custom weapons, one of which replaces the plasma rifle and crashes gzdoom while firing, this issue doesn't occur on other mbf21 ports like dsda-doom. I tested this on both an AUR build (using gcc-libs 14.1.1 on my system) and the flatpak release (uses gcc-libs 13.2.0) with default settings. I also tested the debian package on a pop os laptop with libs version 11 and no such error appeared, while the error did occur on the flatpak version running the same 13.2.0 gcc libs on that same pop os laptop.

Steps to reproduce the behaviour.

Explain how to reproduce

1. Load up gzdoom linux on a new-ish distro or flatpak on default settings with the doom 2 iwad and id1.wad from the new doom release
2. run give all and switch to weapon 6
3. shoot it

Your configuration

<script src="https://gist.github.com/cncz42/1390ce8814a893e967a393865b58843d.js"></script>

Provide a Log

/usr/include/c++/13.2.0/bits/stl_algo.h:3669: constexpr const _Tp& std::clamp(const _Tp&, const _Tp&, const _Tp&) [with _Tp = double]: Assertion '!(__hi < __lo)' failed.

[RicardoLuis0]

that is a known issue with gcc's clamp (happened multiple times in multiple different places in the engine before), but in the first place, why are assertions enabled? is the flatpak using debug builds? (the dev team does not build the package on flathub, so while this is a valid issue for gzdoom, this is also something you'd need to take up on them about their build settings, because assertions just plain shouldn't be triggering on any builds players (and not testers) are using since debug builds massively decrease performance - by multiple orders of magnitude in some extreme scenarios)

[cncz42]

Oh that makes more sense than my gcc libs theory lol. Looks like the flathub package is compiled with -DCMAKE_BUILD_TYPE=RelWithDebInfo for some reason (explains why I got 20% less fps there), and archlinux makepkg appends -Wp,-D_GLIBCXX_ASSERTIONS to non-debug builds by default. I'll go open an issue with the flathub maintainers and see if there's a way for the aur maintainer to override that flag.

[Eonfge]

@RicardoLuis0 Flathub maintainer here.

Background

Ideally, the packages on Flathub are build for a release, having the debug info stripped out afterwards. This allows you to do:

flatpak install flathub org.zdoom.GZDoom.Debug

This will install GZDoom with debug symbols, so you can run GZDoom with debug information easily:

flatpak run --command=gdb --devel org.zdoom.GZDoom

What now

Should I build without release info? I'll build it without release info. This would make debugging harder but if you don't normally use it, it wouldn't matter.

[Eonfge]

@cncz42 Can you test again, and see if the bug still happens?

I'm also interested to know if you now have 20% more FPS. It's impressive to know that -O2 and -O3 make such a difference in performance.

[cncz42]

oh idk how to get devel builds of flathub packages, I did do some debugging around around half an hour ago tho and discovered that it's the -Wp,-D_GLIBCXX_ASSERTIONS getting forced on in cxx args that's causing it and not the release type, there's a comment on the flathub PR with more info

[cncz42]

oh idk how to get devel builds of flathub packages, I did do some debugging around around half an hour ago tho and discovered that it's the -D_GLIBCXX_ASSERTIONS getting forced on in cxx args that's causing it and not the release type, there's a comment on the flathub PR with more info

Should be able to test the crash whenever those cxx changes are done as well as compare a relwithdebinfo build's performance vs a release build (decent chance the performance loss is just from flatpak sandbox overhead) tomorrow, it's like 4am in my timezone rn I gotta get some sleep.

[madame-rachelle]

I'm also interested to know if you now have 20% more FPS. It's impressive to know that -O2 and -O3 make such a difference in performance.

This seems to be greatly exaggerated by the OP. :) I'd actually be extremely surprised if those optimization levels make that much of a difference. The only real difference (at least practically) between RelWithDebInfo and Release should be the generation of debug symbols which, as you already said, help with debugging.

Also apparently there are ways to strip the original binary while keeping the debugging info separate, similar to how Windows does .pdb files: https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html - I haven't had a chance to try this out myself yet, though.

[Eonfge]

oh idk how to get devel builds of flathub packages,

@cncz42 Based on the comment of @RicardoLuis0 I've already pushed a change in the build pipeline to production. You can do flatpak update and it should give you the latest GZDoom, with this problem fixed.

[Eonfge]

I haven't had a chance to try this out myself yet, though.

@madame-rachelle for now I've changed the release package to be in line with your own packages. If you want to try such a thing with flatpak, I can help you set-up a local test scenario.

[cncz42]

@Eonfge the crash still occurs on the latest flathub build, my initial guess that it was due to the build type was wrong, apologies. Commenter in flathub/shared-modules#308 mentioned that flatpak-builder forces -g -Wp,-D_GLIBCXX_ASSERTIONS, which does cause the crash to occur when testing a fresh build and needs to be disabled, you can revert to the relwithdebinfo build if it's still necessary for the package since it won't affect performance.

[Eonfge]

@cncz42 I'll keep the change as is, since it's more in line with upstream; If the ZDoom team does release builds with Release then so will I.

I'll look into the additional build parameters later. If you got time to spare, a pull request here would really help:
https://github.com/flathub/shared-modules

[Eonfge]

@cncz42 Much appreciated. I didn't have any time over the weekend so your assistance really helps.

[Eonfge]

I do feel a bit of reluctance though. Reading up on the GLIBCXX_ASSERTIONS param, it normally signals an error with the application code. @RicardoLuis0 are you sure that this is a compiler bug, and not an application bug? Some for you @madame-rachelle, you sure this compiler-change is desirable?

Discussion:

• https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/TMDJMX2OYE7Z2WHKXMOMTOBDHJKZJ3NV/

Bug example:

• GLIBCXX_ASSERTIONS reveals problem in string match subcommand fish-shell/fish-shell#5479

If you agree with this change, I'll distribute it to all ~100.000 GZDoom users on Flathub.

[mjr4077au]

If clamp() is being used under GCC and it can't be guaranteed that the min input is truly less than the max input, it's probably better to just do min(max()) instead. We've done that in a 1-2 places in Raze.

[cncz42]

I do feel a bit of reluctance though. Reading up on the GLIBCXX_ASSERTIONS param, it normally signals an error with the application code. @RicardoLuis0 are you sure that this is a compiler bug, and not an application bug? Some for you @madame-rachelle, you sure this compiler-change is desirable?

Discussion:

* https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/TMDJMX2OYE7Z2WHKXMOMTOBDHJKZJ3NV/

Bug example:

* [GLIBCXX_ASSERTIONS reveals problem in string match subcommand fish-shell/fish-shell#5479](https://github.com/fish-shell/fish-shell/issues/5479)

If you agree with this change, I'll distribute it to all ~100.000 GZDoom users on Flathub.

#2659 fixes the application bug that was causing this even if you compile with assertions but gzdoom takes a while to ship new versions so the compiler change is a decent workaround for now I guess, I don't know if compiling with assertions causes the slowdown that @RicardoLuis0 warned of with debug builds though.

[Eonfge]

In that case... I would be in favour of patching this bug downstream: We can take the patch, commit and apply it to GZDoom, and ship it like that.

We will then to a Release build (in accordance with the GZDoom's own releases) but we keep the GLIBCXX_ASSERTIONS checks so that future bugs can also be found, without impacting the security of the GZDoom Flathub edition.

[RicardoLuis0]

I do feel a bit of reluctance though. Reading up on the GLIBCXX_ASSERTIONS param, it normally signals an error with the application code. @RicardoLuis0 are you sure that this is a compiler bug, and not an application bug? Some for you @madame-rachelle, you sure this compiler-change is desirable?

Discussion:

• https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/TMDJMX2OYE7Z2WHKXMOMTOBDHJKZJ3NV/

Bug example:

• GLIBCXX_ASSERTIONS reveals problem in string match subcommand fish-shell/fish-shell#5479

If you agree with this change, I'll distribute it to all ~100.000 GZDoom users on Flathub.

it's not a compiler bug, but a gcc issue: most (if not all) devs are on windows, and this kind of assertion is only on gcc's standard library, not on MSVC's, and thus, it keeps popping up without warning - as for what i said about showdowns, that was about debug builds (which i incorrectly assumed the flatpak was using), not builds with assertions, i don't know what kinds of slowdowns enabling assertions on release builds lead to, and whether they would be significant at all

[Eonfge]

I've taken the more conservative approach for now: The flathub version will now include a patch for this specific issue. Thus, the Flathub package can have the additional security checks.

You can still choose to overrule my decision here, since you're ultimately the makers of GZDoom.

And I understand, most development is Windows first. Still, having GCC discover bugs where doubles and ints are mixed, is beneficial to all users.

[cncz42]

this issue can probably be closed now btw