[llvm-commits] [PATCH] don't rely on NDEBUG in public-facing headers

nobled nobled at dreamwidth.org
Sun Oct 16 04:02:56 PDT 2011


The current system of using ifndef NDEBUG in public headers
is broken. Clients of the LLVM libraries need to be able to
enable or disable assertions in their own code regardless of
how the version of LLVM they're building against was configured.
Mesa, for example, ran into this and had to work around it:
http://lists.freedesktop.org/archives/mesa-dev/2011-July/009314.html

But right now, any attempt to force the matter by defining or un-defining
NDEBUG by clients like that breaks the ABI, since a number of LLVM
classes in the public headers have members that exist or don't exist
depending on NDEBUG.

This simply replaces NDEBUG in the public headers with a namespaced macro
LLVM_DISABLE_ASSERTIONS. The rest of LLVM's code still uses NDEBUG.

Not entirely sure about the change to the CMake build, because I'm not sure
why the original code avoids adding NDEBUG for MSVC and Xcode.
Anyone know why?
-------------- next part --------------
From 47d98c85d90ae74aea26958784eb6d1eb1de3c9b Mon Sep 17 00:00:00 2001
From: nobled <nobled at dreamwidth.org>
Date: Sat, 15 Oct 2011 00:48:14 +0000
Subject: [PATCH] s/NDEBUG/LLVM_DISABLE_ASSERTIONS

The current system of using ifndef NDEBUG in public headers
is broken. Clients of the LLVM libraries need to be able to
enable or disable assertions in their own code regardless of
how the version of LLVM they're building against was configured.

Right now, any attempt to force the matter by defining or un-defining
NDEBUG by clients breaks the ABI, since a number of LLVM classes
in the public headers have members that exist or don't exist
depending on NDEBUG.

This simply replaces NDEBUG in the public headers with a namespaced macro
LLVM_DISABLE_ASSERTIONS. The rest of LLVM's code still uses NDEBUG
for now.

