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

[Evgeniy Stepanov]

Nice. Thanks for the pointer.
Does the attached patch look good?



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/20121204/f03e5a0a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.patch
Type: application/octet-stream
Size: 2105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121204/f03e5a0a/attachment.obj>