[llvm] c4779ea - [libc++abi] Avoid raw calls to assert() in libc++abi (#71121)

via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 23 15:21:48 PST 2023


Author: Louis Dionne
Date: 2023-11-23T18:21:43-05:00
New Revision: c4779ea8e709a7bce7288988fabf1ba50e9c9477

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

LOG: [libc++abi] Avoid raw calls to assert() in libc++abi (#71121)

The runtimes now have a principled way of doing assertions in relation
to hardening, so we should use that instead of raw calls to assert()
inside libc++abi. This patch aims to maintain the behavior of the
demangler code when it is used from within LLVM by introducing a simple
DEMANGLE_ASSERT(...) macro that is then defined to the appropriate
assertion mechanism.

Added: 
    

Modified: 
    libcxxabi/src/abort_message.h
    libcxxabi/src/cxa_demangle.cpp
    libcxxabi/src/demangle/DemangleConfig.h
    libcxxabi/src/demangle/ItaniumDemangle.h
    libcxxabi/src/demangle/Utility.h
    libcxxabi/src/fallback_malloc.cpp
    libcxxabi/test/test_fallback_malloc.pass.cpp
    llvm/include/llvm/Demangle/DemangleConfig.h
    llvm/include/llvm/Demangle/ItaniumDemangle.h
    llvm/include/llvm/Demangle/Utility.h

Removed: 
    


################################################################################
diff  --git a/libcxxabi/src/abort_message.h b/libcxxabi/src/abort_message.h
index f1d5c12e252856f..97641777801919b 100644
--- a/libcxxabi/src/abort_message.h
+++ b/libcxxabi/src/abort_message.h
@@ -14,4 +14,15 @@
 extern "C" _LIBCXXABI_HIDDEN _LIBCXXABI_NORETURN void
 abort_message(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
+#ifndef _LIBCXXABI_ASSERT
+#  define _LIBCXXABI_ASSERT(expr, msg)                                                                                 \
+    do {                                                                                                               \
+      if (!(expr)) {                                                                                                   \
+        char const* __msg = (msg);                                                                                     \
+        ::abort_message("%s:%d: %s", __FILE__, __LINE__, __msg);                                                       \
+      }                                                                                                                \
+    } while (false)
+
 #endif
+
+#endif // __ABORT_MESSAGE_H_

diff  --git a/libcxxabi/src/cxa_demangle.cpp b/libcxxabi/src/cxa_demangle.cpp
index 6055b2a538de414..bece33a007fcd3d 100644
--- a/libcxxabi/src/cxa_demangle.cpp
+++ b/libcxxabi/src/cxa_demangle.cpp
@@ -10,10 +10,12 @@
 // file does not yet support:
 //   - C++ modules TS
 
+#include "abort_message.h"
+#define DEMANGLE_ASSERT(expr, msg) _LIBCXXABI_ASSERT(expr, msg)
+
 #include "demangle/DemangleConfig.h"
 #include "demangle/ItaniumDemangle.h"
 #include "__cxxabi_config.h"
-#include <cassert>
 #include <cctype>
 #include <cstdio>
 #include <cstdlib>
@@ -395,7 +397,7 @@ __cxa_demangle(const char *MangledName, char *Buf, size_t *N, int *Status) {
     InternalStatus = demangle_invalid_mangled_name;
   else {
     OutputBuffer O(Buf, N);
-    assert(Parser.ForwardTemplateRefs.empty());
+    DEMANGLE_ASSERT(Parser.ForwardTemplateRefs.empty(), "");
     AST->print(O);
     O += '\0';
     if (N != nullptr)

diff  --git a/libcxxabi/src/demangle/DemangleConfig.h b/libcxxabi/src/demangle/DemangleConfig.h
index d5e11432d986764..dec382d0d38f8ef 100644
--- a/libcxxabi/src/demangle/DemangleConfig.h
+++ b/libcxxabi/src/demangle/DemangleConfig.h
@@ -99,6 +99,11 @@
 #define DEMANGLE_FALLTHROUGH
 #endif
 
+#ifndef DEMANGLE_ASSERT
+#include <cassert>
+#define DEMANGLE_ASSERT(__expr, __msg) assert((__expr) && (__msg))
+#endif
+
 #define DEMANGLE_NAMESPACE_BEGIN namespace { namespace itanium_demangle {
 #define DEMANGLE_NAMESPACE_END } }
 

diff  --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index 2336b84da293bcc..0ebfc0ecb74d8e1 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -21,7 +21,6 @@
 #include "Utility.h"
 #include <__cxxabi_config.h>
 #include <algorithm>
-#include <cassert>
 #include <cctype>
 #include <cstdio>
 #include <cstdlib>
@@ -129,12 +128,12 @@ template <class T, size_t N> class PODSmallVector {
 
   // NOLINTNEXTLINE(readability-identifier-naming)
   void pop_back() {
-    assert(Last != First && "Popping empty vector!");
+    DEMANGLE_ASSERT(Last != First, "Popping empty vector!");
     --Last;
   }
 
   void shrinkToSize(size_t Index) {
-    assert(Index <= size() && "shrinkToSize() can't expand!");
+    DEMANGLE_ASSERT(Index <= size(), "shrinkToSize() can't expand!");
     Last = First + Index;
   }
 
@@ -144,11 +143,11 @@ template <class T, size_t N> class PODSmallVector {
   bool empty() const { return First == Last; }
   size_t size() const { return static_cast<size_t>(Last - First); }
   T &back() {
-    assert(Last != First && "Calling back() on empty vector!");
+    DEMANGLE_ASSERT(Last != First, "Calling back() on empty vector!");
     return *(Last - 1);
   }
   T &operator[](size_t Index) {
-    assert(Index < size() && "Invalid access!");
+    DEMANGLE_ASSERT(Index < size(), "Invalid access!");
     return *(begin() + Index);
   }
   void clear() { Last = First; }
@@ -1678,7 +1677,7 @@ class SpecialSubstitution final : public ExpandedSpecialSubstitution {
     std::string_view SV = ExpandedSpecialSubstitution::getBaseName();
     if (isInstantiation()) {
       // The instantiations are typedefs that drop the "basic_" prefix.
-      assert(starts_with(SV, "basic_"));
+      DEMANGLE_ASSERT(starts_with(SV, "basic_"), "");
       SV.remove_prefix(sizeof("basic_") - 1);
     }
     return SV;
@@ -2569,7 +2568,7 @@ void Node::visit(Fn F) const {
     return F(static_cast<const X *>(this));
 #include "ItaniumNodes.def"
   }
-  assert(0 && "unknown mangling node kind");
+  DEMANGLE_ASSERT(0, "unknown mangling node kind");
 }
 
 /// Determine the kind of a node from its type.
@@ -2611,7 +2610,8 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
       Parser->TemplateParams.push_back(&Params);
     }
     ~ScopedTemplateParamList() {
-      assert(Parser->TemplateParams.size() >= OldNumTemplateParamLists);
+      DEMANGLE_ASSERT(Parser->TemplateParams.size() >= OldNumTemplateParamLists,
+                      "");
       Parser->TemplateParams.shrinkToSize(OldNumTemplateParamLists);
     }
     TemplateParamList *params() { return &Params; }
@@ -2692,7 +2692,7 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
   }
 
   NodeArray popTrailingNodeArray(size_t FromPosition) {
-    assert(FromPosition <= Names.size());
+    DEMANGLE_ASSERT(FromPosition <= Names.size(), "");
     NodeArray res =
         makeNodeArray(Names.begin() + (long)FromPosition, Names.end());
     Names.shrinkToSize(FromPosition);
@@ -2859,8 +2859,8 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
     std::string_view getSymbol() const {
       std::string_view Res = Name;
       if (Kind < Unnameable) {
-        assert(starts_with(Res, "operator") &&
-               "operator name does not start with 'operator'");
+        DEMANGLE_ASSERT(starts_with(Res, "operator"),
+                        "operator name does not start with 'operator'");
         Res.remove_prefix(sizeof("operator") - 1);
         if (starts_with(Res, ' '))
           Res.remove_prefix(1);
@@ -3694,7 +3694,7 @@ Node *AbstractManglingParser<Derived, Alloc>::parseUnresolvedName(bool Global) {
     }
   }
 
-  assert(SoFar != nullptr);
+  DEMANGLE_ASSERT(SoFar != nullptr, "");
 
   Node *Base = getDerived().parseBaseUnresolvedName();
   if (Base == nullptr)
@@ -5635,7 +5635,8 @@ Node *AbstractManglingParser<Derived, Alloc>::parseTemplateParam() {
     Node *ForwardRef = make<ForwardTemplateReference>(Index);
     if (!ForwardRef)
       return nullptr;
-    assert(ForwardRef->getKind() == Node::KForwardTemplateReference);
+    DEMANGLE_ASSERT(ForwardRef->getKind() == Node::KForwardTemplateReference,
+                    "");
     ForwardTemplateRefs.push_back(
         static_cast<ForwardTemplateReference *>(ForwardRef));
     return ForwardRef;

diff  --git a/libcxxabi/src/demangle/Utility.h b/libcxxabi/src/demangle/Utility.h
index f8a36f88f8e7b07..f1fad35d60d98e4 100644
--- a/libcxxabi/src/demangle/Utility.h
+++ b/libcxxabi/src/demangle/Utility.h
@@ -19,7 +19,6 @@
 #include "DemangleConfig.h"
 
 #include <array>
-#include <cassert>
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
@@ -159,7 +158,7 @@ class OutputBuffer {
   }
 
   void insert(size_t Pos, const char *S, size_t N) {
-    assert(Pos <= CurrentPosition);
+    DEMANGLE_ASSERT(Pos <= CurrentPosition, "");
     if (N == 0)
       return;
     grow(N);
@@ -172,7 +171,7 @@ class OutputBuffer {
   void setCurrentPosition(size_t NewPos) { CurrentPosition = NewPos; }
 
   char back() const {
-    assert(CurrentPosition);
+    DEMANGLE_ASSERT(CurrentPosition, "");
     return Buffer[CurrentPosition - 1];
   }
 

diff  --git a/libcxxabi/src/fallback_malloc.cpp b/libcxxabi/src/fallback_malloc.cpp
index f9fb1bc46302a9b..fa802b2d81a7452 100644
--- a/libcxxabi/src/fallback_malloc.cpp
+++ b/libcxxabi/src/fallback_malloc.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "fallback_malloc.h"
+#include "abort_message.h"
 
 #include <__threading_support>
 #ifndef _LIBCXXABI_HAS_NO_THREADS
@@ -16,7 +17,7 @@
 #endif
 
 #include <__memory/aligned_alloc.h>
-#include <assert.h>
+#include <__assert>
 #include <stdlib.h> // for malloc, calloc, free
 #include <string.h> // for memset
 
@@ -142,7 +143,7 @@ void* fallback_malloc(size_t len) {
 
     // Check the invariant that all heap_nodes pointers 'p' are aligned
     // so that 'p + 1' has an alignment of at least RequiredAlignment
-    assert(reinterpret_cast<size_t>(p + 1) % RequiredAlignment == 0);
+    _LIBCXXABI_ASSERT(reinterpret_cast<size_t>(p + 1) % RequiredAlignment == 0, "");
 
     // Calculate the number of extra padding elements needed in order
     // to split 'p' and create a properly aligned heap_node from the tail
@@ -163,7 +164,7 @@ void* fallback_malloc(size_t len) {
       q->next_node = 0;
       q->len = static_cast<heap_size>(aligned_nelems);
       void* ptr = q + 1;
-      assert(reinterpret_cast<size_t>(ptr) % RequiredAlignment == 0);
+      _LIBCXXABI_ASSERT(reinterpret_cast<size_t>(ptr) % RequiredAlignment == 0, "");
       return ptr;
     }
 
@@ -176,7 +177,7 @@ void* fallback_malloc(size_t len) {
         prev->next_node = p->next_node;
       p->next_node = 0;
       void* ptr = p + 1;
-      assert(reinterpret_cast<size_t>(ptr) % RequiredAlignment == 0);
+      _LIBCXXABI_ASSERT(reinterpret_cast<size_t>(ptr) % RequiredAlignment == 0, "");
       return ptr;
     }
   }

diff  --git a/libcxxabi/test/test_fallback_malloc.pass.cpp b/libcxxabi/test/test_fallback_malloc.pass.cpp
index ff6ef60710c568f..265a7a309e0a989 100644
--- a/libcxxabi/test/test_fallback_malloc.pass.cpp
+++ b/libcxxabi/test/test_fallback_malloc.pass.cpp
@@ -27,6 +27,8 @@ typedef std::deque<void *> container;
 
 TEST_DIAGNOSTIC_PUSH
 TEST_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#define _LIBCXXABI_ASSERT(expr, msg) assert((expr) && (msg))
+
 // #define  DEBUG_FALLBACK_MALLOC
 #define INSTRUMENT_FALLBACK_MALLOC
 #include "../src/fallback_malloc.cpp"

diff  --git a/llvm/include/llvm/Demangle/DemangleConfig.h b/llvm/include/llvm/Demangle/DemangleConfig.h
index 2ff95dd8d29e5e6..30f72ffe0d7efac 100644
--- a/llvm/include/llvm/Demangle/DemangleConfig.h
+++ b/llvm/include/llvm/Demangle/DemangleConfig.h
@@ -86,6 +86,11 @@
 #define DEMANGLE_FALLTHROUGH
 #endif
 
+#ifndef DEMANGLE_ASSERT
+#include <cassert>
+#define DEMANGLE_ASSERT(__expr, __msg) assert((__expr) && (__msg))
+#endif
+
 #define DEMANGLE_NAMESPACE_BEGIN namespace llvm { namespace itanium_demangle {
 #define DEMANGLE_NAMESPACE_END } }
 

diff  --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index f68c37258ce0992..a3558d2cfd5be83 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -20,7 +20,6 @@
 #include "StringViewExtras.h"
 #include "Utility.h"
 #include <algorithm>
-#include <cassert>
 #include <cctype>
 #include <cstdio>
 #include <cstdlib>
@@ -128,12 +127,12 @@ template <class T, size_t N> class PODSmallVector {
 
   // NOLINTNEXTLINE(readability-identifier-naming)
   void pop_back() {
-    assert(Last != First && "Popping empty vector!");
+    DEMANGLE_ASSERT(Last != First, "Popping empty vector!");
     --Last;
   }
 
   void shrinkToSize(size_t Index) {
-    assert(Index <= size() && "shrinkToSize() can't expand!");
+    DEMANGLE_ASSERT(Index <= size(), "shrinkToSize() can't expand!");
     Last = First + Index;
   }
 
@@ -143,11 +142,11 @@ template <class T, size_t N> class PODSmallVector {
   bool empty() const { return First == Last; }
   size_t size() const { return static_cast<size_t>(Last - First); }
   T &back() {
-    assert(Last != First && "Calling back() on empty vector!");
+    DEMANGLE_ASSERT(Last != First, "Calling back() on empty vector!");
     return *(Last - 1);
   }
   T &operator[](size_t Index) {
-    assert(Index < size() && "Invalid access!");
+    DEMANGLE_ASSERT(Index < size(), "Invalid access!");
     return *(begin() + Index);
   }
   void clear() { Last = First; }
@@ -1677,7 +1676,7 @@ class SpecialSubstitution final : public ExpandedSpecialSubstitution {
     std::string_view SV = ExpandedSpecialSubstitution::getBaseName();
     if (isInstantiation()) {
       // The instantiations are typedefs that drop the "basic_" prefix.
-      assert(starts_with(SV, "basic_"));
+      DEMANGLE_ASSERT(starts_with(SV, "basic_"), "");
       SV.remove_prefix(sizeof("basic_") - 1);
     }
     return SV;
@@ -2568,7 +2567,7 @@ void Node::visit(Fn F) const {
     return F(static_cast<const X *>(this));
 #include "ItaniumNodes.def"
   }
-  assert(0 && "unknown mangling node kind");
+  DEMANGLE_ASSERT(0, "unknown mangling node kind");
 }
 
 /// Determine the kind of a node from its type.
@@ -2610,7 +2609,8 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
       Parser->TemplateParams.push_back(&Params);
     }
     ~ScopedTemplateParamList() {
-      assert(Parser->TemplateParams.size() >= OldNumTemplateParamLists);
+      DEMANGLE_ASSERT(Parser->TemplateParams.size() >= OldNumTemplateParamLists,
+                      "");
       Parser->TemplateParams.shrinkToSize(OldNumTemplateParamLists);
     }
     TemplateParamList *params() { return &Params; }
@@ -2691,7 +2691,7 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
   }
 
   NodeArray popTrailingNodeArray(size_t FromPosition) {
-    assert(FromPosition <= Names.size());
+    DEMANGLE_ASSERT(FromPosition <= Names.size(), "");
     NodeArray res =
         makeNodeArray(Names.begin() + (long)FromPosition, Names.end());
     Names.shrinkToSize(FromPosition);
@@ -2858,8 +2858,8 @@ template <typename Derived, typename Alloc> struct AbstractManglingParser {
     std::string_view getSymbol() const {
       std::string_view Res = Name;
       if (Kind < Unnameable) {
-        assert(starts_with(Res, "operator") &&
-               "operator name does not start with 'operator'");
+        DEMANGLE_ASSERT(starts_with(Res, "operator"),
+                        "operator name does not start with 'operator'");
         Res.remove_prefix(sizeof("operator") - 1);
         if (starts_with(Res, ' '))
           Res.remove_prefix(1);
@@ -3693,7 +3693,7 @@ Node *AbstractManglingParser<Derived, Alloc>::parseUnresolvedName(bool Global) {
     }
   }
 
-  assert(SoFar != nullptr);
+  DEMANGLE_ASSERT(SoFar != nullptr, "");
 
   Node *Base = getDerived().parseBaseUnresolvedName();
   if (Base == nullptr)
@@ -5634,7 +5634,8 @@ Node *AbstractManglingParser<Derived, Alloc>::parseTemplateParam() {
     Node *ForwardRef = make<ForwardTemplateReference>(Index);
     if (!ForwardRef)
       return nullptr;
-    assert(ForwardRef->getKind() == Node::KForwardTemplateReference);
+    DEMANGLE_ASSERT(ForwardRef->getKind() == Node::KForwardTemplateReference,
+                    "");
     ForwardTemplateRefs.push_back(
         static_cast<ForwardTemplateReference *>(ForwardRef));
     return ForwardRef;

diff  --git a/llvm/include/llvm/Demangle/Utility.h b/llvm/include/llvm/Demangle/Utility.h
index 99ed81461ce4138..e893cceea2cdc48 100644
--- a/llvm/include/llvm/Demangle/Utility.h
+++ b/llvm/include/llvm/Demangle/Utility.h
@@ -19,7 +19,6 @@
 #include "DemangleConfig.h"
 
 #include <array>
-#include <cassert>
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
@@ -159,7 +158,7 @@ class OutputBuffer {
   }
 
   void insert(size_t Pos, const char *S, size_t N) {
-    assert(Pos <= CurrentPosition);
+    DEMANGLE_ASSERT(Pos <= CurrentPosition, "");
     if (N == 0)
       return;
     grow(N);
@@ -172,7 +171,7 @@ class OutputBuffer {
   void setCurrentPosition(size_t NewPos) { CurrentPosition = NewPos; }
 
   char back() const {
-    assert(CurrentPosition);
+    DEMANGLE_ASSERT(CurrentPosition, "");
     return Buffer[CurrentPosition - 1];
   }
 


        


More information about the llvm-commits mailing list