Skip to content

Implement stacktraces in Sys::Drop()/Error() #1741

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

VReaperV
Copy link
Contributor

Requires #1719

Adds stacktraces from <stacktrace> to Sys::Drop()/Error(), to help with debugging.

@illwieckz
Copy link
Member

Thanks a lot!

@VReaperV VReaperV force-pushed the stacktrace branch 2 times, most recently from 6d571de to 9528fb2 Compare August 11, 2025 16:02
@VReaperV VReaperV force-pushed the stacktrace branch 6 times, most recently from 13e2395 to f3c1a28 Compare August 11, 2025 17:32
@VReaperV VReaperV mentioned this pull request Aug 11, 2025
@VReaperV
Copy link
Contributor Author

error: alias declarations are a C++11 extension [-Werror,-Wc++11-extensions]

Seriously? WTF is this garbage?

@VReaperV
Copy link
Contributor Author

GCC failure is unrelated, and for some reason using -Wno-array-bounds doesn't stop it.

@VReaperV VReaperV force-pushed the stacktrace branch 3 times, most recently from aed5b79 to 409027e Compare August 13, 2025 12:26
@VReaperV
Copy link
Contributor Author

I give up trying to fix that piece of shit. Either we NUKE the unsafe vec3 functions or accept the CI not using USE_CPP23 altogether, I don't care which.

Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should fix that vector load thing anyway. It could theoretically cause a segfault if the vector is right against a page boundary.


std::string file = entry.source_file();

size_t pos = std::min(
Copy link
Member

@slipher slipher Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be interested in chopping the baths at build time rather instead of munging them at runtime. You can chop off the path names using the flags -fmacro-prefix-map for GCC and /d1trimfile for MSVC. So if you want to start from src you can put -fmacro-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}/src=. for GCC-like, and for MSVC you do it like this:

string(REPLACE "/" "\\" backslashed_dir ${CMAKE_CURRENT_SOURCE_DIR}/src)
add_compile_options("/d1trimfile:${backslashed_dir}")

The flags can be repeated so you could do it for both Daemon and Unvanquished roots.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying this with msvc, but it doesn't seem to work with gamemodule paths for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With GCC it did not work at all for me.

@VReaperV
Copy link
Contributor Author

Seems not a lot of packages contain libc++_backtrace. Trixie appears to have it in some of the packages. I might check at a later point if it's viable to build it and/or libc++exp for external_deps, but for now I think it's fine to not do so. Probably also need to check which of the 2 libs is present, since it's preferred to use libc++exp, but it might not be available.

@slipher
Copy link
Member

slipher commented Aug 15, 2025

I might check at a later point if it's viable to build it and/or libc++exp for external_deps, but for now I think it's fine to not do so.

Something with a C++ interface is probably not viable for external_deps

@VReaperV VReaperV force-pushed the stacktrace branch 2 times, most recently from 0fc955c to cc1410d Compare August 17, 2025 08:52
@VReaperV VReaperV force-pushed the stacktrace branch 4 times, most recently from 2d2bca8 to 84796ac Compare August 17, 2025 13:25
@VReaperV
Copy link
Contributor Author

I might check at a later point if it's viable to build it and/or libc++exp for external_deps, but for now I think it's fine to not do so.

Something with a C++ interface is probably not viable for external_deps

Yeah.

@VReaperV VReaperV force-pushed the stacktrace branch 5 times, most recently from ee55d7c to 91acbfd Compare August 17, 2025 18:30
@VReaperV
Copy link
Contributor Author

MinGW is spewing garbage:

/home/vsts/work/1/s/src/common/StackTrace.h:47:1: note: ‘std::stacktrace’ is defined in header ‘’; did you forget to ‘#include ’?
46 | #include
+++ |+#include

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants