<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 25, 2013 at 1:12 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@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 class="HOEnZb"><div class="h5">On Mon, Jan 21, 2013 at 12:35 PM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>> wrote:<br>
><br>
> On Mon, Jan 21, 2013 at 9:56 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> On Mon, Jan 21, 2013 at 8:28 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>><br>
>> wrote:<br>
>> > Hi rsmith, kcc,<br>
>> ><br>
>> > This patch changes the behavior of Clang driver for command line like<br>
>> > "clang++ -fsanitize=init-order a.cc": now it ignores<br>
>> > -fsanitize=init-order argument and<br>
>> > warns that it's unused w/o "-fsanitize=address" (instead of reporting an<br>
>> > error).<br>
>><br>
>> So if I understand correctly, the same warning is produced for all<br>
>> these variants:<br>
>> -fsanitize=init-order<br>
>> -fno-sanitize=address -fsanitize=init-order<br>
>> -fsanitize=address,init-order -fno-sanitize=address<br>
><br>
><br>
> Yes.<br>
><br>
>><br>
>><br>
>> Could we specifically special case for situations like the last one?<br>
>> (or at least the last two) & make those silent (no warning) while<br>
>> possibly still producing an error for the first? Maybe that's<br>
>> difficult with our options parsing & not worth the hassle but I<br>
>> figured I'd check/ask.<br>
><br>
><br>
> Hm, interesting. We can somehow hack sanitizer option parsing to<br>
> deal with this and clearly distinguish the first and the last case, but<br>
> we deviation from current behavior ("-fsanitize" enables some set of<br>
> sanitizers,<br>
> "-fno-sanitize'" disables some set) may make things less clear.<br>
> E.g. if "-fsanitize=address,init-order -fno-sanitize=address" is "no<br>
> sanitizing",<br>
<br>
</div></div>(excuse the delay - dropped this draft somewhere along the way)<br>
<br>
Sorry, I didn't mean to suggest a change in behavior - perhaps I<br>
misunderstood your warning/error.<br>
<br>
What I was assuming was that -fsanitize=init-order requires<br>
-fsanitize=address, right?</blockquote><div><br></div><div style>Right.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In that case, -fsanitize=address,init-order<br>
-fno-sanitize=address would be good if it was silent (essentially:<br>
disabling -fsanitize-address would disable any<br>
subfeatures/dependencies of it)<br></blockquote><div><br></div><div style>Yeah, I find this pretty reasonable. I've updated the patch</div><div style>to silently accept this.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> then what should "-fsanitize=address,init-order -fno-sanitize=address<br>
> -fsanitize=address"<br>
> expand into?<br>
<br>
</div>Fair question - I suppose it'd still be best to have that turn on both<br>
address and init-order.<br>
<br>
(but my points here are fairly hypothetical, I assume it's not worth<br>
going to this level of detail for this issue - but I don't know the<br>
use cases well enough to judge that)<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
>><br>
>><br>
>> > As a rationale, consider the case when one wants to optionally build a<br>
>> > large project<br>
>> > with ASan, but disable instrumentation for specific files. Previously,<br>
>> > adding<br>
>> > "-fno-sanitize=address" to a file-specific compile options would produce<br>
>> > an error when building the whole project with<br>
>> > "-fsanitize=address,init-order",<br>
>> > so we'd have to go and fix all "-fno-sanitize=..." occurences to contain<br>
>> > all possible features of ASan.<br>
>> ><br>
>> > <a href="http://llvm-reviews.chandlerc.com/D315" target="_blank">http://llvm-reviews.chandlerc.com/D315</a><br>
>> ><br>
>> > Files:<br>
>> > lib/Driver/Tools.cpp<br>
>> > lib/Driver/SanitizerArgs.h<br>
>> > test/Driver/fsanitize.c<br>
>> > include/clang/Basic/DiagnosticGroups.td<br>
>> > include/clang/Basic/DiagnosticDriverKinds.td<br>
>> ><br>
>> > Index: lib/Driver/Tools.cpp<br>
>> > ===================================================================<br>
>> > --- lib/Driver/Tools.cpp<br>
>> > +++ lib/Driver/Tools.cpp<br>
>> > @@ -1475,10 +1475,10 @@<br>
>> ><br>
>> > // If -fsanitize contains extra features of ASan, it should also<br>
>> > // explicitly contain -fsanitize=address.<br>
>> > - if (NeedsAsan && ((Kind & Address) == 0))<br>
>> > - D.Diag(diag::err_drv_argument_only_allowed_with)<br>
>> > - << lastArgumentForKind(D, Args, NeedsAsanRt)<br>
>> > - << "-fsanitize=address";<br>
>> > + if (((Kind & Address) == 0) && ((Kind & AddressFull) != 0))<br>
>> > + D.Diag(diag::warn_drv_unused_sanitizer)<br>
>> > + << lastArgumentForKind(D, Args, AddressFull)<br>
>> > + << "-fsanitize=address";<br>
>> ><br>
>> > // Parse -f(no-)sanitize-blacklist options.<br>
>> > if (Arg *BLArg = Args.getLastArg(options::OPT_fsanitize_blacklist,<br>
>> > Index: lib/Driver/SanitizerArgs.h<br>
>> > ===================================================================<br>
>> > --- lib/Driver/SanitizerArgs.h<br>
>> > +++ lib/Driver/SanitizerArgs.h<br>
>> > @@ -33,7 +33,7 @@<br>
>> > #define SANITIZER(NAME, ID) ID = 1 << SO_##ID,<br>
>> > #define SANITIZER_GROUP(NAME, ID, ALIAS) ID = ALIAS,<br>
>> > #include "clang/Basic/Sanitizers.def"<br>
>> > - NeedsAsanRt = AddressFull,<br>
>> > + NeedsAsanRt = Address,<br>
>> > NeedsTsanRt = Thread,<br>
>> > NeedsMsanRt = Memory,<br>
>> > NeedsUbsanRt = (Undefined & ~Bounds) | Integer<br>
>> > Index: test/Driver/fsanitize.c<br>
>> > ===================================================================<br>
>> > --- test/Driver/fsanitize.c<br>
>> > +++ test/Driver/fsanitize.c<br>
>> > @@ -31,7 +31,10 @@<br>
>> > // CHECK-ASAN-TSAN: '-faddress-sanitizer' not allowed with<br>
>> > '-fthread-sanitizer'<br>
>> ><br>
>> > // RUN: %clang -target x86_64-linux-gnu -fsanitize=init-order %s -###<br>
>> > 2>&1 | FileCheck %s --check-prefix=CHECK-ONLY-EXTRA-ASAN<br>
>> > -// CHECK-ONLY-EXTRA-ASAN: argument '-fsanitize=init-order' only allowed<br>
>> > with '-fsanitize=address'<br>
>> > +// CHECK-ONLY-EXTRA-ASAN: '-fsanitize=init-order' is ignored in absence<br>
>> > of '-fsanitize=address'<br>
>> > +<br>
>> > +// RUN: %clang -target x86_64-linux-gnu -Wno-unused-sanitize-argument<br>
>> > -fsanitize=init-order %s -### 2>&1 | FileCheck %s<br>
>> > --check-prefix=CHECK-WNO-UNUSED-SANITIZE-ARGUMENT<br>
>> > +// CHECK-WNO-UNUSED-SANITIZE-ARGUMENT-NOT: '-fsanitize=init-order' is<br>
>> > ignored in absence of '-fsanitize=address'<br>
>> ><br>
>> > // RUN: %clang -target x86_64-linux-gnu -fsanitize-memory-track-origins<br>
>> > -pie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ONLY-TRACK-ORIGINS<br>
>> > // CHECK-ONLY-TRACK-ORIGINS: warning: argument unused during<br>
>> > compilation: '-fsanitize-memory-track-origins'<br>
>> > Index: include/clang/Basic/DiagnosticGroups.td<br>
>> > ===================================================================<br>
>> > --- include/clang/Basic/DiagnosticGroups.td<br>
>> > +++ include/clang/Basic/DiagnosticGroups.td<br>
>> > @@ -270,7 +270,9 @@<br>
>> > def UnnamedTypeTemplateArgs : DiagGroup<"unnamed-type-template-args",<br>
>> ><br>
>> > [CXX98CompatUnnamedTypeTemplateArgs]>;<br>
>> > def UnusedArgument : DiagGroup<"unused-argument">;<br>
>> > -def UnusedCommandLineArgument :<br>
>> > DiagGroup<"unused-command-line-argument">;<br>
>> > +def UnusedSanitizeArgument : DiagGroup<"unused-sanitize-argument">;<br>
>> > +def UnusedCommandLineArgument :<br>
>> > DiagGroup<"unused-command-line-argument",<br>
>> > + [UnusedSanitizeArgument]>;<br>
>> > def UnusedComparison : DiagGroup<"unused-comparison">;<br>
>> > def UnusedExceptionParameter : DiagGroup<"unused-exception-parameter">;<br>
>> > def UnneededInternalDecl : DiagGroup<"unneeded-internal-declaration">;<br>
>> > Index: include/clang/Basic/DiagnosticDriverKinds.td<br>
>> > ===================================================================<br>
>> > --- include/clang/Basic/DiagnosticDriverKinds.td<br>
>> > +++ include/clang/Basic/DiagnosticDriverKinds.td<br>
>> > @@ -123,6 +123,8 @@<br>
>> > def warn_drv_empty_joined_argument : Warning<<br>
>> > "joined argument expects additional value: '%0'">,<br>
>> > InGroup<UnusedCommandLineArgument>;<br>
>> > +def warn_drv_unused_sanitizer : Warning<"'%0' is ignored in absence of<br>
>> > '%1'">,<br>
>> > + InGroup<UnusedSanitizeArgument>;<br>
>> > def warn_drv_clang_unsupported : Warning<<br>
>> > "the clang compiler does not support '%0'">;<br>
>> > def warn_drv_deprecated_arg : Warning<<br>
>> ><br>
>> > _______________________________________________<br>
>> > cfe-commits mailing list<br>
>> > <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>> ><br>
><br>
><br>
><br>
><br>
> --<br>
> Alexey Samsonov, MSK<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>