Mark static member functions as static in CodeViewDebug

[amccarth]

Summary

To improve CodeView quality for static member functions, we need to make the
static explicit. In addition to a small change in LLVM's CodeViewDebug to
return the appropriate MethodKind, this requires a small change in Clang to
note the staticness in the debug info metadata.

[rnk]

This should have an IRGen test. You can probably extend clang/test/CodeGenCXX/debug-info-ms-abi.cpp with a static method.

clang/lib/CodeGen/CGDebugInfo.cpp

2907

Please revert the unrelated whitespace change

3218

ditto

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

327–329

I suppose this is where we need to pass down a bool about this being a static member function.

1560–1564

If we had a bool IsInstanceMethod or something here we could skip this drop_front() dance and that would fix our parameters.

1625

I'd do this check first, since static methods can't be virtual.

[amccarth]

Good tip on the lowering. Extra test coming momentarily.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1560–1564

Ah yes, this fixes the Type field for static methods as well. I'll extend the test to show that.

1625

And virtual methods can't be static, so I'm not sure the order really matters. But done because it's not worth a debate.

[amccarth]

Adds code-gen test. Corrects problem with first param of static method being
treated as the type of the this pointer. Addresses style suggestions.

[rnk]

Looks good! Make sure to run check-clang and check-llvm. The CGDebugInfo.cpp change might affect clang codegen tests that generate debug info for DWARF.

[amccarth]

In D37715#868973, @rnk wrote:

Looks good! Make sure to run check-clang and check-llvm. The CGDebugInfo.cpp change might affect clang codegen tests that generate debug info for DWARF.

So check-llvm is fine (except for one unrelated test that fails without my change). I've also used all the various combinations of check-*debuginfo*.

check-clang, however, gets hundreds of Python stack traces because:

TypeError: a bytes-like object is required, not 'str'

This sounded to me like a Python 2/3 problem, so I reset my environment to Python 2.7, re-ran CMake, and tried again. But I got the same set of Python errors. What's the trick here?

[rnk]

In D37715#869699, @amccarth wrote:

This sounded to me like a Python 2/3 problem, so I reset my environment to Python 2.7, re-ran CMake, and tried again. But I got the same set of Python errors. What's the trick here?

CMake discovers the python executable and caches it in CMakeCache.txt on first configuration, so changing the environment generally doesn't have an effect unless you re-configure with a clean CMakeCache.txt and CMakeFiles dir.

The quick way to get back to 2.7 is probably to edit CMakeCache.txt and change the PYTHON_EXECUTABLE variable to look something like this:

PYTHON_EXECUTABLE:FILEPATH=C:/Python27/python.exe

ninja check-clang will pick that up on the next run and use it to run the test suite.

Immediately issue aside, I guess it's worth setting up continuous testing with py3k. =/

clang/lib/CodeGen/CGDebugInfo.cpp

if (CGM.getTarget().getCXXABI().isItaniumFamily()) {
       // it knows the dynamic type of an object.
       ThisAdjustment = CGM.getCXXABI()
                            .getVirtualFunctionPrologueThisAdjustment(GD)
                            .getQuantity();
     }
     ContainingType = RecordTy;
   }
 
+  if (Method->isStatic())
+    Flags |= llvm::DINode::FlagStaticMember;
   if (Method->isImplicit())
     Flags |= llvm::DINode::FlagArtificial;
   Flags |= getAccessFlag(Method->getAccess(), Method->getParent());
   if (const auto *CXXC = dyn_cast<CXXConstructorDecl>(Method)) {
     if (CXXC->isExplicit())
       Flags |= llvm::DINode::FlagExplicit;
   } else if (const auto *CXXC = dyn_cast<CXXConversionDecl>(Method)) {
     if (CXXC->isExplicit())
if (DebugKind >= codegenoptions::LimitedDebugInfo) {
     if (const NamespaceDecl *NSDecl =
         dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))
       FDContext = getOrCreateNamespace(NSDecl);
     else if (const RecordDecl *RDecl =
              dyn_cast_or_null<RecordDecl>(FD->getDeclContext())) {
       llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
       FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
     }
+
     // Check if it is a noreturn-marked function
     if (FD->isNoReturn())
       Flags |= llvm::DINode::FlagNoReturn;
     // Collect template parameters.
     TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
 }
 
