[PATCH] D26454: Implement no_sanitize_address for global vars

[Douglas Katzman via Phabricator via cfe-commits]

dougk added a comment.

Suppression of sanitizing is necessary if the variable is magically a memory-mapped device I/O address.
The linker can arrange for this to be the case using fancy scripts, or even just as simple as a section attribute that requires that you take up exactly a certain number of bytes in the section.
There was some thought that any non-default section should preclude sanitization, but Kostya said that, no, it would make sense to require explicit no-sanitize.  I (mistakenly) took that to mean "just do it", for which I apologize.



================
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,
----------------
aaron.ballman wrote:
> This formatting change is unrelated.
reverted. (clang-format-diff did that on account of proximity to the added lines)


================
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;
----------------
aaron.ballman wrote:
> You diagnose this as an error, but don't early return if the attribute is invalid. Is that intentional?
not intentional. fixed


================
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)
----------------
aaron.ballman wrote:
> You diagnose it as an error, but then add the attribute anyway. Is that intentional?
not intentional. fixed


================
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}}
----------------
aaron.ballman wrote:
> 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.
that's already tested by noanal_testfn which has no_sanitize_address on 'int x = y';


https://reviews.llvm.org/D26454