[llvm] r294318 - Fix the bitcode upgrade for DIGlobalVariable in a DIImportedEntity context.

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 11:38:07 PST 2017


Yes I’ve been by this in practice with LTO!

— 
Mehdi


> On Feb 7, 2017, at 12:18 PM, Adrian Prantl via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> I hereby nominate this commit for merging into the 4.0 release branch. It fixes a bug in the bitcode upgrade for DIGlobalVariables that was introduced in r290153 when DIGlobalVariableExpression was added. The bug manifests as a an IR verifier error.
> 
> -- adrian
> 
> 
>> On Feb 7, 2017, at 9:35 AM, Adrian Prantl via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> Author: adrian
>> Date: Tue Feb  7 11:35:41 2017
>> New Revision: 294318
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=294318&view=rev
>> Log:
>> Fix the bitcode upgrade for DIGlobalVariable in a DIImportedEntity context.
>> 
>> The bitcode upgrade for DIGlobalVariable unconditionally wrapped
>> DIGlobalVariables in a DIGlobalVariableExpression. When a
>> DIGlobalVariable is referenced by a DIImportedEntity, however, this is
>> wrong. This patch fixes the bitcode upgrade by deferring the creation
>> of DIGlobalVariableExpressions until we know the context of the
>> DIGlobalVariable.
>> 
>> <rdar://problem/30134279>
>> 
>> Differential Revision: https://reviews.llvm.org/D29349
>> 
>> Modified:
>>   llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
>>   llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll
>>   llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll.bc
>>   llvm/trunk/test/Bitcode/dityperefs-3.8.ll
>> 
>> Modified: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp?rev=294318&r1=294317&r2=294318&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp (original)
>> +++ llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp Tue Feb  7 11:35:41 2017
>> @@ -451,6 +451,7 @@ class MetadataLoader::MetadataLoaderImpl
>> 
>>  bool StripTBAA = false;
>>  bool HasSeenOldLoopTags = false;
>> +  bool NeedUpgradeToDIGlobalVariableExpression = false;
>> 
>>  /// True if metadata is being parsed for a module being ThinLTO imported.
>>  bool IsImporting = false;
>> @@ -476,6 +477,45 @@ class MetadataLoader::MetadataLoaderImpl
>>    CUSubprograms.clear();
>>  }
>> 
>> +  /// Upgrade old-style bare DIGlobalVariables to DIGlobalVariableExpressions.
>> +  void upgradeCUVariables() {
>> +    if (!NeedUpgradeToDIGlobalVariableExpression)
>> +      return;
>> +
>> +    // Upgrade list of variables attached to the CUs.
>> +    if (NamedMDNode *CUNodes = TheModule.getNamedMetadata("llvm.dbg.cu"))
>> +      for (unsigned I = 0, E = CUNodes->getNumOperands(); I != E; ++I) {
>> +        auto *CU = cast<DICompileUnit>(CUNodes->getOperand(I));
>> +        if (auto *GVs = dyn_cast_or_null<MDTuple>(CU->getRawGlobalVariables()))
>> +          for (unsigned I = 0; I < GVs->getNumOperands(); I++)
>> +            if (auto *GV =
>> +                    dyn_cast_or_null<DIGlobalVariable>(GVs->getOperand(I))) {
>> +              auto *DGVE =
>> +                  DIGlobalVariableExpression::getDistinct(Context, GV, nullptr);
>> +              GVs->replaceOperandWith(I, DGVE);
>> +            }
>> +      }
>> +
>> +    // Upgrade variables attached to globals.
>> +    for (auto &GV : TheModule.globals()) {
>> +      SmallVector<MDNode *, 1> MDs, NewMDs;
>> +      GV.getMetadata(LLVMContext::MD_dbg, MDs);
>> +      GV.eraseMetadata(LLVMContext::MD_dbg);
>> +      for (auto *MD : MDs)
>> +        if (auto *DGV = dyn_cast_or_null<DIGlobalVariable>(MD)) {
>> +          auto *DGVE =
>> +              DIGlobalVariableExpression::getDistinct(Context, DGV, nullptr);
>> +          GV.addMetadata(LLVMContext::MD_dbg, *DGVE);
>> +        } else
>> +          GV.addMetadata(LLVMContext::MD_dbg, *MD);
>> +    }
>> +  }
>> +
>> +  void upgradeDebugInfo() {
>> +    upgradeCUSubprograms();
>> +    upgradeCUVariables();
>> +  }
>> +
>> public:
>>  MetadataLoaderImpl(BitstreamCursor &Stream, Module &TheModule,
>>                     BitcodeReaderValueList &ValueList,
>> @@ -729,7 +769,7 @@ Error MetadataLoader::MetadataLoaderImpl
>>      // Reading the named metadata created forward references and/or
>>      // placeholders, that we flush here.
>>      resolveForwardRefsAndPlaceholders(Placeholders);
>> -      upgradeCUSubprograms();
>> +      upgradeDebugInfo();
>>      // Return at the beginning of the block, since it is easy to skip it
>>      // entirely from there.
>>      Stream.ReadBlockEnd(); // Pop the abbrev block context.
>> @@ -753,7 +793,7 @@ Error MetadataLoader::MetadataLoaderImpl
>>      return error("Malformed block");
>>    case BitstreamEntry::EndBlock:
>>      resolveForwardRefsAndPlaceholders(Placeholders);
>> -      upgradeCUSubprograms();
>> +      upgradeDebugInfo();
>>      return Error::success();
>>    case BitstreamEntry::Record:
>>      // The interesting case.
>> @@ -1424,11 +1464,17 @@ Error MetadataLoader::MetadataLoaderImpl
>>           getDITypeRefOrNull(Record[6]), Record[7], Record[8],
>>           getMDOrNull(Record[10]), AlignInBits));
>> 
>> -      auto *DGVE = DIGlobalVariableExpression::getDistinct(Context, DGV, Expr);
>> -      MetadataList.assignValue(DGVE, NextMetadataNo);
>> -      NextMetadataNo++;
>> +      DIGlobalVariableExpression *DGVE = nullptr;
>> +      if (Attach || Expr)
>> +        DGVE = DIGlobalVariableExpression::getDistinct(Context, DGV, Expr);
>> +      else
>> +        NeedUpgradeToDIGlobalVariableExpression = true;
>>      if (Attach)
>>        Attach->addDebugInfo(DGVE);
>> +
>> +      auto *MDNode = Expr ? cast<Metadata>(DGVE) : cast<Metadata>(DGV);
>> +      MetadataList.assignValue(MDNode, NextMetadataNo);
>> +      NextMetadataNo++;
>>    } else
>>      return error("Invalid record");
>> 
>> 
>> Modified: llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll?rev=294318&r1=294317&r2=294318&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll (original)
>> +++ llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll Tue Feb  7 11:35:41 2017
>> @@ -7,12 +7,16 @@
>> ; CHECK: @h = common global i32 0, align 4, !dbg ![[H:[0-9]+]]
>> ; CHECK: ![[G]] = {{.*}}!DIGlobalVariableExpression(var: ![[GVAR:[0-9]+]], expr: ![[GEXPR:[0-9]+]])
>> ; CHECK: ![[GVAR]] = distinct !DIGlobalVariable(name: "g",
>> +; CHECK: DICompileUnit({{.*}}, imports: ![[IMPORTS:[0-9]+]]
>> ; CHECK: !DIGlobalVariableExpression(var: ![[CVAR:[0-9]+]], expr: ![[CEXPR:[0-9]+]])
>> ; CHECK: ![[CVAR]] = distinct !DIGlobalVariable(name: "c",
>> ; CHECK: ![[CEXPR]] = !DIExpression(DW_OP_constu, 23, DW_OP_stack_value)
>> -; CHECK: ![[H]] = {{.*}}!DIGlobalVariableExpression(var: ![[HVAR:[0-9]+]])
>> -; CHECK: ![[HVAR]] = distinct !DIGlobalVariable(name: "h",
>> +; CHECK: ![[HVAR:[0-9]+]] = distinct !DIGlobalVariable(name: "h",
>> +; CHECK: ![[IMPORTS]] = !{![[CIMPORT:[0-9]+]]}
>> +; CHECK: ![[CIMPORT]] = !DIImportedEntity({{.*}}entity: ![[HVAR]]
>> ; CHECK: ![[GEXPR]] = !DIExpression(DW_OP_plus, 1)
>> +; CHECK: ![[H]] = {{.*}}!DIGlobalVariableExpression(var: ![[HVAR]])
>> +
>> @g = common global i32 0, align 4, !dbg !0
>> @h = common global i32 0, align 4, !dbg !11
>> 
>> @@ -21,9 +25,9 @@
>> !llvm.ident = !{!9}
>> 
>> !0 = distinct !DIGlobalVariable(name: "g", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true, expr: !DIExpression(DW_OP_plus, 1))
>> -!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 4.0.0 (trunk 286129) (llvm/trunk 286128)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !4)
>> +!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 4.0.0 (trunk 286129) (llvm/trunk 286128)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, imports: !3)
>> !2 = !DIFile(filename: "a.c", directory: "/")
>> -!3 = !{}
>> +!3 = !{!12}
>> !4 = !{!0, !10, !11}
>> !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
>> !6 = !{i32 2, !"Dwarf Version", i32 4}
>> @@ -32,3 +36,4 @@
>> !9 = !{!"clang version 4.0.0 (trunk 286129) (llvm/trunk 286128)"}
>> !10 = distinct !DIGlobalVariable(name: "c", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true, expr: !DIExpression(DW_OP_constu, 23, DW_OP_stack_value))
>> !11 = distinct !DIGlobalVariable(name: "h", scope: !1, file: !2, line: 2, type: !5, isLocal: false, isDefinition: true)
>> +!12 = !DIImportedEntity(tag: DW_TAG_imported_declaration, line: 1, scope: !1, entity: !11)
>> 
>> Modified: llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll.bc
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll.bc?rev=294318&r1=294317&r2=294318&view=diff
>> ==============================================================================
>> Binary files llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll.bc (original) and llvm/trunk/test/Bitcode/DIGlobalVariableExpression.ll.bc Tue Feb  7 11:35:41 2017 differ
>> 
>> Modified: llvm/trunk/test/Bitcode/dityperefs-3.8.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/dityperefs-3.8.ll?rev=294318&r1=294317&r2=294318&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Bitcode/dityperefs-3.8.ll (original)
>> +++ llvm/trunk/test/Bitcode/dityperefs-3.8.ll Tue Feb  7 11:35:41 2017
>> @@ -18,14 +18,13 @@
>> ; CHECK-NEXT: !7 = !DILocalVariable(name: "V1", scope: !6, type: !2)
>> ; CHECK-NEXT: !8 = !DIObjCProperty(name: "P1", type: !1)
>> ; CHECK-NEXT: !9 = !DITemplateTypeParameter(type: !1)
>> -; CHECK-NEXT: !10 = distinct !DIGlobalVariableExpression(var: !11)
>> -; CHECK-NEXT: !11 = !DIGlobalVariable(name: "G",{{.*}} type: !1,
>> -; CHECK-NEXT: !12 = !DITemplateValueParameter(type: !1, value: i32* @G1)
>> -; CHECK-NEXT: !13 = !DIImportedEntity(tag: DW_TAG_imported_module, name: "T2", scope: !0, entity: !1)
>> -; CHECK-NEXT: !14 = !DICompositeType(tag: DW_TAG_structure_type, name: "T3", file: !0, elements: !15, identifier: "T3")
>> -; CHECK-NEXT: !15 = !{!16}
>> -; CHECK-NEXT: !16 = !DISubprogram(scope: !14,
>> -; CHECK-NEXT: !17 = !DIDerivedType(tag: DW_TAG_ptr_to_member_type,{{.*}} extraData: !14)
>> +; CHECK-NEXT: !10 = !DIGlobalVariable(name: "G",{{.*}} type: !1,
>> +; CHECK-NEXT: !11 = !DITemplateValueParameter(type: !1, value: i32* @G1)
>> +; CHECK-NEXT: !12 = !DIImportedEntity(tag: DW_TAG_imported_module, name: "T2", scope: !0, entity: !1)
>> +; CHECK-NEXT: !13 = !DICompositeType(tag: DW_TAG_structure_type, name: "T3", file: !0, elements: !14, identifier: "T3")
>> +; CHECK-NEXT: !14 = !{!15}
>> +; CHECK-NEXT: !15 = !DISubprogram(scope: !13,
>> +; CHECK-NEXT: !16 = !DIDerivedType(tag: DW_TAG_ptr_to_member_type,{{.*}} extraData: !13)
>> 
>> !0 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
>> !1 = !DICompositeType(tag: DW_TAG_structure_type, name: "T1", file: !0, identifier: "T1")
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list