[llvm-branch-commits] [clang] 997d922 - This patch adds `#pragma clang header_unsafe` to enable flagging macros

Chris Bieneman via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Jul 29 09:24:43 PDT 2021


Author: Chris Bieneman
Date: 2021-07-29T10:10:51-05:00
New Revision: 997d9222f2ffa08c646076f2154993484e1ade98

URL: https://github.com/llvm/llvm-project/commit/997d9222f2ffa08c646076f2154993484e1ade98
DIFF: https://github.com/llvm/llvm-project/commit/997d9222f2ffa08c646076f2154993484e1ade98.diff

LOG: This patch adds `#pragma clang header_unsafe` to enable flagging macros
as unsafe for header use. This is to allow macros that may have ABI
implications to be avoided in headers that have ABI stability promises.

Using macros in headers (particularly public headers) can cause a
variety of issues relating to ABI and modules. This new pragma logs
warnings when using annotated macros outside the main source file.

This warning is added under a new diagnostics group -Wpedantic-macros.

Added: 
    clang/test/Lexer/Inputs/unsafe-macro-2.h
    clang/test/Lexer/Inputs/unsafe-macro.h
    clang/test/Lexer/unsafe-macro.c

Modified: 
    clang/docs/LanguageExtensions.rst
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Basic/DiagnosticLexKinds.td
    clang/include/clang/Basic/IdentifierTable.h
    clang/include/clang/Lex/Preprocessor.h
    clang/lib/Lex/Pragma.cpp
    clang/lib/Lex/Preprocessor.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 8dafcb2f9a724..952db626535f4 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3492,8 +3492,7 @@ The pragma can take two values: ``on`` and ``off``.
     float v = t + z;
   }
 
-
-``#pragma clang fp contract`` specifies whether the compiler should
+x2``#pragma clang fp contract`` specifies whether the compiler should
 contract a multiply and an addition (or subtraction) into a fused FMA
 operation when supported by the target.
 
@@ -3905,6 +3904,28 @@ provide deprecation warnings for macro uses. For example:
 ``#pragma GCC warning`` because the warning can be controlled with
 ``-Wdeprecated``.
 
+Header Unsafe Macros
+====================
+
+Clang supports the pragma ``#pragma clang header_unsafe``, which can be used to
+mark macros as unsafe to use in headers. This can be valuable when providing
+headers with ABI stability requirements. For example:
+
+.. code-block:: c
+   #define TARGET_ARM 1
+   #pragma clang header_unsafe(TARGET_ARM, "<reason>")
+
+   /// Foo.h
+   struct Foo {
+   #if TARGET_ARM // warning: TARGET_ARM is marked unsafe in headers: <reason>
+     uint32_t X;
+   #else
+     uint64_t X;
+   #endif
+   };
+
+This warning is controlled by ``-Wpedantic-macros``.
+
 Extended Integer Types
 ======================
 

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6857c889b72b6..bd1cc3c7ff3cd 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1305,3 +1305,7 @@ def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
 def RTTI : DiagGroup<"rtti">;
 
 def OpenCLCoreFeaturesDiagGroup : DiagGroup<"pedantic-core-features">;
+
+// Warnings and extensions to make preprocessor macro usage pedantic
+def PedanticMacros : DiagGroup<"pedantic-macros",
+                    [DeprecatedPragma, MacroRedefined, BuiltinMacroRedefined]>;

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 174f6c3dfd4c6..ea4f0b2d5bb0c 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -524,6 +524,12 @@ def warn_pragma_deprecated_macro_use :
   ExtWarn<"macro %0 has been marked as deprecated%select{|: %2}1">,
   InGroup<DeprecatedPragma>;
 
+// - #pragma clang header_unsafe(...)
+def warn_pragma_header_unsafe_macro_use :
+  ExtWarn<"macro %0 has been marked as unsafe for use in headers"
+          "%select{|: %2}1">,
+  InGroup<PedanticMacros>;
+
 // - #pragma execution_character_set(...)
 def warn_pragma_exec_charset_expected :
   ExtWarn<"#pragma execution_character_set expected '%0'">,

