[PATCH] Implement no_sanitize attribute.
Aaron Ballman
aaron at aaronballman.com
Thu May 14 14:06:22 PDT 2015
On Thu, May 14, 2015 at 4:36 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> - Introduce VariadicStringArgument, start using it for no_sanitize; support ObjC methods
> - Add support for C++11 attribute; fix multiple no_sanitize attribute case
>
>
> http://reviews.llvm.org/D9631
>
> Files:
> include/clang/AST/Attr.h
> include/clang/Basic/Attr.td
> include/clang/Basic/AttrDocs.td
> lib/CodeGen/CodeGenFunction.cpp
> lib/CodeGen/CodeGenModule.cpp
> lib/Sema/SemaDeclAttr.cpp
> test/CodeGen/address-safety-attr.cpp
> test/CodeGen/sanitize-thread-attr.cpp
> test/CodeGenCXX/cfi-vcall.cpp
> test/CodeGenObjC/no-sanitize.m
> test/SemaCXX/attr-no-sanitize.cpp
> utils/TableGen/ClangAttrEmitter.cpp
> Index: include/clang/AST/Attr.h
> ===================================================================
> --- include/clang/AST/Attr.h
> +++ include/clang/AST/Attr.h
> @@ -20,6 +20,7 @@
> #include "clang/AST/Type.h"
> #include "clang/Basic/AttrKinds.h"
> #include "clang/Basic/LLVM.h"
> +#include "clang/Basic/Sanitizers.h"
> #include "clang/Basic/SourceLocation.h"
> #include "clang/Basic/VersionTuple.h"
> #include "llvm/ADT/SmallVector.h"
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -144,6 +144,7 @@
> class UnsignedArgument<string name, bit opt = 0> : Argument<name, opt>;
> class VariadicUnsignedArgument<string name> : Argument<name, 1>;
> class VariadicExprArgument<string name> : Argument<name, 1>;
> +class VariadicStringArgument<string name> : Argument<name, 0>;
This should be set to Argument<name, 1> since a variadic string could
potentially be empty. We can add the nonempty logic to the handler.
>
> // A version of the form major.minor[.subminor].
> class VersionArgument<string name, bit opt = 0> : Argument<name, opt>;
> @@ -1386,6 +1387,24 @@
> let Documentation = [Undocumented];
> }
>
> +def NoSanitize : InheritableAttr {
> + let Spellings = [GNU<"no_sanitize">, CXX11<"clang", "no_sanitize">];
> + let Args = [VariadicStringArgument<"Sanitizers">];
> + let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;
> + let Documentation = [NoSanitizeDocs];
> + let AdditionalMembers = [{
> + SanitizerMask getMask() const {
> + SanitizerMask Mask = 0;
> + for (auto SanitizerName : sanitizers()) {
> + SanitizerMask ParsedMask =
> + parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);
> + Mask |= expandSanitizerGroups(ParsedMask);
> + }
> + return Mask;
> + }
> + }];
> +}
> +
> // Attribute to disable AddressSanitizer (or equivalent) checks.
> def NoSanitizeAddress : InheritableAttr {
> let Spellings = [GCC<"no_address_safety_analysis">,
> Index: include/clang/Basic/AttrDocs.td
> ===================================================================
> --- include/clang/Basic/AttrDocs.td
> +++ include/clang/Basic/AttrDocs.td
> @@ -920,6 +920,19 @@
> }];
> }
>
> +def NoSanitizeDocs : Documentation {
> + let Category = DocCatFunction;
> + let Content = [{
> +Use the ``no_sanitize`` attribute on a function declaration to specify
> +that a particular instrumentation or set of instrumentations should not be
> +applied to that function. The attribute takes a list of string literals,
> +which have the same meaning as values accepted by the ``-fno-sanitize=``
> +flag. For example, ``__attribute__((no_sanitize("address", "thread")))``
> +specifies that AddressSanitizer and ThreadSanitizer should not be applied
> +to the function.
> + }];
Do we have the -fno-sanitize flags documented somewhere? If so, it
would be handy to have a link to that documentation from here. If not,
that's fine.
> +}
> +
> def NoSanitizeAddressDocs : Documentation {
> let Category = DocCatFunction;
> // This function has multiple distinct spellings, and so it requires a custom
> Index: lib/CodeGen/CodeGenFunction.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.cpp
> +++ lib/CodeGen/CodeGenFunction.cpp
> @@ -608,6 +608,28 @@
> if (CGM.isInSanitizerBlacklist(Fn, Loc))
> SanOpts.clear();
>
> + if (D) {
> + // Apply the no_sanitize* attributes to SanOpts.
> + for (auto I = D->specific_attr_begin<NoSanitizeAttr>(),
> + E = D->specific_attr_end<NoSanitizeAttr>();
> + I != E; ++I)
Range-based for loop with specific_attrs<NoSanitizeAttr>()?
> + SanOpts.Mask &= ~I->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);
> + }
> +
> + // Apply sanitizer attributes to the function.
> + if (SanOpts.has(SanitizerKind::Address))
> + Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
> + if (SanOpts.has(SanitizerKind::Thread))
> + Fn->addFnAttr(llvm::Attribute::SanitizeThread);
> + if (SanOpts.has(SanitizerKind::Memory))
> + Fn->addFnAttr(llvm::Attribute::SanitizeMemory);
> +
> // Pass inline keyword to optimizer if it appears explicitly on any
> // declaration. Also, in the case of -fno-inline attach NoInline
> // attribute to all function that are not marked AlwaysInline.
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -752,23 +752,6 @@
> else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
> B.addAttribute(llvm::Attribute::StackProtectReq);
>
> - // Add sanitizer attributes if function is not blacklisted.
> - if (!isInSanitizerBlacklist(F, D->getLocation())) {
> - // When AddressSanitizer is enabled, set SanitizeAddress attribute
> - // unless __attribute__((no_sanitize_address)) is used.
> - if (LangOpts.Sanitize.has(SanitizerKind::Address) &&
> - !D->hasAttr<NoSanitizeAddressAttr>())
> - B.addAttribute(llvm::Attribute::SanitizeAddress);
> - // Same for ThreadSanitizer and __attribute__((no_sanitize_thread))
> - if (LangOpts.Sanitize.has(SanitizerKind::Thread) &&
> - !D->hasAttr<NoSanitizeThreadAttr>())
> - B.addAttribute(llvm::Attribute::SanitizeThread);
> - // Same for MemorySanitizer and __attribute__((no_sanitize_memory))
> - if (LangOpts.Sanitize.has(SanitizerKind::Memory) &&
> - !D->hasAttr<NoSanitizeMemoryAttr>())
> - B.addAttribute(llvm::Attribute::SanitizeMemory);
> - }
> -
> F->addAttributes(llvm::AttributeSet::FunctionIndex,
> llvm::AttributeSet::get(
> F->getContext(), llvm::AttributeSet::FunctionIndex, B));
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -4354,6 +4354,24 @@
> handleAttrWithMessage<DeprecatedAttr>(S, D, Attr);
> }
>
> +static void handleNoSanitizeAttr(Sema &S, Decl *D, const AttributeList &Attr) {
> + std::vector<std::string> Sanitizers;
Can call checkAttributeAtLeastNumArgs(S, Attr, 1) to diagnose if no
arguments are specified after changing the definition of
VariadicStringArgument.
> +
> + for (unsigned I = 0, E = Attr.getNumArgs(); I != E; ++I) {
> + StringRef SanitizerName;
> + SourceLocation LiteralLoc;
> +
> + if (!S.checkStringLiteralArgumentAttr(Attr, I, SanitizerName, &LiteralLoc))
> + return;
> +
> + Sanitizers.push_back(SanitizerName);
> + }
> +
> + D->addAttr(::new (S.Context) NoSanitizeAttr(
> + Attr.getRange(), S.Context, Sanitizers.data(), Sanitizers.size(),
> + Attr.getAttributeSpellingListIndex()));
> +}
> +
> /// Handles semantic checking for features that are common to all attributes,
> /// such as checking whether a parameter was properly specified, or the correct
> /// number of arguments were passed, etc.
> @@ -4822,6 +4840,9 @@
> case AttributeList::AT_ScopedLockable:
> handleSimpleAttribute<ScopedLockableAttr>(S, D, Attr);
> break;
> + case AttributeList::AT_NoSanitize:
> + handleNoSanitizeAttr(S, D, Attr);
> + break;
> case AttributeList::AT_NoSanitizeAddress:
> handleSimpleAttribute<NoSanitizeAddressAttr>(S, D, Attr);
> break;
> Index: test/CodeGen/address-safety-attr.cpp
> ===================================================================
> --- test/CodeGen/address-safety-attr.cpp
> +++ test/CodeGen/address-safety-attr.cpp
> @@ -52,6 +52,21 @@
> int NoAddressSafety2(int *a);
> int NoAddressSafety2(int *a) { return *a; }
>
> +// WITHOUT: NoAddressSafety3{{.*}}) [[NOATTR]]
> +// BLFILE: NoAddressSafety3{{.*}}) [[NOATTR]]
> +// BLFUNC: NoAddressSafety3{{.*}}) [[NOATTR]]
> +// ASAN: NoAddressSafety3{{.*}}) [[NOATTR]]
> +__attribute__((no_sanitize("address")))
> +int NoAddressSafety3(int *a) { return *a; }
> +
> +// WITHOUT: NoAddressSafety4{{.*}}) [[NOATTR]]
> +// BLFILE: NoAddressSafety4{{.*}}) [[NOATTR]]
> +// BLFUNC: NoAddressSafety4{{.*}}) [[NOATTR]]
> +// ASAN: NoAddressSafety4{{.*}}) [[NOATTR]]
> +__attribute__((no_sanitize("address")))
> +int NoAddressSafety4(int *a);
> +int NoAddressSafety4(int *a) { return *a; }
> +
> // WITHOUT: AddressSafetyOk{{.*}}) [[NOATTR]]
> // BLFILE: AddressSafetyOk{{.*}}) [[NOATTR]]
> // BLFUNC: AddressSafetyOk{{.*}}) [[WITH]]
> @@ -86,16 +101,25 @@
> template<int i>
> int TemplateAddressSafetyOk() { return i; }
>
> -// WITHOUT: TemplateNoAddressSafety{{.*}}) [[NOATTR]]
> -// BLFILE: TemplateNoAddressSafety{{.*}}) [[NOATTR]]
> -// BLFUNC: TemplateNoAddressSafety{{.*}}) [[NOATTR]]
> -// ASAN: TemplateNoAddressSafety{{.*}}) [[NOATTR]]
> +// WITHOUT: TemplateNoAddressSafety1{{.*}}) [[NOATTR]]
> +// BLFILE: TemplateNoAddressSafety1{{.*}}) [[NOATTR]]
> +// BLFUNC: TemplateNoAddressSafety1{{.*}}) [[NOATTR]]
> +// ASAN: TemplateNoAddressSafety1{{.*}}) [[NOATTR]]
> template<int i>
> __attribute__((no_sanitize_address))
> -int TemplateNoAddressSafety() { return i; }
> +int TemplateNoAddressSafety1() { return i; }
> +
> +// WITHOUT: TemplateNoAddressSafety2{{.*}}) [[NOATTR]]
> +// BLFILE: TemplateNoAddressSafety2{{.*}}) [[NOATTR]]
> +// BLFUNC: TemplateNoAddressSafety2{{.*}}) [[NOATTR]]
> +// ASAN: TemplateNoAddressSafety2{{.*}}) [[NOATTR]]
> +template<int i>
> +__attribute__((no_sanitize("address")))
> +int TemplateNoAddressSafety2() { return i; }
>
> int force_instance = TemplateAddressSafetyOk<42>()
> - + TemplateNoAddressSafety<42>();
> + + TemplateNoAddressSafety1<42>()
> + + TemplateNoAddressSafety2<42>();
>
> // Check that __cxx_global_var_init* get the sanitize_address attribute.
> int global1 = 0;
> Index: test/CodeGen/sanitize-thread-attr.cpp
> ===================================================================
> --- test/CodeGen/sanitize-thread-attr.cpp
> +++ test/CodeGen/sanitize-thread-attr.cpp
> @@ -22,6 +22,12 @@
> int NoTSAN2(int *a);
> int NoTSAN2(int *a) { return *a; }
>
> +// WITHOUT: NoTSAN3{{.*}}) [[NOATTR:#[0-9]+]]
> +// BL: NoTSAN3{{.*}}) [[NOATTR:#[0-9]+]]
> +// TSAN: NoTSAN3{{.*}}) [[NOATTR:#[0-9]+]]
> +__attribute__((no_sanitize("thread")))
> +int NoTSAN3(int *a) { return *a; }
> +
> // WITHOUT: TSANOk{{.*}}) [[NOATTR]]
> // BL: TSANOk{{.*}}) [[NOATTR]]
> // TSAN: TSANOk{{.*}}) [[WITH:#[0-9]+]]
> Index: test/CodeGenCXX/cfi-vcall.cpp
> ===================================================================
> --- test/CodeGenCXX/cfi-vcall.cpp
> +++ test/CodeGenCXX/cfi-vcall.cpp
> @@ -47,16 +47,32 @@
> a->f();
> }
>
> -// CHECK: define internal void @_Z2dfPN12_GLOBAL__N_11DE
> -void df(D *d) {
> +// CHECK: define internal void @_Z3df1PN12_GLOBAL__N_11DE
> +void df1(D *d) {
> // CHECK: {{%[^ ]*}} = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"[{{.*}}cfi-vcall.cpp]N12_GLOBAL__N_11DE")
> d->f();
> }
>
> +// CHECK: define internal void @_Z3df2PN12_GLOBAL__N_11DE
> +__attribute__((no_sanitize("cfi")))
> +void df2(D *d) {
> + // CHECK-NOT: call i1 @llvm.bitset.test
> + d->f();
> +}
> +
> +// CHECK: define internal void @_Z3df3PN12_GLOBAL__N_11DE
> +__attribute__((no_sanitize("address"))) __attribute__((no_sanitize("cfi-vcall")))
> +void df3(D *d) {
> + // CHECK-NOT: call i1 @llvm.bitset.test
> + d->f();
> +}
> +
> D d;
>
> void foo() {
> - df(&d);
> + df1(&d);
> + df2(&d);
> + df3(&d);
> }
>
> // CHECK-DAG: !{!"1A", [3 x i8*]* @_ZTV1A, i64 16}
> Index: test/CodeGenObjC/no-sanitize.m
> ===================================================================
> --- /dev/null
> +++ test/CodeGenObjC/no-sanitize.m
> @@ -0,0 +1,8 @@
> +// RUN: %clang_cc1 %s -emit-llvm -fsanitize=address -o - | FileCheck %s
> +
> + at interface I0 @end
> + at implementation I0
> +// CHECK-NOT: sanitize_address
> +- (void) im0: (int) a0 __attribute__((no_sanitize("address"))) {
> +}
> + at end
> Index: test/SemaCXX/attr-no-sanitize.cpp
> ===================================================================
> --- /dev/null
> +++ test/SemaCXX/attr-no-sanitize.cpp
> @@ -0,0 +1,29 @@
> +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
> +// RUN: not %clang_cc1 -std=c++11 -ast-dump %s 2>&1 | FileCheck --check-prefix=DUMP %s
> +// RUN: not %clang_cc1 -std=c++11 -ast-print %s 2>&1 | FileCheck --check-prefix=PRINT %s
> +
> +int v1 __attribute__((no_sanitize("address"))); // expected-error{{'no_sanitize' attribute only applies to functions and methods}}
> +
> +int f1() __attribute__((no_sanitize)); // expected-error{{'no_sanitize' attribute takes at least 1 argument}}
> +
> +int f2() __attribute__((no_sanitize(1))); // expected-error{{'no_sanitize' attribute requires a string}}
> +
> +// DUMP-LABEL: FunctionDecl {{.*}} f3
> +// DUMP: NoSanitizeAttr {{.*}} address
> +// PRINT: int f3() __attribute__((no_sanitize("address")))
> +int f3() __attribute__((no_sanitize("address")));
> +
> +// DUMP-LABEL: FunctionDecl {{.*}} f4
> +// DUMP: NoSanitizeAttr {{.*}} thread
> +// PRINT: int f4() {{\[\[}}clang::no_sanitize("thread")]]
> +[[clang::no_sanitize("thread")]] int f4();
> +
> +// DUMP-LABEL: FunctionDecl {{.*}} f5
> +// DUMP: NoSanitizeAttr {{.*}} address thread
> +// PRINT: int f5() __attribute__((no_sanitize("address", "thread")))
> +int f5() __attribute__((no_sanitize("address", "thread")));
> +
> +// DUMP-LABEL: FunctionDecl {{.*}} f6
> +// DUMP: NoSanitizeAttr {{.*}} unknown
> +// PRINT: int f6() __attribute__((no_sanitize("unknown")))
> +int f6() __attribute__((no_sanitize("unknown")));
> Index: utils/TableGen/ClangAttrEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangAttrEmitter.cpp
> +++ utils/TableGen/ClangAttrEmitter.cpp
> @@ -82,6 +82,7 @@
> .Case("TypeSourceInfo *", "GetTypeSourceInfo(F, Record, Idx)")
> .Case("Expr *", "ReadExpr(F)")
> .Case("IdentifierInfo *", "GetIdentifierInfo(F, Record, Idx)")
> + .Case("std::string", "ReadString(Record, Idx)")
> .Default("Record[Idx++]");
> }
>
> @@ -95,6 +96,7 @@
> .Case("Expr *", "AddStmt(" + std::string(name) + ");\n")
> .Case("IdentifierInfo *",
> "AddIdentifierRef(" + std::string(name) + ", Record);\n")
> + .Case("std::string", "AddString(" + std::string(name) + ", Record);\n")
> .Default("Record.push_back(" + std::string(name) + ");\n");
> }
>
> @@ -983,6 +985,16 @@
> }
> };
>
> + class VariadicStringArgument : public VariadicArgument {
> + public:
> + VariadicStringArgument(const Record &Arg, StringRef Attr)
> + : VariadicArgument(Arg, Attr, "std::string")
> + {}
> + void writeValueImpl(raw_ostream &OS) const override {
> + OS << " OS << \"\\\"\" << Val << \"\\\"\";\n";
> + }
> + };
Hmm, this is pretty ineffecient (StringArgument uses StringRef), but I
think it's fine for right now. We can certainly refactor it later, as
I doubt this usage is in the hot path anywhere.
> +
> class TypeArgument : public SimpleArgument {
> public:
> TypeArgument(const Record &Arg, StringRef Attr)
> @@ -1044,6 +1056,8 @@
> Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "unsigned");
> else if (ArgName == "VariadicUnsignedArgument")
> Ptr = llvm::make_unique<VariadicArgument>(Arg, Attr, "unsigned");
> + else if (ArgName == "VariadicStringArgument")
> + Ptr = llvm::make_unique<VariadicStringArgument>(Arg, Attr);
> else if (ArgName == "VariadicEnumArgument")
> Ptr = llvm::make_unique<VariadicEnumArgument>(Arg, Attr);
> else if (ArgName == "VariadicExprArgument")
>
~Aaron
More information about the cfe-commits
mailing list