[PATCH] Change how we deal with (implicit) -fno-rtti + -fsanitize=vptr

[Alexey Samsonov]

Hi,

On Wed, Feb 11, 2015 at 8:10 PM, Filipe Cabecinhas <
filcab+llvm.phabricator at gmail.com> wrote:

> Hi Alexey,
>
> On Wed, Feb 11, 2015 at 6:59 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
>
>> Note that currently types::IsCXX(InputType) is not used to determine
>> -fsanitize flag values at all - we just look at the explicit flags (-frtti,
>> -fno-rtti, -fapple-kext, -mkernel), and at the Triple (is it PS4?). Does it
>> make sense to leave it that way? There is a special case:
>> a) we're on PS4
>> b) we have a CXX input
>> c) we've provided -fexceptions, but haven't provided -frtti
>> In this case RTTI will still be implicitly enabled, but we would disable
>> vptr sanitizer earlier at this point.
>>
> I'm fine with allowing this (it's not perfect, but it makes it much easier
> to deal with this edge case).
>
> I'd say it's fine to sacrifice this special case :) (for that matter, to
>> simplify things we could even make PS4 driver to issue a hard error if one
>> is building CXX code with explicit -fexceptions but no explicit -frtti).
>>
> Unfortunately, we can't change that behavior. -fexceptions in C++ mode
> _has_ to turn on rtti, even without an explicit -frtti.
>

I see, you probably already have a bunch of build rules that depend on that.


> That is, if we only look at Triple and input arguments, we can probably
>> have ToolChain::getRTTIMode() much like we have
>> ToolChain::getSanitizerArgs(), and use former in the latter.
>>
> My problem is: If we make a getRTTIMode() that only looks at the triple
> and args, it will have to, either (taking as an example, c++ mode +
> -fexceptions):
>
>   - Not be true all the time (It would return “disabled”, but then, when
> we notice that we have IsCXX() and -fexceptions, it would be wrong, because
> RTTI would then be turned on, which would make that value wrong for all the
> calls after that).
>

I don't see anything except vptr-sanitizer that should depend on the true
information of whether we have RTTI enabled (that is, currently all the
calculations you do inside Clang::ConstructJob are used to solely decide if
we need to provide -fno-rtti flag, and are not reused afterwards). Maybe,
we can get away with getRTTIMode(), and then, if we're on PS4 + CXX input +
-fexceptions, we'll just say "no, we will override it and enable RTTI
anyway".


>
>   - Have to have its value cached in ToolChain (or elsewhere), and have a
> way to directly manipulate that cache when we finally figure out we want
> RTTI enabled (by calling some method to tell the cache that we actually
> want RTTI).
>
> I'm not really fond of these, but I think the second one wouldn't be _too_
> bad.
>
> What do you think?
>
>   Filipe
>
>
> Does it all make sense?
>>
>>
>> On Wed, Feb 11, 2015 at 6:08 PM, Filipe Cabecinhas <
>> filcab+llvm.phabricator at gmail.com> wrote:
>>
>>> This is getting a bit hard to reason about.
>>>
>>> Here are the pieces:
>>>
>>> - Default RTTI behavior for target (depends on other arguments like the
>>> type of input file)
>>> - RTTI related options (explicit)
>>> - Sanitizer options (explicit (vptr) or implicit (undefined))
>>>
>>> SanitizerArgs _wants_ to hide details like “which sanitizers exist/are
>>> enabled”, which means it will have to sort out whether to warn/error or not
>>> by itself (it needs to know if there's an explicit RTTI-related argument) .
>>>
>>> The RTTI-related code needs to know if C++ exceptions are on to decide
>>> if it implicitly turns on RTTI or not (this implies knowing not just the
>>> arguments, but also the input file).
>>>
>>> To decide whether we need additional args for RTTI or not, and which
>>> ones to pass, we need to know the RTTI-related arguments passed and the
>>> input type (actually, we need IsCXX(Input)).
>>>
>>> Easiest (kind of hacky) solution: Add enough arguments to
>>> getSanitizerArgs and to the SanitizerArgs constructor to allow it to decide
>>> whether to warn/error due to the vptr sanitizer (We'd parse the
>>> RTTI-related args, and use those results
>>> (enabled/disabledexplicitly/disabledimplicitly + an RTTI-related arg, if
>>> necessary) when calling getSanitizerArgs()).
>>>
>>> Easy (kind of hacky) solution (with a major drawback): Make it easy to,
>>> via SanitizerArgs, add/remove sanitizers. It would be hard to know if the
>>> vptr sanitizer was enabled explicitly or via -fsanitize=undefined, with
>>> this solution (we'd have to re-parse everything, if we wanted to keep it
>>> working that way).
>>>
>>> Non-hacky solutions: Sorry, couldn't think of one. Additional ones
>>> included stashing stuff in the ToolChain object… It doesn't seem like it's
>>> the proper thing to do.
>>>
>>>
>>> http://reviews.llvm.org/D7525
>>>
>>> EMAIL PREFERENCES
>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>
>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>
>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150212/23847606/attachment.html>