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

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 13:26:48 PST 2017


Merged in r294352.

Thanks everyone!

On Tue, Feb 7, 2017 at 12:05 PM, Eric Christopher via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> I think this is me to approve so: OK.
>
> -eric
>
> On Tue, Feb 7, 2017 at 12:00 PM Mehdi Amini via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> 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
>>
>> _______________________________________________
>> 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