void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
   if (Name.startswith("\01"))
     Name = Name.substr(1);
 
   if (!HasDecl || D->isImplicit()) {
     Flags |= llvm::DINode::FlagArtificial;
     // Artificial functions should not silently reuse CurLoc.
     CurLoc = SourceLocation();
   }
+
   unsigned LineNo = getLineNumber(Loc);
   unsigned ScopeLine = getLineNumber(ScopeLoc);
 
   // FIXME: The function declaration we're constructing here is mostly reusing
   // declarations from CXXMethodDecl and not constructing new ones for arbitrary
   // FunctionDecls. When/if we fix this we can have FDContext be TheCU/null for
   // all subprograms instead of the actual context since subprogram definitions
   // are emitted as CU level entities by the backend.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

TypeIndex CodeViewDebug::getMemberFunctionType(const DISubprogram *SP,
   // with the MemberFuncIdRecord, which is keyed in as {SP, nullptr}.
   auto I = TypeIndices.find({SP, Class});
   if (I != TypeIndices.end())
     return I->second;
 
   // Make sure complete type info for the class is emitted *after* the member
   // function type, as the complete class type is likely to reference this
   // member function type.
   TypeLoweringScope S(*this);
   TypeIndex TI =
       lowerTypeMemberFunction(SP->getType(), Class, SP->getThisAdjustment());
   return recordTypeIndexForDINode(SP, TI, Class);
 }
 
 TypeIndex CodeViewDebug::recordTypeIndexForDINode(const DINode *Node,
                                                   TypeIndex TI,
                                                   const DIType *ClassTy) {
   auto InsertResult = TypeIndices.insert({{Node, ClassTy}, TI});
   (void)InsertResult;
TypeIndex CodeViewDebug::lowerTypeMemberFunction(const DISubroutineType *Ty,
 
   TypeIndex ReturnTypeIndex = TypeIndex::Void();
   ArrayRef<TypeIndex> ArgTypeIndices = None;
   if (!ReturnAndArgTypeIndices.empty()) {
     auto ReturnAndArgTypesRef = makeArrayRef(ReturnAndArgTypeIndices);
     ReturnTypeIndex = ReturnAndArgTypesRef.front();
     ArgTypeIndices = ReturnAndArgTypesRef.drop_front();
   }
   TypeIndex ThisTypeIndex = TypeIndex::Void();
   if (!ArgTypeIndices.empty()) {
     ThisTypeIndex = ArgTypeIndices.front();
     ArgTypeIndices = ArgTypeIndices.drop_front();
   }
 
   ArgListRecord ArgListRec(TypeRecordKind::ArgList, ArgTypeIndices);
   TypeIndex ArgListIndex = TypeTable.writeKnownType(ArgListRec);
 
   CallingConvention CC = dwarfCCToCodeView(Ty->getCC());
 
-  // TODO: Need to use the correct values for:
-  //       FunctionOptions
-  //       ThisPointerAdjustment.
+  // TODO: Need to use the correct values for FunctionOptions.
   MemberFunctionRecord MFR(ReturnTypeIndex, ClassType, ThisTypeIndex, CC,
                            FunctionOptions::None, ArgTypeIndices.size(),
                            ArgListIndex, ThisAdjustment);
   TypeIndex TI = TypeTable.writeKnownType(MFR);
 
   return TI;
 }
 
case dwarf::DW_VIRTUALITY_virtual:
     return Introduced ? MethodKind::IntroducingVirtual : MethodKind::Virtual;
   case dwarf::DW_VIRTUALITY_pure_virtual:
     return Introduced ? MethodKind::PureIntroducingVirtual
                       : MethodKind::PureVirtual;
   default:
     llvm_unreachable("unhandled virtuality case");
   }
 
