[PATCH] Implement no_sanitize attribute.

Peter Collingbourne peter at pcc.me.uk
Thu May 14 13:36:56 PDT 2015


On Mon, May 11, 2015 at 02:23:46PM -0400, Aaron Ballman wrote:
> > +def NoSanitize : InheritableAttr {
> > +  let Spellings = [GNU<"no_sanitize">];
> 
> Why not a C++ spelling as well?

Makes sense, done.

> > +  let Subjects = SubjectList<[Function], ErrorDiag>;
> 
> What about other subjects, such as ObjC methods?

Added ObjC methods.

> Do we also want to deprecate the existing attributes while we're at it?

Probably not. We'll still need them for GCC compatibility.

> > Index: lib/CodeGen/CodeGenFunction.cpp
> > ===================================================================
> > --- lib/CodeGen/CodeGenFunction.cpp
> > +++ lib/CodeGen/CodeGenFunction.cpp
> > @@ -608,6 +608,26 @@
> >    if (CGM.isInSanitizerBlacklist(Fn, Loc))
> >      SanOpts.clear();
> >
> > +  if (D) {
> 
> Is it intentional that the old code only added these attributes if the
> function was not in the sanitizer black list, and the new code always
> adds them?

See above -- we clear SanOpts if the function is in the blacklist.

> > +    // Apply the no_sanitize* attributes to SanOpts.
> > +    if (auto NoSanAttr = D->getAttr<NoSanitizeAttr>())
> > +      SanOpts.Mask &= ~NoSanAttr->getMask();
> > +    if (D->hasAttr<NoSanitizeAddressAttr>())
> > +      SanOpts.set(SanitizerKind::Address, false);
> > +    if (D->hasAttr<NoSanitizeThreadAttr>())
> > +      SanOpts.set(SanitizerKind::Thread, false);
> > +    if (D->hasAttr<NoSanitizeMemoryAttr>())
> > +      SanOpts.set(SanitizerKind::Memory, false);
> 
> I think that the NoSanitize-foo semantic attributes should be removed
> and specified in terms of NoSanitizeAttr. The attributes should still
> be parsed as alternate spellings of no_sanitize.

It isn't clear that they can be parsed as alternate spellings because they
take different numbers of arguments. It seems simplest to keep the semantic
attributes, given that we don't plan to introduce any more of them.

Thanks,
-- 
Peter



More information about the cfe-commits mailing list