[clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

[Michael137]

Summary

Summary

This patch makes debug-info generation aware of the
[[clang::preferred_name]] attribute. The attribute tells clang
to print the annotated class template as some typedef (e.g., in
diagnostics).

When printing a typename for emission into DW_AT_name this patch now
uses the preferred_name (if available). This is behind an LLDB tuning
because by default we try to avoid diverging GCC and Clang typename
format (which is why the PrintingPolicy::UsePreferredNames has
previously been disabled by default in CGDebugInfo).

Motivation

This will reduce noise in type summaries when showing variable
types in LLDB. E.g.,:

(lldb) v
(std::vector<std::basic_string<char> >) vec = size=0 {}

becomes

(lldb) v
(std::vector<std::string>) vec = size=0 {}

Testing

• Added Clang test
• Adjusted LLDB API tests

[Michael137]

The alternative would be attaching the preferred name as a new attribute or an existing attribute like DW_AT_description.

[aprantl]

Nice! Does expr -- std::basic_string<char> s still work after this change? Not that anyone would want to type this over std::string ...

clang/test/CodeGen/debug-info-preferred-names.cpp

2

Is -debug-info-kind=limited needed here? Using it here is odd since LLDB doesn't really support it and favors =standalone instead.

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

61

Out of curiosity: How does this function work when building the testsuite with a compiler that isn't Clang?
I vaguely recall there was someone in the community running a bot that built the LLDB testsuite against GCC.

[Michael137]

clang/test/CodeGen/debug-info-preferred-names.cpp

2

Not needed, just leftover from different test. Will change

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

61

Good point, this should also check for the compiler really. I assume because GCC isn't at that version yet it just works on that GCC buildbot

Will change in a different patch since there's other tests with this condition

[Michael137]

In D143501#4113347, @aprantl wrote:

Nice! Does expr -- std::basic_string<char> s still work after this change? Not that anyone would want to type this over std::string ...

Yup that still works. We would still emit it as basic_string if that was typed out in the source and that's how LLDB would show it.

[Michael137]

• Reword commit
• Use different debuginfo kind in tests

[Michael137]

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

61

Addressed here: https://reviews.llvm.org/D143656

[dblaikie]

In D143501#4110302, @Michael137 wrote:

The alternative would be attaching the preferred name as a new attribute or an existing attribute like DW_AT_description.

I'd recommend a possible long-term solution would be simplified template names (so we don't have to worry about encoding this in the DW_AT_name anyway) and a "DW_AT_LLVM_preferred_name" or similar attribute on a type that refers to the typedef that is the preferred name. This would generalize further than only appearing in template names - the type of a variable inside a template would also gain the beneficial naming (eg: template<typename T> void f1(T t) { } - as it stands, the type of the variable t must be std::basic_string<char... - but if the DW_TAG_class_type for std::basic_string<char... had this preferred-name attribute on it, then a debugger could helpfully render the type by its preferred alias instead)

Alternatively, I suppose, the DWARF emission could just look at the preferred name and use that as the DW_AT_type in all cases anyway? Avoids needing a new attribute, etc, though would be a bit quirky in its own way.

Doing this ^ would also then fix the divergence introduced by changing the naming - and would ensure simple template names still match up with names rebuilt from the DW_TAG_template_type_parameters.

[Michael137]

I'd recommend a possible long-term solution would be simplified template names (so we don't have to worry about encoding this in the DW_AT_name anyway) and a "DW_AT_LLVM_preferred_name" or similar attribute on a type that refers to the typedef that is the preferred name. This would generalize further than only appearing in template names - the type of a variable inside a template would also gain the beneficial naming (eg: template<typename T> void f1(T t) { } - as it stands, the type of the variable t must be std::basic_string<char... - but if the DW_TAG_class_type for std::basic_string<char... had this preferred-name attribute on it, then a debugger could helpfully render the type by its preferred alias instead)

That could be a neat solution. I probably asked this before, but what's the timeline with switching it on by default (if such plan is in the works at all)?

