<div dir="ltr">This one shouldn't have gone in yet, accidentally committed along with something else. Reverting...<div>Teresa</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 23, 2018 at 3:59 PM Teresa Johnson via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: tejohnson<br>
Date: Tue Oct 23 15:57:21 2018<br>
New Revision: 345095<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=345095&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=345095&view=rev</a><br>
Log:<br>
[ThinLTO] Fix a crash in lazy loading of Metadata<br>
<br>
Summary:<br>
This is a revised version of D41474.<br>
<br>
When the debug location is parsed in BitcodeReader::parseFunction, the<br>
scope and inlinedAt MDNodes are obtained via MDLoader->getMDNodeFwdRefOrNull(),<br>
which will create a forward ref if they were not yet loaded.<br>
Specifically, if one of these MDNodes is in the module level metadata<br>
block, and this is during ThinLTO importing, that metadata block is<br>
lazily loaded.<br>
<br>
Most places in that invoke getMDNodeFwdRefOrNull have a corresponding call<br>
to resolveForwardRefsAndPlaceholders which will take care of resolving them.<br>
E.g. places that call getMetadataFwdRefOrLoad, or at the end of parsing a<br>
function-level metadata block, or at the end of the initial lazy load of<br>
module level metadata in order to handle invocations of getMDNodeFwdRefOrNull<br>
for named metadata and global object attachments. However, the calls for<br>
the scope/inlinedAt of debug locations are not backed by any such call to<br>
resolveForwardRefsAndPlaceholders.<br>
<br>
To fix this, change the scope and inlinedAt parsing to instead use<br>
getMetadataFwdRefOrLoad, which will ensure the forward refs to lazily<br>
loaded metadata are resolved.<br>
<br>
Fixes PR35472.<br>
<br>
Reviewers: dexonsmith, Sunil_Srivastava, vsk<br>
<br>
Subscribers: inglorion, eraman, steven_wu, sebpop, mehdi_amini, dmikulin, vsk, hiraditya, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D53596" rel="noreferrer" target="_blank">https://reviews.llvm.org/D53596</a><br>
<br>
Added:<br>
    llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll<br>
    llvm/trunk/test/ThinLTO/X86/pr35472.ll<br>
Modified:<br>
    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
