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. > > > > --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