Alternatively, I suppose, the DWARF emission could just look at the preferred name and use that as the DW_AT_type in all cases anyway? Avoids needing a new attribute, etc, though would be a bit quirky in its own way.

This is how I first thought of implementing it, but ran into some circular dependency quirks and started working on this patch instead. Could take a second stab at doing so if that's the preference. Would be nice to not have this behind a tuning

[dblaikie]

In D143501#4116200, @Michael137 wrote:

I'd recommend a possible long-term solution would be simplified template names (so we don't have to worry about encoding this in the DW_AT_name anyway) and a "DW_AT_LLVM_preferred_name" or similar attribute on a type that refers to the typedef that is the preferred name. This would generalize further than only appearing in template names - the type of a variable inside a template would also gain the beneficial naming (eg: template<typename T> void f1(T t) { } - as it stands, the type of the variable t must be std::basic_string<char... - but if the DW_TAG_class_type for std::basic_string<char... had this preferred-name attribute on it, then a debugger could helpfully render the type by its preferred alias instead)

That could be a neat solution. I probably asked this before, but what's the timeline with switching it on by default (if such plan is in the works at all)?

I haven't especially planned that - though given it's been picked up by Fuschia and Chromium (at least in some build modes), and we got gdb and lldb mostly fixed, maybe it's worth considering. Defaulting on for lldb might be easier than for gdb (gdb has some outstanding index bugs with it). But generally I figure you Apple folks tend to be the ones who have more investment in what the lldb tuning should cover, or not?

If you folks want to try turning it on & we could see about turning it on by default for lldb tuning?

Alternatively, I suppose, the DWARF emission could just look at the preferred name and use that as the DW_AT_type in all cases anyway? Avoids needing a new attribute, etc, though would be a bit quirky in its own way.

This is how I first thought of implementing it, but ran into some circular dependency quirks and started working on this patch instead. Could take a second stab at doing so if that's the preference. Would be nice to not have this behind a tuning

yeah, happy to help with pointers, etc.

I think /maybe/ we had some design principle that DWARF features wouldn't be solely controlled by debugger tuning, the tuning only indicates defaults but tehy can be controlled by flags? Not sure if I'm remembering that quite right, though - maybe @probinson remembers more of that?

[dblaikie]

I think /maybe/ we had some design principle that DWARF features wouldn't be solely controlled by debugger tuning, the tuning only indicates defaults but tehy can be controlled by flags? Not sure if I'm remembering that quite right, though - maybe @probinson remembers more of that?

I guess maybe not - at least there's a handful of debugger tuning checks in similar code around here.

[aprantl]

Alternatively, I suppose, the DWARF emission could just look at the preferred name and use that as the DW_AT_type in all cases anyway? Avoids needing a new attribute, etc, though would be a bit quirky in its own way.

Am I understanding you correctly that you suggest we add a new preferred name attribute to DICompositeType and then swap out all uses of those types with references to a typedef using the preferred name and pointing to the unmodified type declaration in AsmPrinter?
Why wouldn't we just implement this directly in CGDebugInfo and emit a DIDerivedType(name: "preferred", type: <original type>) and replace all uses of <original type> there? Then we wouldn't have to modify the IR format at all?

clang/lib/CodeGen/CGDebugInfo.h

private:
   StringRef internString(StringRef A, StringRef B = StringRef()) {
     char *Data = DebugInfoNames.Allocate<char>(A.size() + B.size());
     if (!A.empty())
       std::memcpy(Data, A.data(), A.size());
     if (!B.empty())
       std::memcpy(Data + A.size(), B.data(), B.size());
     return StringRef(Data, A.size() + B.size());
   }
