Fix insertion of `sanitizer_cov_trace_pc_guard` insertion in optimized code with debug info

[mehdi_amini]

Summary

It is illegal to have a call without debug info attached in a function
with debug info: it'll crash the backend.
However this pattern can happen after jump-threading, so we need to be
robust against this. The instrumentation was only looking at the block
entry, if there is no debug location there we will now default to the
function's one.

[kcc]

is it possible to add an assert() that fails now and gets fixed with your patch?

[mehdi_amini]

In D30307#684972, @kcc wrote:

is it possible to add an assert() that fails now and gets fixed with your patch?

piping to llc hits an assert today, I'm not sure it'd be appropriate to use llc here just for this purpose though?

[kcc]

LGTM
(but not claiming to understand the problem)

[mehdi_amini]

fix test to fail the verifier

[mehdi_amini]

In D30307#684976, @kcc wrote:

LGTM
(but not claiming to understand the problem)

OK, so this patch might not be needed (even though having a better failure here could be helpful). I returned to my original test case to have another look. The verifier is crashing so no need for llc, I updated the test-case. What is needed is the *definition* for the runtime function. This seems to indicate that I'm instrumenting the runtime, which does not seem correct.
My clang is crashing when building for running make check-fuzzer right now, this is how I hit this.

The crash happened in llvm/lib/Fuzzer/FuzzerTracePC.cpp with this invocation:

/Users/mehdi_amini/projects/vanilla/clang/ReleaseAssert/bin/clang++ -DLLVM_BUILD_GLOBAL_ISEL -D_DEBUG -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Fuzzer -I/Users/mehdi_amini/projects/vanilla/clang/llvm-project/llvm/lib/Fuzzer -Iinclude -I/Users/mehdi_amini/projects/vanilla/clang/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -Werror=date-time -std=c++11 -fno-omit-frame-pointer -O1 -fsanitize=address -fsanitize-address-use-after-scope -fsanitize-coverage=trace-pc-guard,indirect-calls,trace-cmp -fcolor-diagnostics -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters -Werror -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk -MD -MT lib/Fuzzer/CMakeFiles/LLVMFuzzerNoMainObjects.dir/FuzzerTracePC.cpp.o -MF lib/Fuzzer/CMakeFiles/LLVMFuzzerNoMainObjects.dir/FuzzerTracePC.cpp.o.d -o lib/Fuzzer/CMakeFiles/LLVMFuzzerNoMainObjects.dir/FuzzerTracePC.cpp.o -c /Users/mehdi_amini/projects/vanilla/clang/llvm-project/llvm/lib/Fuzzer/FuzzerTracePC.cpp -g

See the -fsanitize-coverage=trace-pc-guard,.. without trace-pc-guard being present in the -fno-sanitize-coverage. I think I'm faulty as I was playing with the CMakefiles right now.

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

I->setMetadata(I->getModule()->getMDKindID("nosanitize"),
                  MDNode::get(*C, None));
 }
 
 void SanitizerCoverageModule::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
                                                     size_t Idx, bool UseCalls) {
   BasicBlock::iterator IP = BB.getFirstInsertionPt();
   bool IsEntryBB = &BB == &F.getEntryBlock();
   DebugLoc EntryLoc;
-  if (IsEntryBB) {
-    if (auto SP = F.getSubprogram())
-      EntryLoc = DebugLoc::get(SP->getScopeLine(), 0, SP);
+  if (IsEntryBB) {
     // Keep static allocas and llvm.localescape calls in the entry block.  Even
     // if we aren't splitting the block, it's nice for allocas to be before
     // calls.
     IP = PrepareToSplitEntryBlock(BB, IP);
-  } else {
+  } else if(IP->getDebugLoc()) {
     EntryLoc = IP->getDebugLoc();
   }
 
   IRBuilder<> IRB(&*IP);
   IRB.SetCurrentDebugLocation(EntryLoc);
   if (Options.TracePC) {
     IRB.CreateCall(SanCovTracePC); // gets the PC using GET_CALLER_PC.
     IRB.CreateCall(EmptyAsm, {}); // Avoids callback merge.

llvm/test/Instrumentation/SanitizerCoverage/coverage-dbg-pcguard.ll

+; Test that coverage instrumentation does not lose debug location.
+
+; RUN: opt < %s -sancov -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard  -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @__sanitizer_cov_trace_pc_guard(i32*)  !dbg !13 {
+  ret void
+}
+
+define void @_ZN1A1fEv(i1 %cond) !dbg !13 {
+entry:
+  br i1 %cond, label %cont, label %exit
+cont:
+  br label %exit
+exit:
+  ret void, !dbg !21
+}
+
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!17, !18}
+!llvm.ident = !{!19}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.5.0 (210251)", isOptimized: true, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !3, globals: !2, imports: !2)
+!1 = !DIFile(filename: "../1.cc", directory: "/code/llvm/build0")
+!2 = !{}
+!3 = !{!4}
+!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "A", line: 1, size: 32, align: 32, file: !1, elements: !5, identifier: "_ZTS1A")
+!5 = !{!6, !8}
+!6 = !DIDerivedType(tag: DW_TAG_member, name: "x", line: 3, size: 32, align: 32, file: !1, scope: !4, baseType: !7)
+!7 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!8 = !DISubprogram(name: "f", linkageName: "_ZN1A1fEv", line: 2, isLocal: false, isDefinition: false, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: true, scopeLine: 2, file: !1, scope: !4, type: !9)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!7, !11}
+!11 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, flags: DIFlagArtificial | DIFlagObjectPointer, baseType: !4)
+!13 = distinct !DISubprogram(name: "f", linkageName: "_ZN1A1fEv", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !0, scopeLine: 6, file: !1, scope: !4, type: !9, declaration: !8, variables: !14)
+!14 = !{!15}
+!15 = !DILocalVariable(name: "this", arg: 1, flags: DIFlagArtificial | DIFlagObjectPointer, scope: !13, type: !16)
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, baseType: !4)
+!17 = !{i32 2, !"Dwarf Version", i32 4}
+!18 = !{i32 2, !"Debug Info Version", i32 3}
+!19 = !{!"clang version 3.5.0 (210251)"}
+!20 = !DILocation(line: 0, scope: !13)
+!21 = !DILocation(line: 7, scope: !13)