Hi, On Wed, Feb 11, 2015 at 8:10 PM, Filipe Cabecinhas < [email protected]> wrote:
> Hi Alexey, > > On Wed, Feb 11, 2015 at 6:59 PM, Alexey Samsonov <[email protected]> > 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 < >> [email protected]> 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 >> [email protected] >> > > -- Alexey Samsonov [email protected]
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
