r284272 - Implement no_sanitize_address for global vars
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 8 11:40:58 PST 2016
On Fri, Oct 14, 2016 at 3: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">;
The down-side to this change is that now every no_sanitize attribute
now appertains to a global variable, as far as its subjects go, but
really only the address sanitizer applies to global variables. I'm not
certain there's much to be done for it, but this divergence is
unfortunate. For instance, this means that misuse of no_sanitizer for
thread may tell the user the attribute appertains to global variables,
and when they fix the misuse on a global variable, they're told "just
kidding, this doesn't apply to global variables."
> 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">;
This new diagnostic looks incorrect to me -- the subject list does not
list methods or properties, for instance.
> 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>())
This should be const auto * and probably not use Attr (since that's a
type name).
> + 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()));
> +}
Why are you touching the deprecated attribute code (it seems like
unrelated reformatting).
> +
> +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;
You diagnose this as an error, but don't early return if the attribute
is invalid. Is that intentional?
> 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;
You diagnose it as an error, but then add the attribute anyway. Is
that intentional?
> 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;
You modified no_sanitize_address, but did not add any test cases for it.
> 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}}
> -
Please add a new test case to replace this one, showing that the
attribute is properly diagnosed when applied to something the
attribute cannot appertain to.
> 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}}
> -
Please add a new test case to replace this one, showing that the
attribute is properly diagnosed when applied to something the
attribute cannot appertain to.
~Aaron
> 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
More information about the cfe-commits
mailing list