Page MenuHomePhabricator

[sanitizers][Windows] Better debug output and continuable option for interception failures on Windows
Needs ReviewPublic

Authored by mcgov on Thu, Sep 3, 11:43 AM.

Details

Summary

Currently interception failures just debugbreak, this is confusing to users since the failure can manifest in unexpected ways when a debugger is not attached. When a debugger is attached, there is no good error message explaining what has gone wrong.

This patch fixes this issue and outputs an clear error to the debugger, as well as provides an option for continuing past the interception failure if the user doesn't need the function that has failed to intercept.

Diff Detail

Event Timeline

mcgov requested review of this revision.Thu, Sep 3, 11:43 AM
mcgov planned changes to this revision.
mcgov created this revision.
mcgov updated this revision to Diff 289776.Thu, Sep 3, 11:55 AM

fixed, break should occur if the env variable is not set OR if there was an error finding the variable, so anytime the return code is 0.

mcgov updated this revision to Diff 289779.Thu, Sep 3, 11:59 AM

removed whitespace diff

vitalybuka added inline comments.Fri, Sep 4, 5:20 AM
b\compiler-rt\lib\interception\interception_win.cpp
149

can we use sanitizer_common flags?

mcgov added inline comments.Fri, Sep 4, 10:48 AM
b\compiler-rt\lib\interception\interception_win.cpp
149

This is a good point, I was trying to only use things I was sure was initialized in the winapi but using an asan_option is cleaner.

One hiccup, adding this header to interception_win... There's something additional needed for the common_flag symbol to not be undefined in asan_dll_thunk. Do I need to add something to resolve this at runtime in that thunk library? Any tips here are appreciated

vitalybuka added inline comments.Sun, Sep 6, 9:08 PM
b\compiler-rt\lib\interception\interception_win.cpp
149

Not sure how this works on windows, but in general we init interceptors after common_flags()

vitalybuka added inline comments.Wed, Sep 16, 1:53 PM
b\compiler-rt\lib\interception\interception_win.cpp
149

This stuff is common for all sanitizers so ASAN_ is not good prefix.

149

I think you can add

static bool break_on_interception_failure;
void __interception::SetDebugBreakOnIntercepionFailure(bool v) {
   break_on_interception_failure = v;
}

And then in asan,msan... get it from flags() and forward it here

164

On linux we print and move forward. Usually failed interceptor is no a problem as program may not even call it.
Why it needs to be DebugBreake in a first place?