This is an archive of the discontinued LLVM Phabricator instance.

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

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



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.Sep 3 2020, 11:43 AM
mcgov planned changes to this revision.
mcgov created this revision.
mcgov updated this revision to Diff 289776.Sep 3 2020, 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.Sep 3 2020, 11:59 AM

removed whitespace diff

vitalybuka added inline comments.Sep 4 2020, 5:20 AM

can we use sanitizer_common flags?

mcgov added inline comments.Sep 4 2020, 10:48 AM

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.Sep 6 2020, 9:08 PM

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

vitalybuka added inline comments.Sep 16 2020, 1:53 PM

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


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


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?

mcgov added inline comments.Sep 24 2020, 9:07 AM

I think the issue isn't the init order, just adding the common_flags symbol to a library that previously didn't use it.


This is great, thank you! I'll give it a shot.


It doesn't really need to be one and I like the idea of just printing something and moving forward. The current failure mode is to crash, which prompts the user to investigate in a debugger to get this output spew. Just printing out a warning would be better, I'll look into it before testing that other change above.

rnk added a comment.Oct 29 2020, 4:11 PM

I'm in favor of the direction of the change, too, fwiw.

vitalybuka requested changes to this revision.Feb 17 2021, 1:56 PM
This revision now requires changes to proceed.Feb 17 2021, 1:56 PM