[cfe-commits] [PATCH] Add three new sanitizers depending on ASan.

[Alexey Samsonov]

On Wed, Nov 28, 2012 at 9:17 AM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Wed, Nov 28, 2012 at 9:09 PM, Alexey Samsonov <samsonov at google.com>wrote:
>
>>
>> On Wed, Nov 28, 2012 at 3:22 AM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>>
>>>
>>> On Wed, Nov 28, 2012 at 10:28 AM, Alexey Samsonov <samsonov at google.com>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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121128/a021e839/attachment.html>