On Tue, Nov 8, 2016 at 3:52 PM, Kostya Serebryany via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > > 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).
I concur; please add me as a reviewer. ~Aaron > > --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/SanitizerMetadata.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/SemaDeclAttr.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.getAttributeSpellingListIndex())); >>>>> + D->addAttr(::new (S.Context) >>>>> + DeprecatedAttr(Attr.getRange(), S.Context, Str, >>>>> Replacement, >>>>> + >>>>> Attr.getAttributeSpellingListIndex())); >>>>> +} >>>>> + >>>>> +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.getAttributeSpellingListIndex())); >>>>> >>>>> Modified: cfe/trunk/test/CodeGen/asan-globals.cpp >>>>> URL: >>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/asan-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/attr-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/attr-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 > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