<br>
Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=345095&r1=345094&r2=345095&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=345095&r1=345094&r2=345095&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)<br>
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Tue Oct 23 15:57:21 2018<br>
@@ -3520,12 +3520,14 @@ Error BitcodeReader::parseFunctionBody(F<br>
<br>
       MDNode *Scope = nullptr, *IA = nullptr;<br>
       if (ScopeID) {<br>
-        Scope = MDLoader->getMDNodeFwdRefOrNull(ScopeID - 1);<br>
+        Scope = dyn_cast_or_null<MDNode>(<br>
+            MDLoader->getMetadataFwdRefOrLoad(ScopeID - 1));<br>
         if (!Scope)<br>
           return error("Invalid record");<br>
       }<br>
       if (IAID) {<br>
-        IA = MDLoader->getMDNodeFwdRefOrNull(IAID - 1);<br>
+        IA = dyn_cast_or_null<MDNode>(<br>
+            MDLoader->getMetadataFwdRefOrLoad(IAID - 1));<br>
         if (!IA)<br>
           return error("Invalid record");<br>
       }<br>
<br>
Added: llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll?rev=345095&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll?rev=345095&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll (added)<br>
+++ llvm/trunk/test/ThinLTO/X86/Inputs/pr35472.ll Tue Oct 23 15:57:21 2018<br>
@@ -0,0 +1,13 @@<br>
+; ModuleID = 'b.cpp'<br>
+source_filename = "b.cpp"<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; Function Attrs: noinline optnone uwtable<br>
+define void @_Z5Alphav() {<br>
+entry:<br>
+  call void @_Z5Bravov()<br>
+  ret void<br>
+}<br>
+<br>
+declare void @_Z5Bravov()<br>
<br>
Added: llvm/trunk/test/ThinLTO/X86/pr35472.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/pr35472.ll?rev=345095&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/pr35472.ll?rev=345095&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/ThinLTO/X86/pr35472.ll (added)<br>
+++ llvm/trunk/test/ThinLTO/X86/pr35472.ll Tue Oct 23 15:57:21 2018<br>
@@ -0,0 +1,122 @@<br>
+; Test to make sure that lazily loaded debug location scope metadata is<br>
+; handled properly. Note that we need to have the DILexicalScope !34<br>
+; referenced from multiple function's debug locs for this to be in the<br>
+; lazily loaded module level metadata block.<br>
+<br>
+; RUN: opt -module-hash -module-summary %s -o %t1.bc<br>
+; RUN: opt -module-hash -module-summary %p/Inputs/pr35472.ll -o %t2.bc<br>
+; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc<br>
+; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s -check-prefix=ThinLTOa<br>
+; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck %s -check-prefix=ThinLTOb<br>
+<br>
+; ThinLTOa-DAG: T _Z5Bravov<br>
+; ThinLTOa-DAG: W _ZN4EchoD2Ev<br>
+; ThinLTOb-DAG: T _Z5Alphav<br>
+<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+%struct.Delta = type { %struct.Charlie }<br>
+%struct.Charlie = type { i32 }<br>
+%struct.Echo = type { %struct.Charlie }<br>
+<br>
+$_ZN4EchoD2Ev = comdat any<br>
+$_ZN5DeltaD2Ev = comdat any<br>
+<br>
+define void @_Z5Bravov() !dbg !7 {<br>
+  %Hotel = alloca %struct.Delta, align 4<br>
+  %India = alloca %struct.Echo, align 4<br>
+  call void @llvm.dbg.declare(metadata %struct.Delta* %Hotel, metadata !10, metadata !DIExpression()), !dbg !22<br>
+  call void @_ZN4EchoD2Ev(%struct.Echo* %India), !dbg !28<br>
+  ret void, !dbg !28<br>
+}<br>
+<br>
+declare void @llvm.dbg.declare(metadata, metadata, metadata)<br>
+<br>
+define linkonce_odr void @_ZN4EchoD2Ev(%struct.Echo* %this) unnamed_addr comdat align 2 {<br>
+  %this.addr.i = alloca %struct.Charlie*, align 8<br>
+  call void @llvm.dbg.declare(metadata %struct.Charlie** %this.addr.i, metadata !29, metadata !DIExpression()), !dbg !32<br>
+  %this1.i = load %struct.Charlie*, %struct.Charlie** %this.addr.i, align 8<br>
+  %Golf.i = getelementptr inbounds %struct.Charlie, %struct.Charlie* %this1.i, i32 0, i32 0, !dbg !33<br>
+  ret void<br>
+}<br>
+<br>
+define linkonce_odr void @_ZN5DeltaD2Ev(%struct.Delta* %this) unnamed_addr comdat align 2 !dbg !36 {<br>
+  %this.addr.i = alloca %struct.Charlie*, align 8<br>
+  call void @llvm.dbg.declare(metadata %struct.Charlie** %this.addr.i, metadata !29, metadata !DIExpression()), !dbg !41<br>
+  %this1.i = load %struct.Charlie*, %struct.Charlie** %this.addr.i, align 8<br>
+  %Golf.i = getelementptr inbounds %struct.Charlie, %struct.Charlie* %this1.i, i32 0, i32 0, !dbg !48<br>
+  ret void<br>
+}<br>
+<br>
+!llvm.module.flags = !{!3, !4, !5}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 6.0.0 (trunk 321056)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)<br>
+!1 = !DIFile(filename: "a.cpp", directory: "/home/sunil/185335/302")<br>
+!2 = !{}<br>
+!3 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!5 = !{i32 1, !"wchar_size", i32 4}<br>
+!7 = distinct !DISubprogram(name: "Bravo", linkageName: "_Z5Bravov", scope: !1, file: !1, line: 17, type: !8, isLocal: false, isDefinition: true, scopeLine: 17, flags: DIFlagPrototyped, isOptimized: false, unit: !0)<br>
+!8 = !DISubroutineType(types: !9)<br>
+!9 = !{null}<br>
+!10 = !DILocalVariable(name: "Hotel", scope: !7, file: !1, line: 18, type: !11)<br>
+!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Delta", file: !1, line: 6, size: 32, elements: !12, identifier: "_ZTS5Delta")<br>
+!12 = !{!13}<br>
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "Foxtrot", scope: !11, file: !1, line: 7, baseType: !14, size: 32)<br>
+!14 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Charlie", file: !1, line: 1, size: 32, elements: !15, identifier: "_ZTS7Charlie")<br>
+!15 = !{!16, !18}<br>
+!16 = !DIDerivedType(tag: DW_TAG_member, name: "Golf", scope: !14, file: !1, line: 3, baseType: !17, size: 32)<br>
+!17 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)<br>
+!18 = !DISubprogram(name: "~Charlie", scope: !14, file: !1, line: 2, type: !19, isLocal: false, isDefinition: false, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false)<br>
+!19 = !DISubroutineType(types: !20)<br>
+!20 = !{null, !21}<br>
+!21 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !14, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)<br>
+!22 = !DILocation(line: 18, column: 11, scope: !7)<br>
+!24 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Echo", file: !1, line: 10, size: 32, elements: !25, identifier: "_ZTS4Echo")<br>
+!25 = !{!26}<br>
+!26 = !DIDerivedType(tag: DW_TAG_member, name: "Foxtrot", scope: !24, file: !1, line: 11, baseType: !14, size: 32)<br>
+!28 = !DILocation(line: 20, column: 1, scope: !7)<br>
+!29 = !DILocalVariable(name: "this", arg: 1, scope: !30, type: !31, flags: DIFlagArtificial | DIFlagObjectPointer)<br>
+!30 = distinct !DISubprogram(name: "~Charlie", linkageName: "_ZN7CharlieD2Ev", scope: !14, file: !1, line: 2, type: !19, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false, unit: !0, declaration: !18)<br>
+!31 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !14, size: 64)<br>
+!32 = !DILocation(line: 0, scope: !30)<br>
+!33 = !DILocation(line: 2, column: 53, scope: !34)<br>
+!34 = distinct !DILexicalBlock(scope: !30, file: !1, line: 2, column: 51)<br>
+!36 = distinct !DISubprogram(name: "~Delta", linkageName: "_ZN5DeltaD2Ev", scope: !11, file: !1, line: 6, type: !37, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagArtificial | DIFlagPrototyped, isOptimized: false, unit: !0, declaration: !40)<br>
+!37 = !DISubroutineType(types: !38)<br>
+!38 = !{null, !39}<br>
+!39 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)<br>
+!40 = !DISubprogram(name: "~Delta", scope: !11, type: !37, isLocal: false, isDefinition: false, flags: DIFlagArtificial | DIFlagPrototyped, isOptimized: false)<br>
+!41 = !DILocation(line: 0, scope: !30, inlinedAt: !42)<br>
+!42 = distinct !DILocation(line: 6, column: 8, scope: !43)<br>
+!43 = distinct !DILexicalBlock(scope: !36, file: !1, line: 6, column: 8)<br>
+!48 = !DILocation(line: 2, column: 53, scope: !34, inlinedAt: !42)<br>
+<br>
+;----------------------------------------------------------------------------------------------<br>
+; Compiled from following two source files with 'clang++ -S --std=c++11 -O0 -g -flto=thin' <br>
+; struct Charlie {<br>
+;     __attribute__((__always_inline__)) ~Charlie() { Golf = 0; }<br>
+;     int Golf;<br>
+; };<br>
+; <br>
+; struct Delta {<br>
+;     Charlie Foxtrot;<br>
+; };<br>
+; <br>
+; struct Echo {<br>
+;     Charlie Foxtrot;<br>
+;     __attribute__((nodebug)) ~Echo() = default;<br>
+; };<br>
+; <br>
+; extern void Bravo();<br>
+; <br>
+; void Bravo() {<br>
+;     Delta Hotel;<br>
+;     Echo India;<br>
+; }<br>
+; -----------------------------<br>
+; extern void Bravo();<br>
+; extern void Alpha();<br>
+; void Alpha() { Bravo(); }<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"><br></td></tr></tbody></table></span></div></div></div>