+
+  /// Returns the QualType of the typedef that the PreferredNameAttr
+  /// of 'orig' refers to, if any such attribute exists. Returns 'orig'
+  /// otherwise.
+  QualType maybeGetPreferredNameType(QualType orig) const;
 };
 
 /// A scoped helper to set the current debug location to the specified
 /// location or preferred location of the specified Expr.
 class ApplyDebugLocation {
 private:
   void init(SourceLocation TemporaryLocation, bool DefaultToEmpty = false);
   ApplyDebugLocation(CodeGenFunction &CGF, bool DefaultToEmpty,

clang/lib/CodeGen/CGDebugInfo.cpp

PrintingPolicy CGDebugInfo::getPrintingPolicy() const {
   } else {
     // For DWARF, printing rules are underspecified.
     // SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
     PP.SplitTemplateClosers = true;
   }
 
   PP.SuppressInlineNamespace = false;
   PP.PrintCanonicalTypes = true;
-  PP.UsePreferredNames = false;
+  PP.UsePreferredNames = CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
   PP.AlwaysIncludeTypeForTemplateArgument = true;
   PP.UseEnumerators = false;
 
   // Apply -fdebug-prefix-map.
   PP.Callbacks = &PrintCB;
   return PP;
 }
 
for (unsigned i = 0, e = Args.Args.size(); i != e; ++i) {
     const TemplateArgument &TA = Args.Args[i];
     StringRef Name;
     const bool defaultParameter = TA.getIsDefaulted();
     if (Args.TList)
       Name = Args.TList->getParam(i)->getName();
 
     switch (TA.getKind()) {
     case TemplateArgument::Type: {
-      llvm::DIType *TTy = getOrCreateType(TA.getAsType(), Unit);
+      llvm::DIType *TTy =
+          getOrCreateType(maybeGetPreferredNameType(TA.getAsType()), Unit);
       TemplateParams.push_back(DBuilder.createTemplateTypeParameter(
           TheCU, Name, TTy, defaultParameter));
 
     } break;
     case TemplateArgument::Integral: {
       llvm::DIType *TTy = getOrCreateType(TA.getIntegralType(), Unit);
       TemplateParams.push_back(DBuilder.createTemplateValueParameter(
           TheCU, Name, TTy, defaultParameter,
           llvm::ConstantInt::get(CGM.getLLVMContext(), TA.getAsIntegral())));
     } break;
     case TemplateArgument::Declaration: {
       const ValueDecl *D = TA.getAsDecl();
-      QualType T = TA.getParamTypeForDecl().getDesugaredType(CGM.getContext());
+      QualType T = maybeGetPreferredNameType(
+          TA.getParamTypeForDecl().getDesugaredType(CGM.getContext()));
       llvm::DIType *TTy = getOrCreateType(T, Unit);
       llvm::Constant *V = nullptr;
       // Skip retrieve the value if that template parameter has cuda device
       // attribute, i.e. that value is not available at the host side.
       if (!CGM.getLangOpts().CUDA || CGM.getLangOpts().CUDAIsDevice ||
           !D->hasAttr<CUDADeviceAttr>()) {
         const CXXMethodDecl *MD;
         // Variable pointer template parameters have a value that is the address
llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,
   llvm::DIFile *Unit = nullptr;
   if (!Unwritten)
     Unit = getOrCreateFile(VD->getLocation());
   llvm::DIType *Ty;
   uint64_t XOffset = 0;
   if (VD->hasAttr<BlocksAttr>())
     Ty = EmitTypeForVarWithBlocksAttr(VD, &XOffset).WrappedType;
   else
-    Ty = getOrCreateType(VD->getType(), Unit);
+    Ty = getOrCreateType(maybeGetPreferredNameType(VD->getType()), Unit);
 
   // If there is no debug info for this type then do not emit debug info
   // for this variable.
   if (!Ty)
     return nullptr;
 
   // Get location information.
   unsigned Line = 0;
bool SupportsDWARFv4Ext =
       (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
        CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
   if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
     return llvm::DINode::FlagZero;
 
   return llvm::DINode::FlagAllCallsDescribed;
 }
+
+QualType CGDebugInfo::maybeGetPreferredNameType(QualType orig) const {
+  if (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB)
+    if (auto *RecordDecl = orig->getAsCXXRecordDecl())
+      if (auto *Attr = RecordDecl->getAttr<PreferredNameAttr>())
+        return Attr->getTypedefType();
+
+  return orig;
+}

clang/test/CodeGen/debug-info-preferred-names.cpp

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone %s -o - -debugger-tuning=lldb | FileCheck --check-prefixes=COMMON,LLDB %s
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone %s -o - -debugger-tuning=gdb | FileCheck --check-prefixes=COMMON,GDB %s
+
+template<typename T>
+class Qux {};
+
+template<typename T>
+struct Foo;
+
+template<typename T>
+using Bar = Foo<T>;
+
+template<typename T>
+struct [[clang::preferred_name(Bar<T>)]] Foo {};
+
+int main() {
+    /* Trivial cases */
+
+    Bar<int> b;
+// COMMON: !DIDerivedType(tag: DW_TAG_typedef, name: "Bar<int>"
+
+    Foo<int> f1;
+// COMMON: !DICompositeType(tag: DW_TAG_structure_type, name: "Foo<int>"
+
+    /* Alias template case */
+
+    Bar<Foo<int>> f2;
+// GDB:    !DIDerivedType(tag: DW_TAG_typedef, name: "Bar<Foo<int> >"
+// LLDB:   !DIDerivedType(tag: DW_TAG_typedef, name: "Bar<Bar<int> >"
+
+    /* Nested cases */
+
+    Foo<Foo<int>> f3; 
+// GDB:    !DICompositeType(tag: DW_TAG_structure_type, name: "Foo<Foo<int> >"
+// LLDB:   !DICompositeType(tag: DW_TAG_structure_type, name: "Foo<Bar<int> >"
+    
+    Qux<Foo<int>> f4;
+// GDB:    !DICompositeType(tag: DW_TAG_class_type, name: "Qux<Foo<int> >"
+// LLDB:   !DICompositeType(tag: DW_TAG_class_type, name: "Qux<Bar<int> >"
+
+    return 0;
+}

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

def test_shared_ptr_variables(self):
         valobj = self.expect_var_path(
             "sp_int_ref_ref",
             type="std::shared_ptr<int> &&",
             children=[ValueCheck(name="__ptr_")],
         )
         self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
         self.assertNotEqual(valobj.child[0].unsigned, 0)
 
         if self.expectedCompilerVersion(['>', '16.0']):
-            string_type = "std::basic_string<char>"
+            string_type = "std::string"
         else:
-            string_type = "std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
+            string_type = "std::basic_string<char, std::char_traits<char>, std::allocator<char> > "
 
         valobj = self.expect_var_path(
             "sp_str",
-            type="std::shared_ptr<" + string_type + " >",
+            type="std::shared_ptr<" + string_type + ">",
             children=[ValueCheck(name="__ptr_", summary='"hello"')],
         )
         self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
 
         valobj = self.expect_var_path("sp_user", type="std::shared_ptr<User>")
         self.assertRegex(
             valobj.summary,
             "^std(::__[^:]*)?::shared_ptr<User>::element_type @ 0x0*[1-9a-f][0-9a-f]+( strong=1)? weak=1",

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

def make_expected_type(self, pointee_type: str, qualifiers: str = "") -> str:
 
         if self.expectedCompilerVersion(['>', '16.0']):
             return f'std::unique_ptr<{pointee_type}>{qualifiers}'
         else:
             return f'std::unique_ptr<{pointee_type}, std::default_delete<{pointee_type}> >{qualifiers}'
 
     def make_expected_basic_string_ptr(self) -> str:
         if self.expectedCompilerVersion(['>', '16.0']):
-            return f'std::unique_ptr<std::basic_string<char> >'
+            return f'std::unique_ptr<std::string>'
         else:
             return 'std::unique_ptr<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, ' \
                    'std::default_delete<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >'
 
     @add_test_categories(["libc++"])
     def test_unique_ptr_variables(self):
         """Test `frame variable` output for `std::unique_ptr` types."""
         self.build()