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