diff  --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h
index d75d43f0398d7..599d10f8bd00a 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -124,7 +124,10 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   // True if this is a deprecated macro
   unsigned IsDeprecatedMacro : 1;
 
-  // 24 bits left in a 64-bit word.
+  // True if this macro is unsafe in headers
+  unsigned IsHeaderUnsafe : 1;
+
+  // 23 bits left in a 64-bit word.
 
   // Managed by the language front-end.
   void *FETokenInfo = nullptr;
@@ -138,7 +141,7 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
         NeedsHandleIdentifier(false), IsFromAST(false), ChangedAfterLoad(false),
         FEChangedAfterLoad(false), RevertedTokenID(false), OutOfDate(false),
         IsModulesImport(false), IsMangledOpenMPVariantName(false),
-        IsDeprecatedMacro(false) {}
+        IsDeprecatedMacro(false), IsHeaderUnsafe(false) {}
 
 public:
   IdentifierInfo(const IdentifierInfo &) = delete;
@@ -186,8 +189,11 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
       NeedsHandleIdentifier = true;
       HadMacro = true;
     } else {
+      // Since calling the setters of these calls recompute, just set them
+      // manually to avoid calling recompute a bunch of times.
+      IsDeprecatedMacro = false;
+      IsHeaderUnsafe = false;
       RecomputeNeedsHandleIdentifier();
-      setIsDeprecatedMacro(false);
     }
   }
   /// Returns true if this identifier was \#defined to some value at any
@@ -209,6 +215,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
       RecomputeNeedsHandleIdentifier();
   }
 
+  bool isHeaderUnsafe() const { return IsHeaderUnsafe; }
+
+  void setIsHeaderUnsafe(bool Val) {
+    if (IsHeaderUnsafe == Val)
+      return;
+    IsHeaderUnsafe = Val;
+    if (Val)
+      NeedsHandleIdentifier = true;
+    else
+      RecomputeNeedsHandleIdentifier();
+  }
+
   /// If this is a source-language token (e.g. 'for'), this API
   /// can be used to cause the lexer to map identifiers to source-language
   /// tokens.

diff  --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 45ee0ad9fd21e..177945eb6693b 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -794,6 +794,9 @@ class Preprocessor {
   /// Deprecation messages for macros provided in #pragma clang deprecated
   llvm::DenseMap<const IdentifierInfo *, std::string> MacroDeprecationMsgs;
 
+  /// Usage warning for macros marked by #pragma clang header_unsafe
+  llvm::DenseMap<const IdentifierInfo *, std::string> HeaderUnsafeMacroMsgs;
+
   /// A "freelist" of MacroArg objects that can be
   /// reused for quick allocation.
   MacroArgs *MacroArgCache = nullptr;
@@ -2406,6 +2409,17 @@ class Preprocessor {
     return MsgEntry->second;
   }
 
+  void addHeaderUnsafeMsg(const IdentifierInfo *II, std::string Msg) {
+    HeaderUnsafeMacroMsgs.insert(std::make_pair(II, Msg));
+  }
+
+  llvm::Optional<std::string> getHeaderUnsafeMsg(const IdentifierInfo *II) {
+    auto MsgEntry = HeaderUnsafeMacroMsgs.find(II);
+    if (MsgEntry == HeaderUnsafeMacroMsgs.end())
+      return llvm::None;
+    return MsgEntry->second;
+  }
+
   void emitMacroExpansionWarnings(const Token &Identifier);
 
 private:

diff  --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp
index cb3c5a1f89e31..2f59dd6f2d4c4 100644
--- a/clang/lib/Lex/Pragma.cpp
+++ b/clang/lib/Lex/Pragma.cpp
@@ -1911,54 +1911,83 @@ struct PragmaRegionHandler : public PragmaHandler {
   }
 };
 
