This is an archive of the discontinued LLVM Phabricator instance.

Ensure sanitizer check function calls have a !dbg location
ClosedPublic

Authored by aprantl on Oct 19 2018, 4:33 PM.

Details

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

Diff Detail

Repository
rC Clang

Event Timeline

aprantl created this revision.Oct 19 2018, 4:33 PM
vsk added inline comments.Oct 19 2018, 5:11 PM
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 added inline comments.Oct 19 2018, 5:16 PM
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 added inline comments.Oct 19 2018, 5:21 PM
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 updated this revision to Diff 170433.Oct 22 2018, 8:51 AM

Simplify testcase

aprantl updated this revision to Diff 170434.Oct 22 2018, 8:52 AM

further simplify testcase

vsk accepted this revision.Oct 22 2018, 9:11 AM

Thanks!

This revision is now accepted and ready to land.Oct 22 2018, 9:11 AM
This revision was automatically updated to reflect the committed changes.