r284272 - Implement no_sanitize_address for global vars

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 8 13:00:21 PST 2016 

On Tue, Nov 8, 2016 at 3:52 PM, Kostya Serebryany via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
> On Tue, Nov 8, 2016 at 11:27 AM, Douglas Katzman <dougk at 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 <kcc at google.com> wrote:
>>>
>>> ping
>>>
>>> On Mon, Oct 17, 2016 at 5:57 PM, Kostya Serebryany <kcc at 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 at 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 at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

More information about the cfe-commits mailing list