Skip to content

Commit

Permalink
Remove the obsolete BlockByRefStruct flag from LLVM IR
Browse files Browse the repository at this point in the history
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.

rdar://problem/44304813

Differential Revision: https://reviews.llvm.org/D67453

llvm-svn: 372272
  • Loading branch information
adrian-prantl committed Sep 18, 2019
1 parent 69a9235 commit 0779dff
Show file tree
Hide file tree
Showing 12 changed files with 12 additions and 78 deletions.
2 changes: 1 addition & 1 deletion llvm/bindings/go/llvm/dibuilder.go
Expand Up @@ -44,7 +44,7 @@ const (
FlagProtected
FlagFwdDecl
FlagAppleBlock
FlagBlockByrefStruct
FlagReserved
FlagVirtual
FlagArtificial
FlagExplicit
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm-c/DebugInfo.h
Expand Up @@ -32,7 +32,7 @@ typedef enum {
LLVMDIFlagPublic = 3,
LLVMDIFlagFwdDecl = 1 << 2,
LLVMDIFlagAppleBlock = 1 << 3,
LLVMDIFlagBlockByrefStruct = 1 << 4,
LLVMDIFlagReservedBit4 = 1 << 4,
LLVMDIFlagVirtual = 1 << 5,
LLVMDIFlagArtificial = 1 << 6,
LLVMDIFlagExplicit = 1 << 7,
Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/DebugInfoFlags.def
Expand Up @@ -31,7 +31,8 @@ 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)
Expand Down
1 change: 0 additions & 1 deletion llvm/include/llvm/IR/DebugInfoMetadata.h
Expand Up @@ -650,7 +650,6 @@ class DIType : public DIScope {
}
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; }
Expand Down
8 changes: 0 additions & 8 deletions llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
Expand Up @@ -1165,16 +1165,8 @@ void DwarfCompileUnit::addGlobalTypeUnitType(const DIType *Ty,
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
Expand Down
48 changes: 1 addition & 47 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
Expand Up @@ -198,54 +198,8 @@ bool DebugLocDwarfExpression::isFrameRegister(const TargetRegisterInfo &TRI,
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.
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
Expand Up @@ -216,7 +216,6 @@ class DbgVariable : public DbgEntity {
return !FrameIndexExprs.empty();
}

bool isBlockByrefVariable() const;
const DIType *getType() const;

static bool classof(const DbgEntity *N) {
Expand Down
9 changes: 0 additions & 9 deletions llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
Expand Up @@ -216,15 +216,6 @@ class DwarfUnit : public DIEUnit {
/// 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
Expand Down
8 changes: 3 additions & 5 deletions llvm/lib/IR/Verifier.cpp
Expand Up @@ -982,6 +982,9 @@ void Verifier::visitDICompositeType(const DICompositeType &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();
Expand Down Expand Up @@ -4902,11 +4905,6 @@ void Verifier::visitDbgIntrinsic(StringRef Kind, DbgVariableIntrinsic &DII) {
// 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);
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/debug-info-blocks.ll
Expand Up @@ -155,7 +155,7 @@ define hidden void @foobar_func_block_invoke_0(i8* %.block_descriptor, %0* %load
!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)
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/DebugInfo/Generic/block-asan.ll
Expand Up @@ -70,7 +70,7 @@ attributes #3 = { nounwind }
!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)
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Verifier/blockbyref.ll
@@ -1,6 +1,6 @@
; 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() {
Expand All @@ -16,4 +16,4 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata)
!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)

0 comments on commit 0779dff

Please sign in to comment.