On Wed, Nov 28, 2012 at 9:09 PM, Alexey Samsonov <[email protected]>wrote:
> > On Wed, Nov 28, 2012 at 3:22 AM, Kostya Serebryany <[email protected]> wrote: > >> >> >> On Wed, Nov 28, 2012 at 10:28 AM, Alexey Samsonov <[email protected]>wrote: >> >>> >>> Ok. Kostya, what is your opinion? >>> >> >> On which part? I like the flags. >> > > Ok. > > >> As for "-mllvm -asan-foo" vs createAddressSanitizerPass flags I have >> mixed feelings. >> If we use createAddressSanitizerPass flags, what should be the behavior >> when "-mllvm -asan-foo" is given separately? >> > > I think that explicit -mllvm -asan-use-after-return=0 should override > "-fsanitize=use-after-return" option (questionable). > Anyway, I think we should discourage users to use -mllvm flags. BTW, if we > will add "-mllvm -asan-use-after-return" flag in > clang Driver, then the line "-fsanitize=use-after-return -mllvm > -asan-use-after-return=1" will produce a compiler error, stating that > "-asan-use-after-return" option can be present 0 or 1 times. So, I think > that we should follow Richard's suggestion. > > >> What are other phases doing in similar situations (or we are doing >> something unique)? >> > > GCOV profiler added in clang's BackendUtil is configured from CodeGenOpts: > if (CodeGenOpts.EmitGcovArcs || CodeGenOpts.EmitGcovNotes) { > MPM->add(createGCOVProfilerPass(CodeGenOpts.EmitGcovNotes, > CodeGenOpts.EmitGcovArcs, TargetTriple.isMacOSX())); > with the declaration: > ModulePass *createGCOVProfilerPass(bool EmitNotes = true, bool EmitData > = true, bool Use402Format = false, bool UseExtraChecksum = false); > > We may do smth similar for ASan. > Sounds great > > >> >> > >> >> --kcc >> >> >>> >>> ================ >>> Comment at: lib/Driver/Tools.cpp:1518-1523 >>> @@ -1517,1 +1517,8 @@ >>> + >>> + // 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) >>> + << describeSanitizeArg(Args, AsanArg, NeedsAsanRt) >>> + << "-fsanitize=address"; >>> } >>> ---------------- >>> Richard Smith wrote: >>> > For argument lists like "-fsanitize=use-after-return >>> -fsanitize=address -fno-sanitize=address", we'll say "-fsanitize=address is >>> only allowed with -fsanitize=address". The existing diagnostic has a >>> similar issue for "-fsanitize=address -fsanitize=alignment -fsanitize=vptr >>> -fno-sanitize=vptr", where it says "-fsanitize=vptr not allowed with >>> -fsanitize=address". I think we'd need to teach describeSanitizerArg to >>> re-parse the argument list to handle this properly. >>> RIght, the logic is untrivial here. Mailed D143 to fix this. >>> >>> ================ >>> Comment at: lib/Driver/SanitizerArgs.h:59-71 >>> @@ -58,1 +58,15 @@ >>> + >>> + // Add args for LLVM backend. >>> + if (Kind & InitOrder) { >>> + CmdArgs.push_back("-mllvm"); >>> + CmdArgs.push_back("-asan-initialization-order"); >>> + } >>> + if (Kind & UseAfterReturn) { >>> + CmdArgs.push_back("-mllvm"); >>> + CmdArgs.push_back("-asan-use-after-return"); >>> + } >>> + if (Kind & UseAfterScope) { >>> + CmdArgs.push_back("-mllvm"); >>> + CmdArgs.push_back("-asan-use-lifetime"); >>> + } >>> } >>> ---------------- >>> Richard Smith wrote: >>> > I would prefer this to be handled by the frontend instead of by the >>> driver (the frontend is responsible for adding all the other IR >>> instrumentation, including adding the ASan passes). >>> > >>> > Have you considered passing these flags to ASan when creating the >>> passes in addAddressSanitizerPass, rather than as command-line options? >>> Hm, passing arguments to createAddressSanitizerPass() certainly seems a >>> better (though, more intrusive) solution than playing with -mllvm flags. >>> I'll work on that. >>> >>> >>> http://llvm-reviews.chandlerc.com/D142 >>> >> >> > > > -- > Alexey Samsonov, MSK > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