-/// "\#pragma clang deprecated(...)"
-///
-/// The syntax is
-/// \code
-///   #pragma clang deprecate(MACRO_NAME [, Message])
-/// \endcode
-struct PragmaDeprecatedHandler : public PragmaHandler {
-  PragmaDeprecatedHandler() : PragmaHandler("deprecated") {}
-
-  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
-                    Token &Tok) override {
-    std::string Macro, MessageString;
+/// This handles parsing pragmas that take a macro name and optional message
+static IdentifierInfo *HandleMacroAnnotationPragma(Preprocessor &PP, Token &Tok, const char *Pragma, std::string& MessageString) {
+  std::string Macro;
 
     PP.Lex(Tok);
     if (Tok.isNot(tok::l_paren)) {
       PP.Diag(Tok, diag::err_expected) << "(";
-      return;
+      return nullptr;
     }
 
     PP.LexUnexpandedToken(Tok);
     if (!Tok.is(tok::identifier)) {
       PP.Diag(Tok, diag::err_expected) << tok::identifier;
-      return;
+      return nullptr;
     }
     IdentifierInfo *II = Tok.getIdentifierInfo();
 
     if (!II->hasMacroDefinition()) {
       PP.Diag(Tok, diag::err_pp_visibility_non_macro) << II->getName();
-      return;
+      return nullptr;
     }
 
     PP.Lex(Tok);
     if (Tok.is(tok::comma)) {
       PP.Lex(Tok);
       if (!PP.FinishLexStringLiteral(Tok, MessageString,
-                                     "#pragma clang deprecated",
+                                     Pragma,
                                      /*AllowMacroExpansion=*/true))
-        return;
+        return nullptr;
     }
 
     if (Tok.isNot(tok::r_paren)) {
       PP.Diag(Tok, diag::err_expected) << ")";
-      return;
+      return nullptr;
+    }
+    return II;
+}
+
+/// "\#pragma clang deprecated(...)"
+///
+/// The syntax is
+/// \code
+///   #pragma clang deprecate(MACRO_NAME [, Message])
+/// \endcode
+struct PragmaDeprecatedHandler : public PragmaHandler {
+  PragmaDeprecatedHandler() : PragmaHandler("deprecated") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+                    Token &Tok) override {
+    std::string MessageString;
+
+    if (IdentifierInfo *II = HandleMacroAnnotationPragma(PP, Tok, "#pragma clang deprecated", MessageString)) {
+      II->setIsDeprecatedMacro(true);
+      if (!MessageString.empty())
+        PP.addMacroDeprecationMsg(II, std::move(MessageString));
     }
+  }
+};
+
+/// "\#pragma clang header_unsafe(...)"
+///
+/// The syntax is
+/// \code
+///   #pragma clang header_unsafe(MACRO_NAME [, Message])
+/// \endcode
+struct PragmaHeaderUnsafeHandler : public PragmaHandler {
+  PragmaHeaderUnsafeHandler() : PragmaHandler("header_unsafe") {}
 
-    II->setIsDeprecatedMacro(true);
-    if (!MessageString.empty())
-      PP.addMacroDeprecationMsg(II, std::move(MessageString));
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+                    Token &Tok) override {
+    std::string MessageString;
+
+    if (IdentifierInfo *II = HandleMacroAnnotationPragma(PP, Tok, "#pragma clang header_unsafe", MessageString)) {
+      II->setIsHeaderUnsafe(true);
+      if (!MessageString.empty())
+        PP.addHeaderUnsafeMsg(II, std::move(MessageString));
+    }
   }
 };
 
@@ -1991,6 +2020,7 @@ void Preprocessor::RegisterBuiltinPragmas() {
   AddPragmaHandler("clang", new PragmaARCCFCodeAuditedHandler());
   AddPragmaHandler("clang", new PragmaAssumeNonNullHandler());
   AddPragmaHandler("clang", new PragmaDeprecatedHandler());
+  AddPragmaHandler("clang", new PragmaHeaderUnsafeHandler());
 
   // #pragma clang module ...
   auto *ModuleHandler = new PragmaNamespace("module");

diff  --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index a0722a26cb780..e617a9fac0ea4 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -1423,6 +1423,17 @@ void Preprocessor::emitMacroExpansionWarnings(const Token &Identifier) {
       Diag(Identifier, diag::warn_pragma_deprecated_macro_use)
           << Identifier.getIdentifierInfo() << 1 << *DepMsg;
   }
+
+  if (Identifier.getIdentifierInfo()->isHeaderUnsafe() &&
+      !SourceMgr.isInMainFile(Identifier.getLocation())) {
+    auto DepMsg = getHeaderUnsafeMsg(Identifier.getIdentifierInfo());
+    if (!DepMsg)
+      Diag(Identifier, diag::warn_pragma_header_unsafe_macro_use)
+          << Identifier.getIdentifierInfo() << 0;
+    else
+      Diag(Identifier, diag::warn_pragma_header_unsafe_macro_use)
+          << Identifier.getIdentifierInfo() << 1 << *DepMsg;
+  }
 }
 
 ModuleLoader::~ModuleLoader() = default;

