On Wed, Nov 28, 2012 at 9:17 AM, Kostya Serebryany <[email protected]> wrote:
> > > 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 > Mailed this change for review in D145. > >> >>> >>> >> >>> >>> --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 >> >> > -- Alexey Samsonov, MSK
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
