Emit debug info for dynamic classes if they are imported from a DLL

[amccarth]

Summary

With -debug-info-kind=limited, we omit debug info for dynamic classes that live in other TUs. This reduces duplicate type information. When statically linked, the type information comes together. But if your binary has a class derived from a base in a DLL, the base class info is not available to the debugger.

The decision is made in shouldOmitDefinition (CGDebugInfo.cpp). Per a suggestion from rnk, I've tweaked the decision so that we do include definitions for classes marked as DLL imports. This should be a relatively small number of classes, so we don't pay a large price for duplication of the type info, yet it should cover most cases on Windows.

Essentially this makes debug info for DLLs independent, but we still assume that all TUs within the same DLL will be consistently built with (or without) debug info and the debugger will be able to search across the debug info within that scope to resolve any declarations into definitions, etc.

Tested manually on Windows. I'm working on a lit test, and I'll update this patch with it when it's finished.

[amccarth]

Added a test.

[amccarth]

Thank, dblaikie, for correcting my terminology.

[rnk]

lib/CodeGen/CGDebugInfo.cpp

1688–1694

David, since we're doing this check to work around a limitation of PDB-based debuggers, do you think we should guard check with EmitCodeView, so that we don't emit the full type if we're emitting DWARF?

After that, I'd add a comment like:

// Only emit complete debug info for a dynamic class when its vtable is emitted. However,
// Microsoft debuggers are unable to resolve type information across DLL boundaries,
// so skip this optimization if the class is marked dllimport.

[amccarth]

lib/CodeGen/CGDebugInfo.cpp

1688–1694

Comment added.

I don't think the EmitCodeView check is necessary because it seems unlikely the DLLImportAttr would be set with DWARF debug info, and, even if it is, I wouldn't expect it to do any harm. But I'll defer to dblaikie or other DWARF experts.

[amccarth]

Add comment as requested.

[rnk]

lgtm

[amccarth]

Closed by r278861

lib/CodeGen/CGDebugInfo.cpp

static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   if (!RD->isCompleteDefinitionRequired())
     return true;
 
   const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD);
 
   if (!CXXDecl)
     return false;
 
-  if (CXXDecl->hasDefinition() && CXXDecl->isDynamicClass())
+  // Only emit complete debug info for a dynamic class when its vtable is
+  // emitted.  However, Microsoft debuggers don't resolve type information
+  // across DLL boundaries, so skip this optimization if the class is marked
+  // dllimport.
+  if (CXXDecl->hasDefinition() && CXXDecl->isDynamicClass() &&
+      !CXXDecl->hasAttr<DLLImportAttr>())
     return true;
 
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
     Spec = SD->getSpecializationKind();
 
   if (Spec == TSK_ExplicitInstantiationDeclaration &&
       hasExplicitMemberDefinition(CXXDecl->method_begin(),
                                   CXXDecl->method_end()))

test/CodeGenCXX/debug-info-dllimport-base-class.cpp

+// RUN: %clang_cc1 -triple i386-pc-windows -emit-llvm -gcodeview -debug-info-kind=limited -fms-compatibility %s -x c++ -o - | FileCheck %s
+
+// Ensure we emit debug info for the full definition of base classes that will
+// be imported from a DLL.  Otherwise, the debugger wouldn't be able to show the
+// members.
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "ImportedBase",
+// CHECK-NOT:              DIFlagFwdDecl
+// CHECK-SAME:             ){{$}}
+
+struct __declspec(dllimport) ImportedBase {
+  ImportedBase();
+  virtual void Foo();
+};
+
+struct DerivedFromImported : public ImportedBase {};
+
+int main() {
+  DerivedFromImported d;
+}