diff  --git a/clang/test/Lexer/Inputs/unsafe-macro-2.h b/clang/test/Lexer/Inputs/unsafe-macro-2.h
new file mode 100644
index 0000000000000..f906c8328d4c0
--- /dev/null
+++ b/clang/test/Lexer/Inputs/unsafe-macro-2.h
@@ -0,0 +1,70 @@
+// expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#if UNSAFE_MACRO
+#endif
+
+// expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#if defined(UNSAFE_MACRO)
+#endif
+
+// expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#ifdef UNSAFE_MACRO
+#endif
+
+// expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#ifndef UNSAFE_MACRO
+#endif
+
+// expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+const int x = UNSAFE_MACRO;
+
+// expected-warning at +1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}}
+const int y = UNSAFE_MACRO_2;
+
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}}
+#undef UNSAFE_MACRO_2
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}}
+#define UNSAFE_MACRO_2 2
+
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}}
+const int z = UNSAFE_MACRO_2;
+
+
+// Test that we diagnose on #elif.
+#if 0
+#elif UNSAFE_MACRO
+// expected-warning at -1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#endif
+
+
+// Test that we diagnose on #elifdef.
+#ifdef baz
+#elifdef UNSAFE_MACRO
+// expected-warning at -1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#endif
+
+// Test that we diagnose on #elifndef.
+#ifdef baz
+#elifndef UNSAFE_MACRO
+#endif
+// expected-warning at -2{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+
+// FIXME: These cases are currently not handled because clang doesn't expand
+// conditions on skipped #elif* blocks. See the FIXME notes in
+// Preprocessor::SkipExcludedConditionalBlock.
+
+#define frobble
+
+#ifdef frobble
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#elifndef UNSAFE_MACRO
+#endif
+
+#ifdef frobble
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#elifdef UNSAFE_MACRO
+#endif
+
+#if 1
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#elif UNSAFE_MACRO
+#endif

diff  --git a/clang/test/Lexer/Inputs/unsafe-macro.h b/clang/test/Lexer/Inputs/unsafe-macro.h
new file mode 100644
index 0000000000000..2b5747acbaf17
--- /dev/null
+++ b/clang/test/Lexer/Inputs/unsafe-macro.h
@@ -0,0 +1,18 @@
+// expected-error at +1{{expected (}}
+#pragma clang header_unsafe
+
+// expected-error at +1{{expected identifier}}
+#pragma clang header_unsafe(4
+
+// expected-error at +1{{no macro named foo}}
+#pragma clang header_unsafe(foo)
+
+
+#define UNSAFE_MACRO 1
+#pragma clang header_unsafe(UNSAFE_MACRO, "Don't use this!")
+
+#define UNSAFE_MACRO_2 2
+#pragma clang header_unsafe(UNSAFE_MACRO_2)
+
+// expected-error at +1{{expected )}}
+#pragma clang deprecated(UNSAFE_MACRO

diff  --git a/clang/test/Lexer/unsafe-macro.c b/clang/test/Lexer/unsafe-macro.c
new file mode 100644
index 0000000000000..bb426d8aa77b1
--- /dev/null
+++ b/clang/test/Lexer/unsafe-macro.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -Wpedantic-macros %s -fsyntax-only -verify
+#include "Inputs/unsafe-macro.h"
+#include "Inputs/unsafe-macro-2.h"
+
+// not-expected-warning at +1{{macro 'UNSAFE_MACRO' has been marked as unsafe for use in headers: Don't use this!}}
+#if UNSAFE_MACRO
+#endif


        


More information about the llvm-branch-commits mailing list