<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 8, 2014 at 10:46 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@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><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Mon, Dec 8, 2014 at 2:00 AM, Alexander Potapenko <span dir="ltr"><<a href="mailto:glider@google.com" target="_blank">glider@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hope you're assuming there's always a single copy of common_flags in<br>
the process.<br>
This isn't the case for e.g. ASan+UBSan on Mac, but that's a broken setup.<br>
<br>
What if we let the tools protect specific flags (by adding a bool to<br>
each flag) once they set their values (when setting the default<br>
values, parsing xSAN_OPTIONS, etc)<br>
Generally, I think we need to protect each flag when anyone's trying<br>
to read it (so we'll need getters and setters for the flags), unless<br>
the code opts out of protecting a particular flag (e.g. if we want to<br>
allow it to be changed dynamically).<br>
This way we can prevent the flags from being overridden with conflicting values.<br>
<br>
Irrespective of whether we protect the flags or not, I think we need<br>
to distinguish between the flags that can be set once and flags that<br>
may change their values (e.g. on Android).<br></blockquote><div><br></div></span><div>Ideally, things that may change their values should not be flags at all. </div></div></div></div></blockquote><div><br></div><div>I think this is a good idea. We probably should enforce that flags (especially common flags)</div><div>are only allowed to be modified when they are parsed early during tool initialization by these methods:</div><div>1) SetCommonFlagsDefaults()</div><div>2) <some function/marco that would allow the tool to override some default values).</div><div>3) ParseCommonFlagsFromString()</div><div><br></div><div>After that the flags can only be accessed via common_flags(), that would return const pointer.</div><div>This would require some changes to runtime code, which seems like the Right Thing. E.g.</div><div>"allocator_may_return_null" flag should really be a member of Allocator object - we already</div><div>modify this flag in the unit tests and during ASan activation (which can introduce a data race).</div><div><br></div><div>If you agree that we should follow this path now, I will fix that. Also, do you have more</div><div>comments regarding common flags initialization in ASan/LSan/UBSan bundle I propose?</div><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><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><br>
On Sat, Dec 6, 2014 at 2:51 AM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>> wrote:<br>
> Hi all,<br>
><br>
> TL;DR<br>
> 1) We should change the way we parse common runtime flags in sanitizers.<br>
> 2) We should make ASan aware of the tools it can be combined with (LSan and<br>
> UBSan).<br>
> 3) We may have to restrict the tools UBSan can be combined with (currently<br>
> to ASan) (see [1])<br>
><br>
> Currently we have two kinds of sanitizer runtime flags: tool-specific flags<br>
> and "common flags", defined in sanitizer_common library and shared across<br>
> all the sanitizers. Many of these common flags are used early during tool<br>
> initialization (for example, we use "suppressions" flag to create<br>
> SuppressionContext singleton object). That's the reason why we parse both<br>
> tool-specific flags and common flags as early as possible at program<br>
> startup, before running the rest of initialization code. It all works fine<br>
> until we have a single sanitizer - e.g. for TSan or MSan.<br>
><br>
> The situation gets crazy when we combine multiple sanitizers in a single<br>
> process, for instance use ASan+LSan+UBSan (the default use case in some<br>
> setups). Each tool has its own defaults for common flag values, and each of<br>
> ASAN_OPTIONS, LSAN_OPTIONS and UBSAN_OPTIONS can define both tool-specific<br>
> and common flags. These environment variables are parsed at different time,<br>
> sometimes in undefined order. We can easily end up in situation where ASan<br>
> initializes some parts of sanitizer_common assuming certain values of common<br>
> runtime flags, but then these flags are overwritten by LSAN_OPTIONS. All<br>
> this is very complicated and fragile.<br>
><br>
> I propose to implement the following:<br>
><br>
> ASan flag parsing:<br>
> 1) Detect all sanitizers ASan is combined with (We know that we have LSan if<br>
> "CAN_SANITIZE_LEAKS" is true. We can detect if we have UBSan in the process<br>
> somehow.)<br>
> 2) Setup defaults for ASan-specific flags and override them from<br>
> ASAN_OPTIONS.<br>
> 3) Setup defaults for common flags and override them from all of<br>
> ASAN_OPTIONS, LSAN_OPTIONS and UBSAN_OPTIONS (if corresponding sanitizers<br>
> are enabled for the process).<br>
> 4) Proceed with initialization, and call LSan/UBSan initializers when<br>
> appropriate.<br>
><br>
> LSan/UBSan flag parsing:<br>
> 1) Learn if LSan/UBSan are combined with ASan (the "main" tool) or run as<br>
> standalone tool. This is already done for LSan and can be done for UBSan by<br>
> slightly modifying the way we structure runtimes.<br>
> 2) Setup defaults for tool-specific flags and override them from<br>
> LSAN/UBSAN_OPTIONS.<br>
> 3) If the tools run in standalone mode, setup the defaults for common flags<br>
> and override them from LSAN/UBSAN_OPTIONS. If the tools are combined with<br>
> "main" tool, do nothing.<br>
> 4) Proceed with initialization.<br>
><br>
> [1] Conjecture:<br>
> 1) We will have to add the hook to initialize UBSan that we would invoke<br>
> from ASan (much like we do for LSan). It means that we would have to<br>
> restrict the set of sanitizers that can be combined with UBSan (allowing it<br>
> for another sanitizers would be very easy, but manual process). The<br>
> alternative would be to make UBSan setup no-op except for parsing the flags,<br>
> so that UBSan initializer could be safely called before main tool<br>
> initializer.<br>
><br>
> Bonus:<br>
> 1) We will be able to significantly improve runtime flag parsing diagnostic.<br>
> For instance, we would be able to report conflicting flag definitions (e.g.<br>
> when one provides ASAN_OPTIONS=symbolize=0 LSAN_OPTIONS=symbolize=1).<br>
><br>
> Comments/objections?<br>
><br>
> --<br>
> Alexey Samsonov<br>
> <a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a><br>
><br>
</div></div>> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
><br>
<span><font color="#888888"><br>
<br>
<br>
--<br>
Alexander Potapenko<br>
Software Engineer<br>
Google Moscow<br>
</font></span></blockquote></div></div></div><br></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>