<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 28, 2015 at 10:23 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">вт, 28 апр. 2015 г. в 20:03, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>>:<span class=""><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 8:38 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.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"><div class="gmail_extra"><br><div class="gmail_quote"><span>On Sat, Apr 11, 2015 at 2:36 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"><p dir="ltr">__attribute__((no_instrumentation)) ?  Sounds like a perfect fit for the job</p></blockquote><div><br></div></span><div>However, IIRC, Clang folks wanted positive attributes ("sanitize_address", "sanitize_memory" etc.) on LLVM</div><div>functions, not sure "no_instrumentation" will be accepted. Otherwise we will have to emit "sanitize_coverage"</div><div>attribute on all functions that need to be instrumented, and that would be somewhat unfortunate.</div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So, essentially you need an attribute to disable coverage instrumentation of a particular function</div><div>(as ASan instrumentation is already disabled), right?</div></div></div></div></blockquote><div><br></div></span><div>Yes, or...</div><div>What I really want is a way to tell the compiler "hey, I don't want this function to do any memory accesses other than those required by its code".</div><div>Do you think we should maybe have an attribute for exactly this purpose?</div></div></div></blockquote><div><br></div><div>Oh, so this would be not really relevant to sanitizers. This attribute makes sense, but... how would you enforce it? It will have to be supported by</div><div>all sanitizers, coverage instrumentation, profiling, and maybe even optimization passes.</div><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_quote"><div><div class="h5"><div><br></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"><div>I was trying to make the point that you can add</div><div>Clang's __attribute__((no_sanitize_coverage)), and that would be aligned with attributes we already have,</div><div>but LLVM function attribute should be "positive": so you will have to attach "sanitize_coverage" attributes</div><div>to all the LLVM functions you want to include in coverage.</div><div><br></div><div>This can be tricky, as we might not see missing attributes, and don't notice that some functions</div><div>(e.g. compiler-generated ones) are excluded from coverage.</div><div><br></div><div>However, I'm OK if you decide to take this path - once again, it will be consistent with current</div><div>state of affairs with ASan/TSan/MSan.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><div> <span style="color:rgb(80,0,80)"> </span></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"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><div class="gmail_quote">сб, 11 апр. 2015, 21:58, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>>:<div><div><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></div></div>
</blockquote></div></div><span><font color="#888888"><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>
</font></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></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div>