[PATCH] D26454: Implement no_sanitize_address for global vars

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/CodeGen/SanitizerMetadata.cpp:68
+  bool IsBlacklisted = false;
+  for (auto Attr : D.specific_attrs<NoSanitizeAttr>())
+    if (Attr->getMask() & SanitizerKind::Address)
----------------
Should use `const auto *` (feel free to change the other use(s) in a separate commit, no review required).


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5316
 
-  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,
----------------
This formatting change is unrelated.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5343
+    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?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5364
+    S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+        << Attr.getName() << ExpectedFunction;
   D->addAttr(::new (S.Context)
----------------
You diagnose it as an error, but then add the attribute anyway. Is that intentional?


================
Comment at: test/CodeGen/asan-globals.cpp:10
 int dyn_init_global = global;
+int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
----------------
You modified no_sanitize_address, but did not add any test cases for it.


================
Comment at: test/SemaCXX/attr-no-sanitize-address.cpp:24
 
-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.


================
Comment at: test/SemaCXX/attr-no-sanitize.cpp:5
 
-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.


https://reviews.llvm.org/D26454