Remove the obsolete BlockByRefStruct flag from LLVM IR

[aprantl]

Summary

DIFlagBlockByRefStruct is an unused DIFlag that originally was used by clang to express (Objective-)C block captures in debug info. For the last year Clang has been emitting complex DIExpressions to describe block captures instead, which makes all the code supporting this flag redundant.

This patch removes the flag and all supporting "dead" code, so we can reuse the bit for something else in the future.

Since this only affects debug info generated by Clang with the block extension this mostly affects Apple platforms and I don't have any bitcode compatibility concerns for removing this. The Verifier will reject debug info that uses the bit and thus degrade gracefully when LTO'ing older bitcode with a newer compiler.

[vsk]

Looks nice! One question: if DIFlagReservedBit4 is ever reused, will it break bitcode upgrades? Or is the idea that it won't ever be reused?

[aprantl]

My plan is to reuse the bit for an attribute that does not appear inside of DICompositeType, so we could leave the Verifier check in indefinitely. I should have said so though.

[vsk]

Makes sense, thanks!

llvm/include/llvm/IR/DebugInfoFlags.def

34–35

Maybe add a comment here explaining that ReservedBit4 can't be used within DICompositeType?

[aprantl]

Good point!

[aprantl]

Found one more stale decl.

llvm/bindings/go/llvm/dibuilder.go

