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

Filipe Cabecinhas filcab+llvm.phabricator at gmail.com
Wed Feb 11 20:10:24 PST 2015


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.

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).

  - 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150211/824e6375/attachment.html>


More information about the cfe-commits mailing list