On Tue, Nov 8, 2016 at 11:27 AM, Douglas Katzman <do...@google.com> wrote:
> oh, sorry for missing this email. > I'll say "no" - I was hoping you'd audit it! > We do indeed have this practice for post-commit audit, but only for the authors or active maintainers of the piece of code in question. I afraid I do not fully understand the consequences of this patch -- they may be non-trivial. Please initiate a code review (even though the code is already committed). --kcc > > jyknight looked at it and gave me the suggestion to fail the attribute > parsing if, in the non-deprecated syntax, _any_ of the no_sanitize > modifiers are inapplicable to global vars. > > On Tue, Oct 25, 2016 at 7:19 PM, Kostya Serebryany <k...@google.com> wrote: > >> ping >> >> On Mon, Oct 17, 2016 at 5:57 PM, Kostya Serebryany <k...@google.com> >> wrote: >> >>> Did you code-review this? >>> (sorry if I missed it) >>> >>> On Fri, Oct 14, 2016 at 12:55 PM, Douglas Katzman via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>> Author: dougk >>>> Date: Fri Oct 14 14:55:09 2016 >>>> New Revision: 284272 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=284272&view=rev >>>> Log: >>>> Implement no_sanitize_address for global vars >>>> >>>> Modified: >>>> cfe/trunk/include/clang/Basic/Attr.td >>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>>> cfe/trunk/include/clang/Sema/AttributeList.h >>>> cfe/trunk/lib/CodeGen/SanitizerMetadata.cpp >>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>>> cfe/trunk/test/CodeGen/asan-globals.cpp >>>> cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp >>>> cfe/trunk/test/SemaCXX/attr-no-sanitize.cpp >>>> >>>> Modified: cfe/trunk/include/clang/Basic/Attr.td >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>> Basic/Attr.td?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/include/clang/Basic/Attr.td (original) >>>> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Oct 14 14:55:09 2016 >>>> @@ -1705,7 +1705,8 @@ def X86ForceAlignArgPointer : Inheritabl >>>> def NoSanitize : InheritableAttr { >>>> let Spellings = [GNU<"no_sanitize">, CXX11<"clang", "no_sanitize">]; >>>> let Args = [VariadicStringArgument<"Sanitizers">]; >>>> - let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>; >>>> + let Subjects = SubjectList<[Function, ObjCMethod, GlobalVar], >>>> ErrorDiag, >>>> + "ExpectedFunctionMethodOrGlobalVar">; >>>> let Documentation = [NoSanitizeDocs]; >>>> let AdditionalMembers = [{ >>>> SanitizerMask getMask() const { >>>> @@ -1727,7 +1728,8 @@ def NoSanitizeSpecific : InheritableAttr >>>> GCC<"no_sanitize_address">, >>>> GCC<"no_sanitize_thread">, >>>> GNU<"no_sanitize_memory">]; >>>> - let Subjects = SubjectList<[Function], ErrorDiag>; >>>> + let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag, >>>> + "ExpectedFunctionGlobalVarMethodOrProperty">; >>>> let Documentation = [NoSanitizeAddressDocs, NoSanitizeThreadDocs, >>>> NoSanitizeMemoryDocs]; >>>> let ASTNode = 0; >>>> >>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>> Basic/DiagnosticSemaKinds.td?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct 14 >>>> 14:55:09 2016 >>>> @@ -2577,6 +2577,7 @@ def warn_attribute_wrong_decl_type : War >>>> "|functions, methods and blocks" >>>> "|functions, methods, and classes" >>>> "|functions, methods, and parameters" >>>> + "|functions, methods, and global variables" >>>> "|classes" >>>> "|enums" >>>> "|variables" >>>> >>>> Modified: cfe/trunk/include/clang/Sema/AttributeList.h >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>> Sema/AttributeList.h?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/include/clang/Sema/AttributeList.h (original) >>>> +++ cfe/trunk/include/clang/Sema/AttributeList.h Fri Oct 14 14:55:09 >>>> 2016 >>>> @@ -891,6 +891,7 @@ enum AttributeDeclKind { >>>> ExpectedFunctionMethodOrBlock, >>>> ExpectedFunctionMethodOrClass, >>>> ExpectedFunctionMethodOrParameter, >>>> + ExpectedFunctionMethodOrGlobalVar, >>>> ExpectedClass, >>>> ExpectedEnum, >>>> ExpectedVariable, >>>> >>>> Modified: cfe/trunk/lib/CodeGen/SanitizerMetadata.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/Sa >>>> nitizerMetadata.cpp?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/lib/CodeGen/SanitizerMetadata.cpp (original) >>>> +++ cfe/trunk/lib/CodeGen/SanitizerMetadata.cpp Fri Oct 14 14:55:09 >>>> 2016 >>>> @@ -63,7 +63,13 @@ void SanitizerMetadata::reportGlobalToAS >>>> std::string QualName; >>>> llvm::raw_string_ostream OS(QualName); >>>> D.printQualifiedName(OS); >>>> - reportGlobalToASan(GV, D.getLocation(), OS.str(), D.getType(), >>>> IsDynInit); >>>> + >>>> + bool IsBlacklisted = false; >>>> + for (auto Attr : D.specific_attrs<NoSanitizeAttr>()) >>>> + if (Attr->getMask() & SanitizerKind::Address) >>>> + IsBlacklisted = true; >>>> + reportGlobalToASan(GV, D.getLocation(), OS.str(), D.getType(), >>>> IsDynInit, >>>> + IsBlacklisted); >>>> } >>>> >>>> void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable >>>> *GV) { >>>> >>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >>>> eclAttr.cpp?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Oct 14 14:55:09 2016 >>>> @@ -5313,9 +5313,15 @@ static void handleDeprecatedAttr(Sema &S >>>> !(Attr.hasScope() && Attr.getScopeName()->isStr("gnu"))) >>>> S.Diag(Attr.getLoc(), diag::ext_cxx14_attr) << Attr.getName(); >>>> >>>> - D->addAttr(::new (S.Context) DeprecatedAttr(Attr.getRange(), >>>> S.Context, Str, >>>> - Replacement, >>>> - Attr.getAttributeSpellingList >>>> Index())); >>>> + D->addAttr(::new (S.Context) >>>> + DeprecatedAttr(Attr.getRange(), S.Context, Str, >>>> Replacement, >>>> + Attr.getAttributeSpellingListI >>>> ndex())); >>>> +} >>>> + >>>> +static bool isGlobalVar(const Decl *D) { >>>> + if (const auto *S = dyn_cast<VarDecl>(D)) >>>> + return S->hasGlobalStorage(); >>>> + return false; >>>> } >>>> >>>> static void handleNoSanitizeAttr(Sema &S, Decl *D, const AttributeList >>>> &Attr) { >>>> @@ -5333,7 +5339,9 @@ static void handleNoSanitizeAttr(Sema &S >>>> >>>> if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0) >>>> S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << >>>> SanitizerName; >>>> - >>>> + else if (isGlobalVar(D) && SanitizerName != "address") >>>> + S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type) >>>> + << Attr.getName() << ExpectedFunctionOrMethod; >>>> Sanitizers.push_back(SanitizerName); >>>> } >>>> >>>> @@ -5346,12 +5354,14 @@ static void handleNoSanitizeSpecificAttr >>>> const AttributeList &Attr) { >>>> StringRef AttrName = Attr.getName()->getName(); >>>> normalizeName(AttrName); >>>> - StringRef SanitizerName = >>>> - llvm::StringSwitch<StringRef>(AttrName) >>>> - .Case("no_address_safety_analysis", "address") >>>> - .Case("no_sanitize_address", "address") >>>> - .Case("no_sanitize_thread", "thread") >>>> - .Case("no_sanitize_memory", "memory"); >>>> + StringRef SanitizerName = llvm::StringSwitch<StringRef>(AttrName) >>>> + .Case("no_address_safety_analysis", >>>> "address") >>>> + .Case("no_sanitize_address", "address") >>>> + .Case("no_sanitize_thread", "thread") >>>> + .Case("no_sanitize_memory", "memory"); >>>> + if (isGlobalVar(D) && SanitizerName != "address") >>>> + S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type) >>>> + << Attr.getName() << ExpectedFunction; >>>> D->addAttr(::new (S.Context) >>>> NoSanitizeAttr(Attr.getRange(), S.Context, >>>> &SanitizerName, 1, >>>> Attr.getAttributeSpellingList >>>> Index())); >>>> >>>> Modified: cfe/trunk/test/CodeGen/asan-globals.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/a >>>> san-globals.cpp?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/test/CodeGen/asan-globals.cpp (original) >>>> +++ cfe/trunk/test/CodeGen/asan-globals.cpp Fri Oct 14 14:55:09 2016 >>>> @@ -7,6 +7,7 @@ >>>> >>>> int global; >>>> int dyn_init_global = global; >>>> +int __attribute__((no_sanitize("address"))) attributed_global; >>>> int blacklisted_global; >>>> >>>> void func() { >>>> @@ -14,24 +15,26 @@ void func() { >>>> const char *literal = "Hello, world!"; >>>> } >>>> >>>> -// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], >>>> ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], >>>> ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], >>>> ![[LITERAL:[0-9]+]]} >>>> +// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], >>>> ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], >>>> ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], >>>> ![[LITERAL:[0-9]+]]} >>>> // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], >>>> !"extra_global", i1 false, i1 false} >>>> // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, >>>> i32 5} >>>> // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 >>>> false, i1 false} >>>> // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5} >>>> // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], >>>> !"dyn_init_global", i1 true, i1 false} >>>> // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 >>>> 5} >>>> +// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true} >>>> // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 >>>> true} >>>> // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], >>>> !"static_var", i1 false, i1 false} >>>> -// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 13, i32 >>>> 14} >>>> +// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 >>>> 14} >>>> // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"<string >>>> literal>", i1 false, i1 false} >>>> -// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 >>>> 25} >>>> +// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 >>>> 25} >>>> >>>> -// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], >>>> ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], >>>> ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], >>>> ![[LITERAL:[0-9]+]]} >>>> +// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], >>>> ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], >>>> ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], >>>> ![[LITERAL:[0-9]+]]} >>>> // BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} >>>> ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false} >>>> // BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", >>>> i32 1, i32 5} >>>> // BLACKLIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true} >>>> // BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, >>>> i1 true} >>>> +// BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, >>>> i1 true} >>>> // BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 >>>> false, i1 true} >>>> // BLACKLIST-SRC: ![[STATIC_VAR]] = !{{{.*}} null, null, i1 false, i1 >>>> true} >>>> // BLACKLIST-SRC: ![[LITERAL]] = !{{{.*}} null, null, i1 false, i1 >>>> true} >>>> >>>> Modified: cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/a >>>> ttr-no-sanitize-address.cpp?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp (original) >>>> +++ cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp Fri Oct 14 >>>> 14:55:09 2016 >>>> @@ -21,9 +21,6 @@ int noanal_testfn(int y) { >>>> return x; >>>> } >>>> >>>> -int noanal_test_var NO_SANITIZE_ADDRESS; // \ >>>> - // expected-error {{'no_sanitize_address' attribute only applies to >>>> functions}} >>>> - >>>> class NoanalFoo { >>>> private: >>>> int test_field NO_SANITIZE_ADDRESS; // \ >>>> >>>> Modified: cfe/trunk/test/SemaCXX/attr-no-sanitize.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/a >>>> ttr-no-sanitize.cpp?rev=284272&r1=284271&r2=284272&view=diff >>>> ============================================================ >>>> ================== >>>> --- cfe/trunk/test/SemaCXX/attr-no-sanitize.cpp (original) >>>> +++ cfe/trunk/test/SemaCXX/attr-no-sanitize.cpp Fri Oct 14 14:55:09 >>>> 2016 >>>> @@ -2,8 +2,6 @@ >>>> // RUN: not %clang_cc1 -std=c++11 -ast-dump %s | FileCheck >>>> --check-prefix=DUMP %s >>>> // RUN: not %clang_cc1 -std=c++11 -ast-print %s | 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}} >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> >>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
