[ThinLTO] Write TYPE_IDs for types used in functions imported by aliases

[vitalybuka]

Summary

ThinLTO imports alias as a copy of a aliasee, so when we import such functions with type tests we will
need type ids used by function. However after D49565 we pick types only during processing of
FunctionSummary which is not happening for such aliesees.

Example:
Unit U1 with a type, a functions F with the type check, and an alias A to the function.
Unit U2 with only call to the alias A.

In particular, this happens when we use -mconstructor-aliases, which is default.
So if c++ unit only creates instance of the class, without calling any other methods it will lack of
necessary type ids, which will result in false CFI reports.

[tejohnson]

Can you update description to point out that this is an issue because we will import the alias as a copy of aliasee?

Out of curiosity, what is the failure mode with this bug?

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3881 ↗

(On Diff #165855)

We have already invoked AS->getAliasee() above (which would assert if there was no aliasee). So this check shouldn't be needed.

llvm/test/ThinLTO/X86/cfi-distributed.ll

24 ↗

(On Diff #165855)

Typo in RUN. But it looks like this RUN shouldn't be here as I don't see any COMBINED checks?

27 ↗

(On Diff #165855)

Comment needs updating.

34 ↗

(On Diff #165855)

This comment seems wrong - I think it should be "should only contain the typeid for _ZTS1A2" (which is what we're checking) - can you fix that? I think the comment/check will also need an update for your change - it should also contain the typeid for _ZTS1B2, right?

[vitalybuka]

Addressed comments from @tejohnson

llvm/test/ThinLTO/X86/cfi-distributed.ll

34 ↗

(On Diff #165855)

_ZTS1A3, done

[vitalybuka]

In D52201#1239405, @tejohnson wrote:

Can you update description to point out that this is an issue because we will import the alias as a copy of aliasee?

Out of curiosity, what is the failure mode with this bug?

False CFI errrors. I've just added that into description.

[tejohnson]

LGTM

llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

for (auto *AS : Aliases) {
     auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
     assert(AliaseeValueId);
     NameVals.push_back(AliaseeValueId);
 
     // Emit the finished record.
     Stream.EmitRecord(bitc::FS_COMBINED_ALIAS, NameVals, FSAliasAbbrev);
     NameVals.clear();
     MaybeEmitOriginalName(*AS);
+
+    if (auto *FS = dyn_cast<FunctionSummary>(&AS->getAliasee()))
+      getReferencedTypeIds(FS, ReferencedTypeIds);
   }
 
   if (!Index.cfiFunctionDefs().empty()) {
     for (auto &S : Index.cfiFunctionDefs()) {
       NameVals.push_back(StrtabBuilder.add(S));
       NameVals.push_back(S.size());
     }
     Stream.EmitRecord(bitc::FS_CFI_FUNCTION_DEFS, NameVals);

llvm/trunk/test/ThinLTO/X86/Inputs/cfi-distributed.ll

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.B2 = type { %struct.A2 }
 %struct.A2 = type { i32 (...)** }
+%struct.B3 = type { %struct.A3 }
+%struct.A3 = type { i32 (...)** }
 
 @_ZTV1B2 = constant { [3 x i8*] } { [3 x i8*] [i8* undef, i8* undef, i8* undef] }, !type !0
 
+@_ZTV1B3 = constant { [3 x i8*] } { [3 x i8*] [i8* undef, i8* undef, i8* undef] }, !type !1
+
 define void @test2(i8* %b) {
 entry:
   %0 = bitcast i8* %b to i8**
   %vtable2 = load i8*, i8** %0
   %1 = tail call i1 @llvm.type.test(i8* %vtable2, metadata !"_ZTS1A2")
   br i1 %1, label %cont, label %trap
 
 trap:
   tail call void @llvm.trap()
   unreachable
 
 cont:
   ret void
 }
 
+define void @test1(i8* %b) {
+entry:
+  %0 = bitcast i8* %b to i8**
+  %vtable2 = load i8*, i8** %0
+  %1 = tail call i1 @llvm.type.test(i8* %vtable2, metadata !"_ZTS1A3")
+  br i1 %1, label %cont, label %trap
+
+trap:
+  tail call void @llvm.trap()
+  unreachable
+
+cont:
+  ret void
+}
+
+@test3 = hidden unnamed_addr alias void (i8*), void (i8*)* @test1
+
 declare i1 @llvm.type.test(i8*, metadata)
 declare void @llvm.trap()
 
 !0 = !{i64 16, !"_ZTS1A2"}
-!1 = !{i64 16, !"_ZTS1B2"}
+!1 = !{i64 16, !"_ZTS1A3"}

llvm/trunk/test/ThinLTO/X86/cfi-distributed.ll

 ; REQUIRES: x86-registered-target
 
 ; Test to ensure that only referenced type ID records are emitted into
 ; each distributed index file.
 
 ; RUN: opt -thinlto-bc -o %t1.o %s
 ; RUN: opt -thinlto-bc -o %t2.o %p/Inputs/cfi-distributed.ll
 
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes %t1.o %t2.o \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t1.o,test,px \
 ; RUN:   -r=%t1.o,_ZTV1B, \
 ; RUN:   -r=%t1.o,_ZTV1B,px \
 ; RUN:   -r=%t1.o,test2, \
+; RUN:   -r=%t1.o,test3, \
+; RUN:   -r=%t2.o,test1, \
+; RUN:   -r=%t2.o,test3,p \
 ; RUN:   -r=%t2.o,test2,px \
 ; RUN:   -r=%t2.o,_ZTV1B2, \
-; RUN:   -r=%t2.o,_ZTV1B2,px
+; RUN:   -r=%t2.o,_ZTV1B2,px \
+; RUN:   -r=%t2.o,_ZTV1B3, \
+; RUN:   -r=%t2.o,_ZTV1B3,px
 
-; Since @test calls @test2, the latter should be imported here and the
-; first index file should reference both type ids.
+; Check that we have all needed type IDs.
 ; RUN: llvm-dis %t1.o.thinlto.bc -o - | FileCheck %s --check-prefix=INDEX1
+
+; Used by @llvm.type.test in @test
 ; INDEX1: typeid: (name: "_ZTS1A"
+
+; Used by @llvm.type.test in @test2 imported to be called from @test
 ; INDEX1: typeid: (name: "_ZTS1A2"
 
-; The second index file, corresponding to @test2, should only contain the
-; typeid for _ZTS1A.
+; Used by @llvm.type.test in @test1 imported to be called by alias
+; @test3 from @test
+; INDEX1: typeid: (name: "_ZTS1A3"
+
+; The second index file, should only contain the type IDs used in @test1 and @test2.
 ; RUN: llvm-dis %t2.o.thinlto.bc -o - | FileCheck %s --check-prefix=INDEX2
 ; INDEX2-NOT: typeid: (name: "_ZTS1A"
 ; INDEX2: typeid: (name: "_ZTS1A2"
+; INDEX2: typeid: (name: "_ZTS1A3"
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.B = type { %struct.A }
 %struct.A = type { i32 (...)** }
 
 @_ZTV1B = constant { [3 x i8*] } { [3 x i8*] [i8* undef, i8* undef, i8* undef] }, !type !0
 
 define void @test(i8* %b) {
 entry:
   tail call void @test2(i8* %b)
+  tail call void @test3(i8* %b)
   %0 = bitcast i8* %b to i8**
   %vtable2 = load i8*, i8** %0
   %1 = tail call i1 @llvm.type.test(i8* %vtable2, metadata !"_ZTS1A")
   br i1 %1, label %cont, label %trap
 
 trap:
   tail call void @llvm.trap()
   unreachable
 
 cont:
   ret void
 }
 
 declare void @test2(i8*)
+declare void @test3(i8*)
 declare i1 @llvm.type.test(i8*, metadata)
 declare void @llvm.trap()
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTS1B"}