[cfe-commits] [PATCH] Clang support for MemorySanitizer

[Evgeniy Stepanov]

Done and committed. I also noticed that we do not catch conflicts between
memory and other sanitizers.

Let me know if you are not comfortable with a bit of code duplication
there. I think that N*(N-1) explicit checks (for N=3) is still better and
more readable than generic code for arbitrary number of sanitizers.


On Wed, Dec 5, 2012 at 2:35 AM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Tue, Dec 4, 2012 at 3:04 AM, Evgeniy Stepanov <eugenis at google.com>wrote:
>
>> Nice. Thanks for the pointer.
>> Does the attached patch look good?
>>
>
> Looks fine. Presumably a test needs updating too?
>
>
>> On Tue, Dec 4, 2012 at 2:00 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> On Fri, Nov 30, 2012 at 3:01 AM, Chandler Carruth <chandlerc at gmail.com>wrote:
>>>
>>>>
>>>>
>>>> ================
>>>> Comment at:
>>>> llvm/tools/clang/include/clang/Basic/DiagnosticDriverKinds.td:104-107
>>>> @@ -103,2 +103,6 @@
>>>>    "AddressSanitizer on Android requires '-pie'">;
>>>> +def err_drv_tsan_requires_pie : Error<
>>>> +  "ThreadSanitizer requires '-pie'">;
>>>> +def err_drv_msan_requires_pie : Error<
>>>> +  "MemorySanitizer requires '-pie'">;
>>>>  def err_drv_unknown_objc_runtime : Error<
>>>> ----------------
>>>> Evgeniy Stepanov wrote:
>>>> > Dmitri Gribenko wrote:
>>>> > > Chandler Carruth wrote:
>>>> > > > You don't need two diagnostics:
>>>> > > >
>>>> > > >   def err_drv_sanitizer_requires_pie : Error<
>>>> > > >     "%select{thread|memory}0 sanitizer requires the '-pie'
>>>> option">;
>>>> > > I think it is better to use two diagnostics since %select would
>>>> require magic numbers in the source.
>>>> > I also prefer two diagnosticts.
>>>> >
>>>> > Btw, %select{a|b}0 evaluates to "b" if true, and "a" if false. I find
>>>> this _really_ weird.
>>>> >
>>>> I don't like that pattern any more than you do, but Clang's diagnostics
>>>> use it *extensively*, adding a comment next to the magic number indicating
>>>> which is which.
>>>>
>>>> I also dislike the duplicated text intensely.
>>>>
>>>> A good solution would be something like this:
>>>>
>>>>   def err_drv_sanitizer_requires_pie : Error<
>>>>     "%select{thread|memory}0 sanitizer requires the '-pie' option">,
>>>>     SelectNames<0, { SDK_Thread, SDK_Memory }>;
>>>>
>>>> Where this automatically teaches the diagnostic engine that select #0
>>>> should accept a specific enum rather than an unsigned, and defines that
>>>> enum in the diag namespace with the given enumerators.
>>>>
>>>> But this would be a huge (good) change and shouldn't be in this patch.
>>>> I think I mildly prefer consistency with other diagnostics until we get
>>>> something like this.
>>>
>>>
>>> We already have diag::err_drv_argument_only_allowed_with for indicating
>>> that one argument requires another.
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121205/9a72cc35/attachment.html>