Ensure sanitizer check function calls have a !dbg location

[aprantl]

Summary

Function calls without a !dbg location inside a function that has a DISubprogram make it impossible to construct inline information and are rejected by the verifier. This patch ensures that sanitizer check function calls have a !dbg location, by carrying forward the location of the preceding instruction or by inserting an artificial location if necessary.

This fixes a crash when compiling the attached testcase with -Os.

rdar://problem/45311226

[vsk]

lib/CodeGen/CGExpr.cpp

2871

Why shouldn't this always be line 0? A call to a check handler is always auto-generated.

test/CodeGenCXX/ubsan-check-debuglocs.cpp

3

Are all three sanitizers needed here to reproduce the bug? Seems like a simpler test would be:

// RUN: ... -fsanitize=null ...

int deref(int *p) { return *p; }
// CHECK-LABEL: @deref
// CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg [[ubsan_handler_loc:![0-9]+]]
// CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0

[aprantl]

lib/CodeGen/CGExpr.cpp

2871

I was thinking that it might be nice to have the source location of the expression with the UB in the backtrace, too — not just in the error message. (Which is the current behavior by accident, since no location means carry over the previous instruction's location). But I'm fine with just setting line 0, too.

test/CodeGenCXX/ubsan-check-debuglocs.cpp

3

Probably not for the testcase; only to trigger the subsequent crash.

[vsk]

lib/CodeGen/CGExpr.cpp

2871

I see, that seems reasonable.

test/CodeGenCXX/ubsan-check-debuglocs.cpp

3

I see -- we probably don't need the full crash repro for this change. At any rate I don't think we support ubsan-itizing the ubsan runtime..

[aprantl]

Simplify testcase

[aprantl]

further simplify testcase

[vsk]

Thanks!

lib/CodeGen/CGExpr.cpp

 
 static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                  llvm::FunctionType *FnType,
                                  ArrayRef<llvm::Value *> FnArgs,
                                  SanitizerHandler CheckHandler,
                                  CheckRecoverableKind RecoverKind, bool IsFatal,
                                  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
       IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
   const SanitizerHandlerInfo &CheckInfo = SanitizerHandlers[CheckHandler];
   const StringRef CheckName = CheckInfo.Name;
   std::string FnName = "__ubsan_handle_" + CheckName.str();
   if (CheckInfo.Version && !MinimalRuntime)
     FnName += "_v" + llvm::utostr(CheckInfo.Version);

test/CodeGenCXX/ubsan-check-debuglocs.cpp

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
+// CHECK-SAME: !dbg
+
+struct SourceLocation {
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}