[clang] Give better message for unsupported no_sanitize on globals

[abrachet]

Summary

Previously if you specified no_sanitize("known_sanitizer") on a global you
would yield a misleading error "'no_sanitize' attribute only applies to
functions and methods", but no_sanitize("unknown") would simply be a warning,
"unknown sanitizer 'unknown' ignored". This changes the former to a warning
"'no_sanitize' attribute argument not supported for globals: known_sanitizer".

[hctim]

Thanks!

[aaron.ballman]

LGTM aside from a requested change in diagnostic wording (no need for additional review), though please also add a release note when landing so people know about the improvement here.

clang/include/clang/Basic/DiagnosticSemaKinds.td

4056

A slightly different color of bikeshed that might be easier on the eyes.

[abrachet]

In D133117#3764721, @aaron.ballman wrote:

LGTM aside from a requested change in diagnostic wording (no need for additional review), though please also add a release note when landing so people know about the improvement here.

Sure, this and the inline have been addressed in the commit

clang/docs/ReleaseNotes.rst

- Clang will now print more information about failed static assertions. In
   compile-time value and printed out if the assertion fails.
 - Diagnostics about uninitialized ``constexpr`` varaibles have been improved
   to mention the missing constant initializer.
 - Correctly diagnose a future keyword if it exist as a keyword in the higher
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
 - When diagnosing multi-level pack expansions of mismatched lengths, Clang will
   now, in most cases, be able to point to the relevant outer parameter.
+- no_sanitize("...") on a global variable for known but not relevant sanitizers
+  is now just a warning. It now says that this will be ignored instead of
+  incorrectly saying no_sanitize only applies to functions and methods.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 New Compiler Flags
 ------------------
 
 Deprecated Compiler Flags

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_transparent_union_attribute_floating : Warning<
   InGroup<IgnoredAttributes>;
 def warn_transparent_union_attribute_zero_fields : Warning<
   "transparent union definition must contain at least one field; "
   "transparent_union attribute ignored">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_type_not_supported : Warning<
   "%0 attribute argument not supported: %1">,
   InGroup<IgnoredAttributes>;
+def warn_attribute_type_not_supported_global : Warning<
+  "%0 attribute argument '%1' not supported on a global variable">,
+  InGroup<IgnoredAttributes>;
 def warn_attribute_unknown_visibility : Warning<"unknown visibility %0">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_protected_visibility :
   Warning<"target does not support 'protected' visibility; using 'default'">,
   InGroup<DiagGroup<"unsupported-visibility">>;
 def err_mismatched_visibility: Error<"visibility does not match previous declaration">;
 def note_previous_attribute : Note<"previous attribute is here">;
 def note_conflicting_attribute : Note<"conflicting attribute is here">;

clang/lib/Sema/SemaDeclAttr.cpp

for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
     if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
       return;
 
     if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
             SanitizerMask() &&
         SanitizerName != "coverage")
       S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
     else if (isGlobalVar(D) && !isSanitizerAttributeAllowedOnGlobals(SanitizerName))
-      S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
-          << AL << ExpectedFunctionOrMethod;
+      S.Diag(D->getLocation(), diag::warn_attribute_type_not_supported_global)
+          << AL << SanitizerName;
     Sanitizers.push_back(SanitizerName);
   }
 
   D->addAttr(::new (S.Context) NoSanitizeAttr(S.Context, AL, Sanitizers.data(),
                                               Sanitizers.size()));
 }
 
 static void handleNoSanitizeSpecificAttr(Sema &S, Decl *D,

clang/test/SemaCXX/attr-no-sanitize.cpp

 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 // 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 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}}
 
+__attribute__((no_sanitize("all"))) int global; // expected-warning{{'no_sanitize' attribute argument 'all' not supported on a global variable}}
+__attribute__((no_sanitize("unknown"))) int global2; // expected-warning{{unknown sanitizer 'unknown' ignored}}
+
 // DUMP-LABEL: FunctionDecl {{.*}} f3
 // DUMP: NoSanitizeAttr {{.*}} address
 // PRINT: int f3() __attribute__((no_sanitize("address")))
 int f3() __attribute__((no_sanitize("address")));
 
 // DUMP-LABEL: FunctionDecl {{.*}} f4
 // DUMP: NoSanitizeAttr {{.*}} thread
 // PRINT: int f4() {{\[\[}}clang::no_sanitize("thread")]]