const (
   DW_TAG_arg_variable    DwarfTag = 0x101
 )
 
 const (
   FlagPrivate = 1 << iota
   FlagProtected
   FlagFwdDecl
   FlagAppleBlock
-  FlagBlockByrefStruct
+  FlagReserved
   FlagVirtual
   FlagArtificial
   FlagExplicit
   FlagPrototyped
   FlagObjcClassComplete
   FlagObjectPointer
   FlagVector
   FlagStaticMember

llvm/docs/SourceLevelDebugging.rst

 
 This will inject synthetic DI to ``sample.ll`` run the ``pass-to-test``
 and then check for missing DI.
 
 Some other ways to run debugify are avaliable:
 
 .. code-block:: bash
 
-   # Same as the above example.
    $ opt -enable-debugify -pass-to-test sample.ll
 
    # Suppresses verbose debugify output.
    $ opt -enable-debugify -debugify-quiet -pass-to-test sample.ll
 
    # Prepend -debugify before and append -check-debugify -strip after
    # each pass on the pipeline (similar to -verify-each).
    $ opt -debugify-each -O2 sample.ll

llvm/include/llvm-c/DebugInfo.h

  */
 typedef enum {
   LLVMDIFlagZero = 0,
   LLVMDIFlagPrivate = 1,
   LLVMDIFlagProtected = 2,
   LLVMDIFlagPublic = 3,
   LLVMDIFlagFwdDecl = 1 << 2,
   LLVMDIFlagAppleBlock = 1 << 3,
-  LLVMDIFlagBlockByrefStruct = 1 << 4,
+  LLVMDIFlagReservedBit4 = 1 << 4,
   LLVMDIFlagVirtual = 1 << 5,
   LLVMDIFlagArtificial = 1 << 6,
   LLVMDIFlagExplicit = 1 << 7,
   LLVMDIFlagPrototyped = 1 << 8,
   LLVMDIFlagObjcClassComplete = 1 << 9,
   LLVMDIFlagObjectPointer = 1 << 10,
   LLVMDIFlagVector = 1 << 11,
   LLVMDIFlagStaticMember = 1 << 12,

llvm/include/llvm/IR/DebugInfoFlags.def

 
 HANDLE_DI_FLAG(0, Zero) // Use it as zero value.
                         // For example: void foo(DIFlags Flags = FlagZero).
 HANDLE_DI_FLAG(1, Private)
 HANDLE_DI_FLAG(2, Protected)
 HANDLE_DI_FLAG(3, Public)
 HANDLE_DI_FLAG((1 << 2), FwdDecl)
 HANDLE_DI_FLAG((1 << 3), AppleBlock)
-HANDLE_DI_FLAG((1 << 4), BlockByrefStruct)
+// Used to be BlockByRef, can be reused for anything except DICompositeType.
+HANDLE_DI_FLAG((1 << 4), ReservedBit4)
 HANDLE_DI_FLAG((1 << 5), Virtual)
 HANDLE_DI_FLAG((1 << 6), Artificial)
 HANDLE_DI_FLAG((1 << 7), Explicit)
 HANDLE_DI_FLAG((1 << 8), Prototyped)
 HANDLE_DI_FLAG((1 << 9), ObjcClassComplete)
 HANDLE_DI_FLAG((1 << 10), ObjectPointer)
 HANDLE_DI_FLAG((1 << 11), Vector)
 HANDLE_DI_FLAG((1 << 12), StaticMember)

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   bool isProtected() const {
     return (getFlags() & FlagAccessibility) == FlagProtected;
   }
   bool isPublic() const {
     return (getFlags() & FlagAccessibility) == FlagPublic;
   }
   bool isForwardDecl() const { return getFlags() & FlagFwdDecl; }
   bool isAppleBlockExtension() const { return getFlags() & FlagAppleBlock; }
-  bool isBlockByrefStruct() const { return getFlags() & FlagBlockByrefStruct; }
   bool isVirtual() const { return getFlags() & FlagVirtual; }
   bool isArtificial() const { return getFlags() & FlagArtificial; }
   bool isObjectPointer() const { return getFlags() & FlagObjectPointer; }
   bool isObjcClassComplete() const {
     return getFlags() & FlagObjcClassComplete;
   }
   bool isVector() const { return getFlags() & FlagVector; }
   bool isBitField() const { return getFlags() & FlagBitField; }

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

void DwarfCompileUnit::addGlobalTypeUnitType(const DIType *Ty,
   std::string FullName = getParentContextString(Context) + Ty->getName().str();
   // Insert, allowing the entry to remain as-is if it's already present
   // This way the CU-level type DIE is preferred over the "can't describe this
   // type as a unit offset because it's not really in the CU at all, it's only
   // in a type unit"
   GlobalTypes.insert(std::make_pair(std::move(FullName), &getUnitDie()));
 }
 
-/// addVariableAddress - Add DW_AT_location attribute for a
-/// DbgVariable based on provided MachineLocation.
 void DwarfCompileUnit::addVariableAddress(const DbgVariable &DV, DIE &Die,
                                           MachineLocation Location) {
-  // addBlockByrefAddress is obsolete and will be removed soon.
-  // The clang frontend always generates block byref variables with a
-  // complex expression that encodes exactly what addBlockByrefAddress
-  // would do.
-  assert((!DV.isBlockByrefVariable() || DV.hasComplexAddress()) &&
-         "block byref variable without a complex expression");
   if (DV.hasComplexAddress())
     addComplexAddress(DV, Die, dwarf::DW_AT_location, Location);
   else
     addAddress(Die, dwarf::DW_AT_location, Location);
 }
 
 /// Add an address attribute to a die based on the location provided.
 void DwarfCompileUnit::addAddress(DIE &Die, dwarf::Attribute Attribute,

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

bool hasComplexAddress() const {
     assert(ValueLoc.get() && "Expected DBG_VALUE, not MMI variable");
     assert((FrameIndexExprs.empty() ||
             (FrameIndexExprs.size() == 1 &&
              FrameIndexExprs[0].Expr->getNumElements())) &&
            "Invalid Expr for DBG_VALUE");
     return !FrameIndexExprs.empty();
   }
 
-  bool isBlockByrefVariable() const;
   const DIType *getType() const;
 
   static bool classof(const DbgEntity *N) {
     return N->getDbgEntityID() == DbgVariableKind;
   }
 };
 
 //===----------------------------------------------------------------------===//

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 }
 
 bool DebugLocDwarfExpression::isFrameRegister(const TargetRegisterInfo &TRI,
                                               unsigned MachineReg) {
   // This information is not available while emitting .debug_loc entries.
   return false;
 }
 
-bool DbgVariable::isBlockByrefVariable() const {
-  assert(getVariable() && "Invalid complex DbgVariable!");
-  return getVariable()->getType()->isBlockByrefStruct();
-}
-
 const DIType *DbgVariable::getType() const {
-  DIType *Ty = getVariable()->getType();
-  // FIXME: isBlockByrefVariable should be reformulated in terms of complex
-  // addresses instead.
-  if (Ty->isBlockByrefStruct()) {
-    /* Byref variables, in Blocks, are declared by the programmer as
-       "SomeType VarName;", but the compiler creates a
-       __Block_byref_x_VarName struct, and gives the variable VarName
-       either the struct, or a pointer to the struct, as its type.  This
-       is necessary for various behind-the-scenes things the compiler
-       needs to do with by-reference variables in blocks.
-
-       However, as far as the original *programmer* is concerned, the
-       variable should still have type 'SomeType', as originally declared.
-
-       The following function dives into the __Block_byref_x_VarName
-       struct to find the original type of the variable.  This will be
-       passed back to the code generating the type for the Debug
-       Information Entry for the variable 'VarName'.  'VarName' will then
-       have the original type 'SomeType' in its debug information.
-
-       The original type 'SomeType' will be the type of the field named
-       'VarName' inside the __Block_byref_x_VarName struct.
-
-       NOTE: In order for this to not completely fail on the debugger
-       side, the Debug Information Entry for the variable VarName needs to
-       have a DW_AT_location that tells the debugger how to unwind through
-       the pointers and __Block_byref_x_VarName struct to find the actual
-       value of the variable.  The function addBlockByrefType does this.  */
-    DIType *subType = Ty;
-    uint16_t tag = Ty->getTag();
-
-    if (tag == dwarf::DW_TAG_pointer_type)
-      subType = cast<DIDerivedType>(Ty)->getBaseType();
-
-    auto Elements = cast<DICompositeType>(subType)->getElements();
-    for (unsigned i = 0, N = Elements.size(); i < N; ++i) {
-      auto *DT = cast<DIDerivedType>(Elements[i]);
-      if (getName() == DT->getName())
-        return DT->getBaseType();
-    }
-  }
-  return Ty;
+  return getVariable()->getType();
 }
 
 /// Get .debug_loc entry for the instruction range starting at MI.
 static DbgValueLoc getDebugLocValue(const MachineInstr *MI) {
   const DIExpression *Expr = MI->getDebugExpression();
   assert(MI->getNumOperands() == 4);
   if (MI->getOperand(0).isReg()) {
     auto RegOp = MI->getOperand(0);

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h

public:
   void addLinkageName(DIE &Die, StringRef LinkageName);
 
   /// Add template parameters in buffer.
   void addTemplateParams(DIE &Buffer, DINodeArray TParams);
 
   /// Add thrown types.
   void addThrownTypes(DIE &Die, DINodeArray ThrownTypes);
 
-  // FIXME: Should be reformulated in terms of addComplexAddress.
-  /// Start with the address based on the location provided, and generate the
-  /// DWARF information necessary to find the actual Block variable (navigating
-  /// the Block struct) based on the starting location.  Add the DWARF
-  /// information to the die.  Obsolete, please use addComplexAddress instead.
-  void addBlockByrefAddress(const DbgVariable &DV, DIE &Die,
-                            dwarf::Attribute Attribute,
-                            const MachineLocation &Location);
-
   /// Add a new type attribute to the specified entity.
   ///
   /// This takes and attribute parameter because DW_AT_friend attributes are
   /// also type references.
   void addType(DIE &Entity, const DIType *Ty,
                dwarf::Attribute Attribute = dwarf::DW_AT_type);
 
   DIE *getOrCreateNameSpace(const DINamespace *NS);

llvm/lib/IR/Verifier.cpp

AssertDI(isType(N.getRawBaseType()), "invalid base type", &N,
            N.getRawBaseType());
 
   AssertDI(!N.getRawElements() || isa<MDTuple>(N.getRawElements()),
            "invalid composite elements", &N, N.getRawElements());
   AssertDI(isType(N.getRawVTableHolder()), "invalid vtable holder", &N,
            N.getRawVTableHolder());
   AssertDI(!hasConflictingReferenceFlags(N.getFlags()),
            "invalid reference flags", &N);
+  unsigned DIBlockByRefStruct = 1 << 4;
+  AssertDI((N.getFlags() & DIBlockByRefStruct) == 0,
+           "DIBlockByRefStruct on DICompositeType is no longer supported", &N);
 
   if (N.isVector()) {
     const DINodeArray Elements = N.getElements();
     AssertDI(Elements.size() == 1 &&
              Elements[0]->getTag() == dwarf::DW_TAG_subrange_type,
              "invalid vector, expected one element of type subrange", &N);
   }
 
void Verifier::visitDbgIntrinsic(StringRef Kind, DbgVariableIntrinsic &DII) {
   AssertDI(VarSP == LocSP, "mismatched subprogram between llvm.dbg." + Kind +
                                " variable and !dbg attachment",
            &DII, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
            Loc->getScope()->getSubprogram());
 
   // This check is redundant with one in visitLocalVariable().
   AssertDI(isType(Var->getRawType()), "invalid type ref", Var,
            Var->getRawType());
-  if (auto *Type = dyn_cast_or_null<DIType>(Var->getRawType()))
-    if (Type->isBlockByrefStruct())
-      AssertDI(DII.getExpression() && DII.getExpression()->getNumElements(),
-               "BlockByRef variable without complex expression", Var, &DII);
-
   verifyFnArgs(DII);
 }
 
 void Verifier::visitDbgLabelIntrinsic(StringRef Kind, DbgLabelInst &DLI) {
   AssertDI(isa<DILabel>(DLI.getRawLabel()),
          "invalid llvm.dbg." + Kind + " intrinsic variable", &DLI,
          DLI.getRawLabel());
 

llvm/test/CodeGen/ARM/debug-info-blocks.ll

 !42 = !DIDerivedType(tag: DW_TAG_member, name: "reserved", line: 307, size: 32, align: 32, file: !153, scope: !40, baseType: !43)
 !43 = !DIBasicType(tag: DW_TAG_base_type, name: "long unsigned int", size: 32, align: 32, encoding: DW_ATE_unsigned)
 !44 = !DIDerivedType(tag: DW_TAG_member, name: "Size", line: 307, size: 32, align: 32, offset: 32, file: !153, scope: !40, baseType: !43)
 !45 = !DIDerivedType(tag: DW_TAG_member, name: "CopyFuncPtr", line: 307, size: 32, align: 32, offset: 64, file: !153, scope: !40, baseType: !46)
 !46 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 32, align: 32, scope: !0, baseType: !32)
 !47 = !DIDerivedType(tag: DW_TAG_member, name: "DestroyFuncPtr", line: 307, size: 32, align: 32, offset: 96, file: !153, scope: !40, baseType: !46)
 !48 = !DIDerivedType(tag: DW_TAG_member, name: "mydata", line: 609, size: 32, align: 32, offset: 160, file: !152, scope: !24, baseType: !49)
 !49 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 32, scope: !0, baseType: !50)
-!50 = !DICompositeType(tag: DW_TAG_structure_type, size: 224, flags: DIFlagBlockByrefStruct, file: !152, scope: !24, elements: !51)
+!50 = !DICompositeType(tag: DW_TAG_structure_type, size: 224, file: !152, scope: !24, elements: !51)
 !51 = !{!52, !53, !54, !55, !56, !57, !58}
 !52 = !DIDerivedType(tag: DW_TAG_member, name: "__isa", size: 32, align: 32, file: !152, scope: !24, baseType: !32)
 !53 = !DIDerivedType(tag: DW_TAG_member, name: "__forwarding", size: 32, align: 32, offset: 32, file: !152, scope: !24, baseType: !32)
 !54 = !DIDerivedType(tag: DW_TAG_member, name: "__flags", size: 32, align: 32, offset: 64, file: !152, scope: !24, baseType: !34)
 !55 = !DIDerivedType(tag: DW_TAG_member, name: "__size", size: 32, align: 32, offset: 96, file: !152, scope: !24, baseType: !34)
 !56 = !DIDerivedType(tag: DW_TAG_member, name: "__copy_helper", size: 32, align: 32, offset: 128, file: !152, scope: !24, baseType: !32)
 !57 = !DIDerivedType(tag: DW_TAG_member, name: "__destroy_helper", size: 32, align: 32, offset: 160, file: !152, scope: !24, baseType: !32)
 !58 = !DIDerivedType(tag: DW_TAG_member, name: "mydata", size: 32, align: 32, offset: 192, file: !152, scope: !24, baseType: !59)

llvm/test/DebugInfo/Generic/block-asan.ll

 !5 = !DIFile(filename: "block.c", directory: "/tmp")
 !6 = !DISubroutineType(types: !7)
 !7 = !{null}
 !8 = !{i32 2, !"Dwarf Version", i32 2}
 !9 = !{i32 2, !"Debug Info Version", i32 3}
 !10 = !{i32 1, !"PIC Level", i32 2}
 !11 = !{!"clang version 3.6.0 (trunk 223120) (llvm/trunk 223119)"}
 !12 = !DILocalVariable(name: "x", line: 4, scope: !4, file: !5, type: !13)
-!13 = !DICompositeType(tag: DW_TAG_structure_type, size: 224, flags: DIFlagBlockByrefStruct, file: !1, scope: !5, elements: !14)
+!13 = !DICompositeType(tag: DW_TAG_structure_type, size: 224, file: !1, scope: !5, elements: !14)
 !14 = !{!15, !17, !18, !20, !21}
 !15 = !DIDerivedType(tag: DW_TAG_member, name: "__isa", size: 64, align: 64, file: !1, scope: !5, baseType: !16)
 !16 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, baseType: null)
 !17 = !DIDerivedType(tag: DW_TAG_member, name: "__forwarding", size: 64, align: 64, offset: 64, file: !1, scope: !5, baseType: !16)
 !18 = !DIDerivedType(tag: DW_TAG_member, name: "__flags", size: 32, align: 32, offset: 128, file: !1, scope: !5, baseType: !19)
 !19 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !20 = !DIDerivedType(tag: DW_TAG_member, name: "__size", size: 32, align: 32, offset: 160, file: !1, scope: !5, baseType: !19)
 !21 = !DIDerivedType(tag: DW_TAG_member, name: "x", size: 32, align: 32, offset: 192, file: !1, scope: !5, baseType: !19)
 !22 = !DIExpression(DW_OP_plus_uconst, 8, DW_OP_deref, DW_OP_plus_uconst, 24)
 !23 = !DILocation(line: 4, column: 15, scope: !4)
 !24 = !DILocation(line: 4, column: 3, scope: !4)
 !25 = !DILocation(line: 5, column: 3, scope: !4)
 !26 = !DILocation(line: 6, column: 1, scope: !4)

llvm/test/Verifier/blockbyref.ll

 ; RUN: llvm-as -disable-output <%s 2>&1| FileCheck %s
 
-; CHECK: BlockByRef variable without complex expression
+; CHECK: DIBlockByRefStruct on DICompositeType is no longer supported
 ; CHECK: warning: ignoring invalid debug info
 
 define void @foo() {
 entry:
   %s = alloca i32
   call void @llvm.dbg.declare(metadata i32* %s, metadata !2, metadata !DIExpression()), !dbg !DILocation(scope: !1)
   ret void
 }
 
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 
 !llvm.module.flags = !{!0}
 !0 = !{i32 2, !"Debug Info Version", i32 3}
 !1 = distinct !DISubprogram()
 !2 = !DILocalVariable(scope: !1, type: !3)
-!3 = !DICompositeType(tag: DW_TAG_structure_type, flags: DIFlagBlockByrefStruct)
+!3 = !DICompositeType(tag: DW_TAG_structure_type, flags: DIFlagReservedBit4)