Color stacktraces printed by the crash handler when in a TTY

[Calinou]

Follow-up to #44118.

This makes crash backtraces easier to read, which in turn improves the developer experience.

Preview

Windows 10 (MSVC)

Linux

PS: For testing purposes, you can call CRASH_NOW(); anywhere to make Godot crash.

[aaronfranke]

@Calinou Is this still desired? If so, it needs to be rebased on the latest master branch.

[akien-mga]

Needs rebasing. Should be fine to merge then if it works.

[Calinou]

Rebased and tested again on Linux, it works as expected.

Please test this on macOS before merging. The Windows portion of this PR won't work correctly until #44118 is merged (which still needs testers on Windows 7/8.1).

[Anutrix]

Since, #44118 is merged now, maybe we can get this merged after a rebase.

[Calinou]

Rebased and tested again, it works as expected on Linux and Windows (MSVC). See OP for updated screenshots.

@bruvzg If you have time, could you test and review the macOS side of this just in case? Thanks in advance 🙂

[bruvzg]

Seems to be working fine, with a few minor changes.

Note: at least on macOS, CRASH_NOW will generate SIGTRAP, so it will not be processed by Godot crash handler unless signal(SIGTRAP, handle_crash); is added.

[Calinou]

Note: at least on macOS, CRASH_NOW will generate SIGTRAP, so it will not be processed by Godot crash handler unless signal(SIGTRAP, handle_crash); is added.

Should this be changed in another PR, either in the crash handler or the CRASH_NOW() macro?

[bruvzg]

Probably a crash handler should be changed to handle SIGTRAP. CRASH_NOW is calling __builtin_trap, so I'm not sure if it can be changed, it's likely platform dependent how it's work.

[deralmas]

Just noticed this, nice work!

Is it really needed to explicitly add TTY checks and ANSI codes everywhere instead of having some sort of generic, optionally colored print system though? That might sound a bit excessive (and can definitely become such) but I'm pretty sure that there might be a clean and simple way of going along with it (hell, perhaps even a bunch of macros?). If it's done in more places that might be material for another PR but I can't stop wondering about this.

[Calinou]

Is it really needed to explicitly add TTY checks and ANSI codes everywhere instead of having some sort of generic, optionally colored print system though? That might sound a bit excessive (and can definitely become such) but I'm pretty sure that there might be a clean and simple way of going along with it (hell, perhaps even a bunch of macros?). If it's done in more places that might be material for another PR but I can't stop wondering about this.

print_line(), print_error(), print_line_rich() and all other print functions could have a check that strips ANSI escape codes from the printed string if the terminal isn't a TTY. This may have a performance impact when not running from a TTY and printing lots of text though.

[Calinou]

Rebased and tested again, it works as expected.

[fire]

@Calinou Would you like to rebase this? I'm interested.

[ghost]

I'm interested in this PR. Will it be merged after 4.6 release or is it still undetermined when this PR may be merged?

[bruvzg]

This can be changed to OS::get_singleton()->get_stderr_type() == OS::STD_HANDLE_CONSOLE and used on all platforms, including Windows.

[Calinou]

Rebased and tested again, it works as expected. Putting this back to draft as I'm working on reorganizing colors based on recent changes to the various crash handlers. I'll also need to figure out a way to factor out as much code as possible to avoid duplication.

Currently, it looks like this on Linux with the second commit and https://github.com/gimli-rs/addr2line installed:

Godot functions are highlighted (as we're usually interested in those in crash handlers), and function/parameter names are distinguished.