Change char type in debuginfo to DW_ATE_UTF

[arlosi]

Rust previously encoded the char type as DW_ATE_unsigned_char. The more appropriate encoding is DW_ATE_UTF.

Clang also uses the DW_ATE_UTF for char32_t in C++.

This fixes the display of the char type in the Windows debuggers. Without this change, the variable did not show in the locals window.

LLDB 13 is also able to display the char value, when before it failed with need to add support for DW_TAG_base_type 'char' encoded with DW_ATE = 0x8, bit_size = 32

r? @wesleywiser

[joshtriplett]

Looks reasonable to me. The test failure with gdb seems concerning, though; does gdb (or the tested version of gdb) not understand this?

[arlosi]

It seems that with this change, GDB only prints the numerical value of the character :/.

When I do set language c++ in gdb, then it prints correctly. So there's something in GDB's rust language mode that's causing char to not be interpreted as a character anymore with this change.

We could make the change MSVC specific, using the msvc_like_names variable, but I think we need to understand the situation a bit better first.

[joshtriplett]

@arlosi Sounds like we should fix gdb's rust mode to handle this better, like the C++ mode does.

[arlosi]

@joshtriplett I agree, I think we should fix gdb and continue with this change. I'll update the tests.

TLDR: only GDB is regressed, and other debuggers are either the same or improved by this change.

• LLDB 10 is broken both with and without the change.
• LLDB 13 works correctly with the change, and does not work without it.
• WinDbg, VS, CDB work correctly with the change and do not work without it.
• GDB is regressed by this change such that only the numeric value is printed.

LLDB 13 (without change)

(lldb) p c
error: need to add support for DW_TAG_base_type 'char' encoded with DW_ATE = 0x8, bit_size = 32
error: Couldn't materialize: couldn't get the value of variable asdf: Unable to determine byte size.

LLDB 13 (with change):

(lldb) p c
(char32_t) $0 = U+0x00000061 U'a'

LLDB 10 (with change)

(lldb) p c
error: need to add support for DW_TAG_base_type 'char' encoded with DW_ATE = 0x10, bit_size = 32
error: Couldn't materialize: couldn't get the value of variable asdf: Unable to determine byte size.

LLDB 10 (without change)

(lldb) p c
error: need to add support for DW_TAG_base_type 'char' encoded with DW_ATE = 0x8, bit_size = 32
error: Couldn't materialize: couldn't get the value of variable asdf: Unable to determine byte size.

Tested GDB versions 9, 11 with same result
GDB (without change)

(gdb) p c
$1 = 97 'a'

GDB (with change)

(gdb) p c
$1 = 97

[arlosi]

@joshtriplett do you know of someone who could look in to the GDB issue? I work on debuggers at work and can't look at GDB's source.

Even with the (hopefully temporary) GDB regression I feel like this is worth merging, since it fixes both recent LLDB and Windows debuggers (which were previously unusable) and GDB still prints the numeric representation.

[joshtriplett]

I'm not sure. cc @tromey perhaps?

[tromey]

I sent a gdb patch: https://sourceware.org/pipermail/gdb-patches/2021-October/182960.html

[arlosi]

@tromey thanks for getting this fixed in GDB!

@joshtriplett how long should we wait before merging this?

[wesleywiser]

Nominating for compiler team discussion. This PR changes debug info for char which fixes LLDB and WinDbg and causes variables of this type to be correctly displayed in the debugger. However, this PR also causes GDB to regress slightly in that it no longer will display the character value, just the numeric value. @tromey has fixed this in gdb but we're unsure how long we should wait before merging this PR.

[Mark-Simulacrum]

Has that patch been merged upstream? I think I would block on a merge at least, personally.

Can we have an interim situation where we change the type (as per this PR) only on Windows targets (which seem the primary audience for WinDbg, though not LLDB) and then enable it more broadly when the next GDB release occurs?

[tromey]

Has that patch been merged upstream? I think I would block on a merge at least, personally.

I'll be merging it soon. It will appear in gdb 11.2 as well. This should be coming reasonably soon; the last status report is here: https://sourceware.org/pipermail/gdb-patches/2021-November/183905.html

Can we have an interim situation where we change the type (as per this PR) only on Windows targets (which seem the primary audience for WinDbg, though not LLDB) and then enable it more broadly when the next GDB release occurs?

I think it's better to simply ship it. Distros that are concerned with this, and that don't want to wait for GDB 11.2, can just apply the upstream patch.

[wesleywiser]

We discussed this in yesterday's compiler team meeting and the consensus was that we should wait for the gdb 11.2 release to occur before merging. It sounds like that should be relatively soon but we will re-evaluate if that proves incorrect.

I'm going to mark this PR as blocked for now.

[mati865]

Gdb 11.2 has been released. Shouldn't this wait for it to popularise a bit though?

[tromey]

Gdb 11.2 has been released. Shouldn't this wait for it to popularise a bit though?

What's the lead time on a rustc patch making it into a release? It used to be multiple weeks, and if that's still true, then I personally think it's fine to land it now.

[wesleywiser]

If we merged right now, this would go into 1.60 which ships on April 7th.

[mati865]

FWIW Ubuntu 22.04 LTS with this version should be available by the end of April.

[mati865]

Beta promotion has happened already which means distribuitions with working GDB version should be already available when this hits stable. So should be good to go from POV.

[wesleywiser]

Thanks for the reminder @mati865! Since the compiler team has already decided the gdb 11.2 would be sufficient and we're now at the point where that release will be available in various distros by the time this reaches stable, I think we should merge.

Thanks to @arlosi for your patience and also to @tromey for contributing that gdb patch! It's very much appreciated.

@bors r+

[bors]

📌 Commit 7680fcfa6daf480eac4b5fd502268bd120d692f2 has been approved by wesleywiser

[wesleywiser]

@bors r+

[bors]

📌 Commit be454f0 has been approved by wesleywiser