[Verifier] Skip debug location check for some non-inlinable functions

[ychen]

Summary

If a callee function is not interposable, skip debug location check for its callsites. Doing this is instrumentation-friendly otherwise under some conditions this check triggers for some un-inlinable call sites.

[aprantl]

This comment from GlobalValue::isDefinitionExact() makes me wonder if this is the right condition? Sounds like there are some non-exact functions that may be inlined?

/// Inlining is okay across non-exact linkage types as long as they're not
/// interposable (see \c isInterposable), since in such cases the currently
/// visible variant is *a* correct implementation of the original source
/// function; it just isn't the *only* correct implementation.

[ychen]

In D133060#3765354, @aprantl wrote:

This comment from GlobalValue::isDefinitionExact() makes me wonder if this is the right condition? Sounds like there are some non-exact functions that may be inlined?

/// Inlining is okay across non-exact linkage types as long as they're not
/// interposable (see \c isInterposable), since in such cases the currently
/// visible variant is *a* correct implementation of the original source
/// function; it just isn't the *only* correct implementation.

Hmm, thanks for catching that. There are actually a bunch of other attributes that matter (https://github.com/llvm/llvm-project/blob/fdec50182d85ec0b8518af3baae37ae28b102f1c/llvm/lib/Analysis/InlineCost.cpp#L2861). However, I think checking linkages should avoid most of the unnecessary firing of this assertion. I'll change it to check isInterposable.

[ychen]

• use isInterposable

[aprantl]

I wish we had an "isInlineable" function call.

llvm/lib/IR/Verifier.cpp

3448

Can you add a comment here that the intention is to ignore non-inlineable functions? Future readers will probably wonder what isInterposable even means.

[aprantl]

LGTM with the comment added.

[ychen]

• add comment

[ychen]

Thanks for the review.

llvm/lib/IR/Verifier.cpp

void Verifier::visitCallBase(CallBase &Call) {
   }
 
   // Verify that callee and callsite agree on whether to use pointer auth.
   Check(!(Call.getCalledFunction() && FoundPtrauthBundle),
         "Direct call cannot have a ptrauth bundle", Call);
 
   // Verify that each inlinable callsite of a debug-info-bearing function in a
   // debug-info-bearing function has a debug location attached to it. Failure to
-  // do so causes assertion failures when the inliner sets up inline scope info.
+  // do so causes assertion failures when the inliner sets up inline scope info
+  // (Interposable functions are not inlinable).
   if (Call.getFunction()->getSubprogram() && Call.getCalledFunction() &&
+      !Call.getCalledFunction()->isInterposable() &&
       Call.getCalledFunction()->getSubprogram())
     CheckDI(Call.getDebugLoc(),
             "inlinable function call in a function with "
             "debug info must have a !dbg location",
             Call);
 
   if (Call.isInlineAsm())
     verifyInlineAsmCall(Call);

llvm/test/Verifier/callsite-dbgloc.ll

 
 ; Function Attrs: nounwind ssp uwtable
 define void @h() #0 !dbg !7 {
 entry:
   call void (...) @i(), !dbg !9
   ret void, !dbg !10
 }
 
+; Function Attrs: nounwind ssp uwtable
+define weak void @j() #0 !dbg !17 {
+entry:
+  call void (...) @i(), !dbg !18
+  ret void, !dbg !19
+}
+
+; Function Attrs: nounwind ssp uwtable
+define linkonce_odr void @k() #0 !dbg !20 {
+entry:
+  call void (...) @i(), !dbg !21
+  ret void, !dbg !22
+}
+
 declare void @i(...) #1
 
 ; Function Attrs: nounwind ssp uwtable
 define void @g() #0 !dbg !11 {
 entry:
 ; Manually removed !dbg.
 ; CHECK: inlinable function call in a function with debug info must have a !dbg location
   call void @h()
+; CHECK-NOT: inlinable function call in a function with debug info must have a !dbg location
+  call void @j()
+; CHECK: inlinable function call in a function with debug info must have a !dbg location
+  call void @k()
   ret void, !dbg !13
 }
 
 ; Function Attrs: nounwind ssp uwtable
 define void @f() #0 !dbg !14 {
 entry:
   call void @g(), !dbg !15
   ret void, !dbg !16

 !9 = !DILocation(line: 2, column: 12, scope: !7)
 !10 = !DILocation(line: 2, column: 17, scope: !7)
 !11 = distinct !DISubprogram(name: "g", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, isOptimized: false, unit: !0, retainedNodes: !2)
 !12 = !DILocation(line: 3, column: 12, scope: !11)
 !13 = !DILocation(line: 3, column: 17, scope: !11)
 !14 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !0, retainedNodes: !2)
 !15 = !DILocation(line: 4, column: 12, scope: !14)
 !16 = !DILocation(line: 4, column: 17, scope: !14)
+!17 = distinct !DISubprogram(name: "j", scope: !1, file: !1, line: 5, type: !8, isLocal: false, isDefinition: true, scopeLine: 5, isOptimized: false, unit: !0, retainedNodes: !2)
+!18 = !DILocation(line: 4, column: 12, scope: !17)
+!19 = !DILocation(line: 4, column: 17, scope: !17)
+!20 = distinct !DISubprogram(name: "k", scope: !1, file: !1, line: 6, type: !8, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: false, unit: !0, retainedNodes: !2)
+!21 = !DILocation(line: 4, column: 12, scope: !20)
+!22 = !DILocation(line: 4, column: 17, scope: !20)