sed -i -b -s -e "s/NDEBUG/LLVM_DISABLE_ASSERTIONS/" include/llvm/*/*.h
---
 Makefile.rules                                    |    2 +-
 cmake/modules/HandleLLVMOptions.cmake             |    9 +++++++--
 include/llvm/ADT/DenseMap.h                       |    8 ++++----
 include/llvm/ADT/IntervalMap.h                    |    2 +-
 include/llvm/Analysis/LoopInfo.h                  |    2 +-
 include/llvm/Analysis/ScalarEvolutionExpander.h   |    6 +++---
 include/llvm/CodeGen/FunctionLoweringInfo.h       |    4 ++--
 include/llvm/CodeGen/MachineRegisterInfo.h        |    2 +-
 include/llvm/CodeGen/ScheduleDAG.h                |    4 ++--
 include/llvm/CodeGen/ScoreboardHazardRecognizer.h |    2 +-
 include/llvm/CodeGen/SelectionDAG.h               |    2 +-
 include/llvm/MC/MachineLocation.h                 |    2 +-
 include/llvm/Support/Debug.h                      |    2 +-
 include/llvm/Support/ErrorHandling.h              |    6 +++---
 include/llvm/Support/LeakDetector.h               |   12 ++++++------
 include/llvm/Support/ValueHandle.h                |    8 ++++----
 16 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/Makefile.rules b/Makefile.rules
index d057f04..b770eba 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -291,7 +291,7 @@ endif
 # If DISABLE_ASSERTIONS=1 is specified (make command line or configured),
 # then disable assertions by defining the appropriate preprocessor symbols.
 ifeq ($(DISABLE_ASSERTIONS),1)
-  CPP.Defines += -DNDEBUG
+  CPP.Defines += -DNDEBUG -DLLVM_DISABLE_ASSERTIONS
 else
   BuildMode := $(BuildMode)+Asserts
   CPP.Defines += -D_DEBUG
diff --git a/cmake/modules/HandleLLVMOptions.cmake b/cmake/modules/HandleLLVMOptions.cmake
index 9dc1624..1eccee4 100644
--- a/cmake/modules/HandleLLVMOptions.cmake
+++ b/cmake/modules/HandleLLVMOptions.cmake
@@ -38,8 +38,13 @@ if( LLVM_ENABLE_ASSERTIONS )
     add_definitions( -UNDEBUG )
   endif()
 else()
-  if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE" )
-    if( NOT MSVC_IDE AND NOT XCODE )
+  if( NOT MSVC_IDE AND NOT XCODE )
+    add_definitions( -DLLVM_DISABLE_ASSERTIONS )
+    # NDEBUG must be defined if and only if LLVM_DISABLE_ASSERTIONS
+    # is, to maintain binary compatibility with external clients
+    # of the LLVM libraries. But CMake already defines NDEBUG
+    # in Release builds.
+    if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE" )
       add_definitions( -DNDEBUG )
     endif()
   endif()
diff --git a/include/llvm/ADT/DenseMap.h b/include/llvm/ADT/DenseMap.h
index e70cacf..0c29b0c 100644
--- a/include/llvm/ADT/DenseMap.h
+++ b/include/llvm/ADT/DenseMap.h
@@ -71,7 +71,7 @@ public:
         P->second.~ValueT();
       P->first.~KeyT();
     }
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     if (NumBuckets)
       memset((void*)Buckets, 0x5a, sizeof(BucketT)*NumBuckets);
 #endif
@@ -251,7 +251,7 @@ private:
     NumTombstones = other.NumTombstones;
 
     if (NumBuckets) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
       memset((void*)Buckets, 0x5a, sizeof(BucketT)*NumBuckets);
 #endif
       operator delete(Buckets);
@@ -424,7 +424,7 @@ private:
       B->first.~KeyT();
     }
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     if (OldNumBuckets)
       memset((void*)OldBuckets, 0x5a, sizeof(BucketT)*OldNumBuckets);
 #endif
@@ -458,7 +458,7 @@ private:
       B->first.~KeyT();
     }
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     memset((void*)OldBuckets, 0x5a, sizeof(BucketT)*OldNumBuckets);
 #endif
     // Free the old table.
diff --git a/include/llvm/ADT/IntervalMap.h b/include/llvm/ADT/IntervalMap.h
index 1230e8f..b8a5323 100644
--- a/include/llvm/ADT/IntervalMap.h
+++ b/include/llvm/ADT/IntervalMap.h
@@ -342,7 +342,7 @@ void adjustSiblingSizes(NodeT *Node[], unsigned Nodes,
     }
   }
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   for (unsigned n = 0; n != Nodes; n++)
     assert(CurSize[n] == NewSize[n] && "Insufficient element shuffle");
 #endif
diff --git a/include/llvm/Analysis/LoopInfo.h b/include/llvm/Analysis/LoopInfo.h
index 12cb6c5..cba16dc 100644
--- a/include/llvm/Analysis/LoopInfo.h
+++ b/include/llvm/Analysis/LoopInfo.h
@@ -413,7 +413,7 @@ public:
 
   /// verifyLoop - Verify loop structure
   void verifyLoop() const {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     assert(!Blocks.empty() && "Loop header is missing");
 
     // Sort the blocks vector so that we can use binary search to do quick
diff --git a/include/llvm/Analysis/ScalarEvolutionExpander.h b/include/llvm/Analysis/ScalarEvolutionExpander.h
index 1080580..683cc29 100644
--- a/include/llvm/Analysis/ScalarEvolutionExpander.h
+++ b/include/llvm/Analysis/ScalarEvolutionExpander.h
@@ -72,7 +72,7 @@ namespace llvm {
     typedef IRBuilder<true, TargetFolder> BuilderType;
     BuilderType Builder;
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     const char *DebugType;
 #endif
 
@@ -84,12 +84,12 @@ namespace llvm {
       : SE(se), IVName(name), IVIncInsertLoop(0), IVIncInsertPos(0),
         CanonicalMode(true), LSRMode(false),
         Builder(se.getContext(), TargetFolder(se.TD)) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
       DebugType = "";
 #endif
     }
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     void setDebugType(const char* s) { DebugType = s; }
 #endif
 
diff --git a/include/llvm/CodeGen/FunctionLoweringInfo.h b/include/llvm/CodeGen/FunctionLoweringInfo.h
index 09dac85..ad4302d 100644
--- a/include/llvm/CodeGen/FunctionLoweringInfo.h
+++ b/include/llvm/CodeGen/FunctionLoweringInfo.h
@@ -22,7 +22,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/ADT/SmallVector.h"
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
 #include "llvm/ADT/SmallSet.h"
 #endif
 #include "llvm/Analysis/BranchProbabilityInfo.h"
@@ -97,7 +97,7 @@ public:
   /// MBB - The current insert position inside the current block.
   MachineBasicBlock::iterator InsertPt;
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   SmallSet<const Instruction *, 8> CatchInfoLost;
   SmallSet<const Instruction *, 8> CatchInfoFound;
 #endif
diff --git a/include/llvm/CodeGen/MachineRegisterInfo.h b/include/llvm/CodeGen/MachineRegisterInfo.h
index 3866b26..b82f162 100644
--- a/include/llvm/CodeGen/MachineRegisterInfo.h
+++ b/include/llvm/CodeGen/MachineRegisterInfo.h
@@ -198,7 +198,7 @@ public:
   /// preserve conservative kill flag information.
   void clearKillFlags(unsigned Reg) const;
   
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   void dumpUses(unsigned RegNo) const;
 #endif
   
diff --git a/include/llvm/CodeGen/ScheduleDAG.h b/include/llvm/CodeGen/ScheduleDAG.h
index 1bbc6c5..741f76d 100644
--- a/include/llvm/CodeGen/ScheduleDAG.h
+++ b/include/llvm/CodeGen/ScheduleDAG.h
@@ -497,7 +497,7 @@ namespace llvm {
     SUnit EntrySU;                        // Special node for the region entry.
     SUnit ExitSU;                         // Special node for the region exit.
 
-#ifdef NDEBUG
+#ifdef LLVM_DISABLE_ASSERTIONS
     static const bool StressSched = false;
 #else
     bool StressSched;
@@ -536,7 +536,7 @@ namespace llvm {
     /// the ScheduleDAG.
     virtual void addCustomGraphFeatures(GraphWriter<ScheduleDAG*> &) const {}
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     /// VerifySchedule - Verify that all SUnits were scheduled and that
     /// their state is consistent.
     void VerifySchedule(bool isBottomUp);
diff --git a/include/llvm/CodeGen/ScoreboardHazardRecognizer.h b/include/llvm/CodeGen/ScoreboardHazardRecognizer.h
index 060e89a..636c1e8 100644
--- a/include/llvm/CodeGen/ScoreboardHazardRecognizer.h
+++ b/include/llvm/CodeGen/ScoreboardHazardRecognizer.h
@@ -84,7 +84,7 @@ class ScoreboardHazardRecognizer : public ScheduleHazardRecognizer {
     void dump() const;
   };
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   // Support for tracing ScoreboardHazardRecognizer as a component within
   // another module. Follows the current thread-unsafe model of tracing.
   static const char *DebugType;
diff --git a/include/llvm/CodeGen/SelectionDAG.h b/include/llvm/CodeGen/SelectionDAG.h
index 132983c..274958a 100644
--- a/include/llvm/CodeGen/SelectionDAG.h
+++ b/include/llvm/CodeGen/SelectionDAG.h
@@ -210,7 +210,7 @@ public:
   void viewGraph(const std::string &Title);
   void viewGraph();
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   std::map<const SDNode *, std::string> NodeGraphAttrs;
 #endif
 
diff --git a/include/llvm/MC/MachineLocation.h b/include/llvm/MC/MachineLocation.h
index 8ddfdbc..5bcd9bd 100644
--- a/include/llvm/MC/MachineLocation.h
+++ b/include/llvm/MC/MachineLocation.h
@@ -65,7 +65,7 @@ public:
     Offset = O;
   }
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   void dump();
 #endif
 };
diff --git a/include/llvm/Support/Debug.h b/include/llvm/Support/Debug.h
index 8651fc1..a82a33f 100644
--- a/include/llvm/Support/Debug.h
+++ b/include/llvm/Support/Debug.h
@@ -36,7 +36,7 @@ class raw_ostream;
 #define DEBUG_TYPE ""
 #endif
   
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
 /// DebugFlag - This boolean is set to true if the '-debug' command line option
 /// is specified.  This should probably not be referenced directly, instead, use
 /// the DEBUG macro below.
diff --git a/include/llvm/Support/ErrorHandling.h b/include/llvm/Support/ErrorHandling.h
index 95b0109..a6211db 100644
--- a/include/llvm/Support/ErrorHandling.h
+++ b/include/llvm/Support/ErrorHandling.h
@@ -87,14 +87,14 @@ namespace llvm {
 }
 
 /// Marks that the current location is not supposed to be reachable.
-/// In !NDEBUG builds, prints the message and location info to stderr.
-/// In NDEBUG builds, becomes an optimizer hint that the current location
+/// In !LLVM_DISABLE_ASSERTIONS builds, prints the message and location info to stderr.
+/// In LLVM_DISABLE_ASSERTIONS builds, becomes an optimizer hint that the current location
 /// is not supposed to be reachable.  On compilers that don't support
 /// such hints, prints a reduced message instead.
 ///
 /// Use this instead of assert(0).  It conveys intent more clearly and
 /// allows compilers to omit some unnecessary code.
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
 #define llvm_unreachable(msg) \
   ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
 #elif defined(LLVM_BUILTIN_UNREACHABLE)
diff --git a/include/llvm/Support/LeakDetector.h b/include/llvm/Support/LeakDetector.h
index 501a9db..f1be43b 100644
--- a/include/llvm/Support/LeakDetector.h
+++ b/include/llvm/Support/LeakDetector.h
@@ -11,7 +11,7 @@
 // checks for an API.  Basically LLVM uses this to make sure that Instructions,
 // for example, are deleted when they are supposed to be, and not leaked away.
 //
-// When compiling with NDEBUG (Release build), this class does nothing, thus
+// When compiling with LLVM_DISABLE_ASSERTIONS (Release build), this class does nothing, thus
 // adding no checking overhead to release builds.  Note that this class is
 // implemented in a very simple way, requiring completely manual manipulation
 // and checking for garbage, but this is intentional: users should not be using
@@ -35,7 +35,7 @@ struct LeakDetector {
   /// taken out of an owning collection.
   ///
   static void addGarbageObject(void *Object) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     addGarbageObjectImpl(Object);
 #endif
   }
@@ -45,7 +45,7 @@ struct LeakDetector {
   /// an "owning" collection.
   ///
   static void removeGarbageObject(void *Object) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     removeGarbageObjectImpl(Object);
 #endif
   }
@@ -58,7 +58,7 @@ struct LeakDetector {
   /// performed.
   ///
   static void checkForGarbage(LLVMContext &C, const std::string &Message) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     checkForGarbageImpl(C, Message);
 #endif
   }
@@ -68,12 +68,12 @@ struct LeakDetector {
   /// functioning of this class, it just makes the warning messages nicer.
   ///
   static void addGarbageObject(const Value *Object) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     addGarbageObjectImpl(Object);
 #endif
   }
   static void removeGarbageObject(const Value *Object) {
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
     removeGarbageObjectImpl(Object);
 #endif
   }
diff --git a/include/llvm/Support/ValueHandle.h b/include/llvm/Support/ValueHandle.h
index c0cdc35..24c4ebb 100644
--- a/include/llvm/Support/ValueHandle.h
+++ b/include/llvm/Support/ValueHandle.h
@@ -176,12 +176,12 @@ template<> struct simplify_type<WeakVH> : public simplify_type<const WeakVH> {};
 /// class turns into a trivial wrapper around a pointer.
 template <typename ValueTy>
 class AssertingVH
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   : public ValueHandleBase
 #endif
   {
 
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   ValueTy *getValPtr() const {
     return static_cast<ValueTy*>(ValueHandleBase::getValPtr());
   }
@@ -200,7 +200,7 @@ class AssertingVH
   static Value *GetAsValue(const Value *V) { return const_cast<Value*>(V); }
 
 public:
-#ifndef NDEBUG
+#ifndef LLVM_DISABLE_ASSERTIONS
   AssertingVH() : ValueHandleBase(Assert) {}
   AssertingVH(ValueTy *P) : ValueHandleBase(Assert, GetAsValue(P)) {}
   AssertingVH(const AssertingVH &RHS) : ValueHandleBase(Assert, RHS) {}
@@ -258,7 +258,7 @@ struct DenseMapInfo<AssertingVH<T> > {
   
 template <typename T>
 struct isPodLike<AssertingVH<T> > {
-#ifdef NDEBUG
+#ifdef LLVM_DISABLE_ASSERTIONS
   static const bool value = true;
 #else
   static const bool value = false;
-- 
1.7.6.msysgit.0



More information about the llvm-commits mailing list