-  // FIXME: Get Clang to mark DISubprogram as static and do something with it.
+  if (SP->getFlags() & DINode::FlagStaticMember)
+    return MethodKind::Static;
 
   return MethodKind::Vanilla;
 }
 
 static TypeRecordKind getRecordKind(const DICompositeType *Ty) {
   switch (Ty->getTag()) {
   case dwarf::DW_TAG_class_type:     return TypeRecordKind::Class;
   case dwarf::DW_TAG_structure_type: return TypeRecordKind::Struct;

llvm/test/DebugInfo/COFF/static-methods.ll

+; RUN: llc < %s -filetype=obj | llvm-readobj - -codeview | FileCheck %s
+
+; Check for the appropriate MethodKind below.
+
+; C++ source used to generate IR:
+; $ cat t.cpp
+; struct A {
+;   static void f();
+;   void g();
+; };
+; A *p = new A;
+; $ clang t.cpp -S -emit-llvm -g -gcodeview -o t.ll
+
+; CHECK:      OneMethod {
+; CHECK-NEXT:   TypeLeafKind: LF_ONEMETHOD (0x1511)
+; CHECK-NEXT:   AccessSpecifier: Public (0x3)
+; CHECK-NEXT:   MethodKind: Static (0x2)
+; CHECK-NEXT:   Type: void A::() ({{.*}})
+; CHECK-NEXT:   Name: f
+; CHECK-NEXT: }
+; CHECK-NEXT: OneMethod {
+; CHECK-NEXT:   TypeLeafKind: LF_ONEMETHOD (0x1511)
+; CHECK-NEXT:   AccessSpecifier: Public (0x3)
+; CHECK-NEXT:   Type: void A::() ({{.*}})
+; CHECK-NEXT:   Name: g
+; CHECK-NEXT: }
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-pc-windows-msvc19.0.24215"
+
+%struct.A = type { i8 }
+
+@"\01?p@@3PAUA@@A" = global %struct.A* null, align 4, !dbg !0
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_t.cpp, i8* null }]
+
+; Function Attrs: noinline
+define internal void @"\01??__Ep@@YAXXZ"() #0 !dbg !21 {
+entry:
+  %call = call i8* @"\01??2@YAPAXI@Z"(i32 1) #2, !dbg !22
+  %0 = bitcast i8* %call to %struct.A*, !dbg !22
+  store %struct.A* %0, %struct.A** @"\01?p@@3PAUA@@A", align 4, !dbg !22
+  ret void, !dbg !23
+}
+
+; Function Attrs: nobuiltin
+declare noalias i8* @"\01??2@YAPAXI@Z"(i32) #1
+
+; Function Attrs: noinline
+define internal void @_GLOBAL__sub_I_t.cpp() #0 !dbg !24 {
+entry:
+  call void @"\01??__Ep@@YAXXZ"(), !dbg !26
+  ret void
+}
+
+attributes #0 = { noinline "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nobuiltin "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { builtin }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!16, !17, !18, !19}
+!llvm.ident = !{!20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "p", linkageName: "\01?p@@3PAUA@@A", scope: !2, file: !3, line: 7, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 6.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "t.cpp", directory: "D:\5Csrc\5Ct", checksumkind: CSK_MD5, checksum: "56dde2d1d262be28d44b4315a2abb09c")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 32)
+!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "A", file: !3, line: 1, size: 8, elements: !8, identifier: ".?AUA@@")
+!8 = !{!9, !12}
+!9 = !DISubprogram(name: "f", linkageName: "\01?f@A@@SAXXZ", scope: !7, file: !3, line: 2, type: !10, isLocal: false, isDefinition: false, scopeLine: 2, flags: DIFlagPrototyped | DIFlagStaticMember, isOptimized: false)
+!10 = !DISubroutineType(types: !11)
+!11 = !{null}
+!12 = !DISubprogram(name: "g", linkageName: "\01?g@A@@QAEXXZ", scope: !7, file: !3, line: 4, type: !13, isLocal: false, isDefinition: false, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false)
+!13 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !14)
+!14 = !{null, !15}
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 32, flags: DIFlagArtificial | DIFlagObjectPointer)
+!16 = !{i32 1, !"NumRegisterParameters", i32 0}
+!17 = !{i32 2, !"CodeView", i32 1}
+!18 = !{i32 2, !"Debug Info Version", i32 3}
+!19 = !{i32 1, !"wchar_size", i32 2}
+!20 = !{!"clang version 6.0.0 "}
+!21 = distinct !DISubprogram(name: "??__Ep@@YAXXZ", scope: !3, file: !3, line: 7, type: !10, isLocal: true, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: false, unit: !2, variables: !4)
+!22 = !DILocation(line: 7, column: 8, scope: !21)
+!23 = !DILocation(line: 7, column: 12, scope: !21)
+!24 = distinct !DISubprogram(linkageName: "_GLOBAL__sub_I_t.cpp", scope: !3, file: !3, type: !25, isLocal: true, isDefinition: true, flags: DIFlagArtificial, isOptimized: false, unit: !2, variables: !4)
+!25 = !DISubroutineType(types: !4)
+!26 = !DILocation(line: 0, scope: !24)