<p dir="ltr">__attribute__((no_instrumentation)) ? Sounds like a perfect fit for the job</p>
<br><div class="gmail_quote">сб, 11 апр. 2015, 21:58, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Apr 11, 2015 at 11:33 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote">сб, 11 апр. 2015 г. в 20:17, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>>:<span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think that using blacklist to disable instrumentation because of sandbox is a pretty gross hack, and<div>the wrong thing to do. I think the better "solution" would be to move sandbox-related code to a different</div><div>source files, and compile them with -fno-sanitize=all. Then you can also compile them with -fsanitize-coverage=0,</div><div>and the problem would be solved.</div></div></blockquote><div><br></div></span><div>I don't agree this is a better solution.</div><div>Changing the file/code structure of a project to please a deficiency of every tool we want to use doesn't scale in general.</div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Fair enough, but I'd argue we should have added __attribute__((no_sanitize_address)) to these functions instead of blacklisting them.</div><div>I'm OK with implementing similar attribute for coverage.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>For example, Chromium's sandbox/ code depends on base/ and we do blacklist some code from base/. Some of that code is located in a header file...</div><div>I doubt sufficiently changing the file structure would be welcome.</div><div>I'm not a GYP expert, so the thought of passing an extra compiler flag for select TUs scares me as well.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"></div></div></div></div><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote">On Fri, Apr 10, 2015 at 12:16 PM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">(After looking at the code)<br>Is it fine to just return from SanitizerCoverageModule::runOnFunction if a given function doesn't have one of the Sanitize{Address,Thread,Memory} attributes for the "current" sanitizer set?</div></blockquote><div><br></div></div></div></div></div><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div>No. Coverage is also supposed to work with DFSan, and with UBSan (the latter doesn't add any attributes to LLVM functions).</div></div></div></div></div></blockquote><div><br></div></span><div>Alright, can we support that exclusion just for ASan for the time being?</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Logic could be like that:</div><div>– instrument all the code if we're only doing coverage [w/o asan,msan,tsan]</div><div>– instrument all the non-blacklisted code, use the same instrument/no-instrument attribute as the "main" sanitizer being used</div><div><br></div><div>WDYT?</div><div><br><div class="gmail_quote">пт, 10 апр. 2015 г. в 21:32, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>>:</div><div><div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">... especially since the blacklist option is called -fsanitize-blacklist.<br>Just looking at its name, I'd had assumed it affected -fsanitize-coverage too.</div></blockquote></div></div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div>I'd try to abstain from using blacklist for coverage instrumentation. If we would still definitely have to add it for</div><div>some reason (see above for suggestion regarding sandbox issue in Chrome), we can use categories:</div><div><br></div><div>src:*/dont/use/coverage.cc=coverage</div><div>fun:DontUseCoverage=coverage</div></div></div></div></div><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><div class="gmail_quote">пт, 10 апр. 2015 г. в 21:08, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>>:</div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Kostya, Alexey,<div><br></div><div>I'm currently working on sanitizer coverage on Windows for Chromium.</div><div>As described in <a href="http://crbug.com/459166" target="_blank">http://crbug.com/459166</a> , we currently blacklist some sandbox code from ASan instrumentation, otherwise shadow memory is accessed before the RTL is even loaded.</div><div>As it turns out, we should also disable coverage instrumentation for that code, otherwise the guard variables are checked before the RTL is loaded.</div><div><br></div><div>That being said, the ASan blacklist should probably be used in the coverage instrumentation module pass. Or it should have a separate blacklist...</div><div><br></div><div>What do you think?</div><div>––</div></div><div dir="ltr"><div>Timur</div></div></blockquote></div></blockquote></div></div></div></div></div>
</blockquote></div></div></div></div><div dir="ltr"><div><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div></div></blockquote></span></div></div>
</blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div></blockquote></div>