[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D26454: Implement no_sanitize_address for global vars
From:       Douglas Katzman via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2016-11-30 20:23:32
Message-ID: 4638f719e78b3a546cec22fd628306dc () localhost ! localdomain
[Download RAW message or body]

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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]