[cfe-commits] [PATCH] Add -fsanitize= command line argument to control -fcatch-undefined-behavior, -faddress-sanitizer, and -fthread-sanitizer

[Richard Smith]

On Fri, Nov 2, 2012 at 6:06 AM, Kostya Serebryany <kcc at google.com> wrote:
> Assuming the implemented flag naming is ok with everyone, the patch is good.
> Few questions:
>
> +  unsigned Kind;
>    Do we only allow 32 sanitizers? Why not 64?

We currently have only 13. I don't think we'll forget to update this
if we go past 32 sanitizers: we won't pass the relevant flag on to
-cc1 (so sanitizers past the first 32 will not work at all and tests
will fail), and we will also get a compiler diagnostic in self-host,
at least; we'll probably also get one from g++.

> Do we want to change -faddress-sanitizer in asan/tsan lit tests?
>   projects/compiler-rt/lib/asan/lit_tests
>   projects/compiler-rt/lib/tsan/lit_tests
>
> How long do we keep the aliases (-faddress-sanitizer => fsanitizer=address)?
> (I like the old name more than the new name, but having two names is
> confusing)

Just so we have something specific to talk about, here are some options:

1) For the 3.2 release, we remove the existing switches.
2) For the 3.2 release, we keep the existing switches, and produce a
deprecation warning if they are used. At some point between 3.2 and
3.3, once you think most of your users have transitioned, we remove
the aliases.
3) We keep the compatibility switches for the time being (and thus
probably forever).

I think my preference is (2).

> Will this allow to easily add *san-specific subflags?
> E.g. --sanitizer=address,global-init-order,use-after-return,use-after-scope

Yes, this makes perfect sense to me. Additionally, you could break up
'address' into subflags too, and make 'address' be an alias for them
all. The intent is for -fsanitizer= flags to act essentially like -W
flags in this regard.

> or --sanitizer=memory,memory-origins

I'm hesitant about this one. Following the warning flags analogy, this
seems analogous to a -fdiagnostic option rather than a -W option.

> (The first one indicates 3 asan sub passes that are curretly off by default,
> the second one indicates that msan should be run with "track origins"
> feature)

-- 
Richard

> On Fri, Nov 2, 2012 at 10:03 AM, Kostya Serebryany <kcc at google.com> wrote:
>>
>>
>>
>> On Fri, Nov 2, 2012 at 9:41 AM, Alexander Potapenko <glider at google.com>
>> wrote:
>>>
>>> I haven't seen the patch yet, but here are two thoughts:
>>> - some tools may be incompatible with each other (e.g. ASan and TSan), so
>>> we shouldn't allow to use them together;
>>> - there are many users of TSan and ASan, thus we can't easily rename the
>>> corresponding command line options. The right thing to do is to make
>>> -fsanitize fully functional, announce that and then remove the existing
>>> flags.
>>> I'm also unsure whether having such an umbrella flag for such different
>>> tools won't confuse the users.
>>
>> As Richard says, the old flags remain (they become synonyms of the new
>> flags). That sounds fine.
>> I'll check the patches later today.
>>
>> --kcc
>>
>>>
>>> On Nov 2, 2012 4:51 AM, "Richard Smith" <richard at metafoo.co.uk> wrote:
>>>>
>>>> Hi,
>>>>
>>>> The attached patch series adds a
>>>> -fsanitize=sanitizer1,sanitizer2,sanitizer3 command-line option to
>>>> Clang (and a corresponding -fno-sanitize=...). This works as follows
>>>> for the driver options:
>>>>
>>>>   -faddress-sanitizer becomes a synonym for -fsanitize=address
>>>>   -fno-address-sanitizer becomes a synonym for -fno-sanitize=address
>>>>   -fthread-sanitizer becomes a synonym for -fsanitize=thread
>>>>   -fno-thread-sanitizer becomes a synonym for -fno-sanitize=thread
>>>>   -fcatch-undefined-behavior becomes a synonym for -fsanitize=undefined
>>>>   -fsanitize=undefined is expanded by the driver into a list of
>>>> individual undefined behavior sanitizers
>>>>
>>>> The -cc1 interface is deliberately very simplistic: it loses support
>>>> for the existing flags, and instead only supports -fsanitize=..., and
>>>> only supports naming the individual checks (and not groups like
>>>> -fsanitize=undefined).
>>>>
>>>> Patch 1 just renames the existing ASan and TSan LangOptions to follow
>>>> the naming convention used by the later patches.
>>>> Patch 2 adds support to the driver and frontend for parsing
>>>> -fsanitize, converting the other options to it, and filling in
>>>> LangOptions.
>>>> Patch 3 uses the -fsanitize=... values to enable/disable the existing
>>>> -fcatch-undefined-behavior checks, and removes frontend support for
>>>> the other options.
>>>>
>>>> Please review! I'd like to get this in for the 3.2 release.
>>>>
>>>> Thanks!
>>>> Richard
>>
>>
>