Consolidate SK_CRASH and sk_throw into SK_ABORT

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1648343003

Review URL: https://codereview.chromium.org/1648343003

diff --git a/bench/ChecksumBench.cpp b/bench/ChecksumBench.cpp
index 92e7da6..bac7bdd 100644 (file)
--- a/bench/ChecksumBench.cpp
+++ b/bench/ChecksumBench.cpp
@@ -45,7 +45,7 @@ protected:
             case kSHA1_ChecksumType: return "compute_sha1";
             case kMurmur3_ChecksumType: return "compute_murmur3";
 
-            default: SK_CRASH(); return "";
+            default: SK_ABORT("Invalid Type"); return "";
         }
     }
 

diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 5a00952..492cb0b 100644 (file)
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -889,7 +889,7 @@ public:
             log->configOption("clip",
                     SkStringPrintf("%d %d %d %d", fClip.fLeft, fClip.fTop,
                                                   fClip.fRight, fClip.fBottom).c_str());
-            SK_ALWAYSBREAK(fCurrentScale < fScales.count());  // debugging paranoia
+            SkASSERT_RELEASE(fCurrentScale < fScales.count());  // debugging paranoia
             log->configOption("scale", SkStringPrintf("%.2g", fScales[fCurrentScale]).c_str());
             if (fCurrentUseMPD > 0) {
                 SkASSERT(1 == fCurrentUseMPD || 2 == fCurrentUseMPD);

diff --git a/gm/textblob.cpp b/gm/textblob.cpp
index a382156..02f3c87 100644 (file)
--- a/gm/textblob.cpp
+++ b/gm/textblob.cpp
@@ -170,7 +170,7 @@ private:
                     memcpy(buf.pos, pos.begin(), count * sizeof(SkScalar) * 2);
                 } break;
                 default:
-                    SkFAIL("unhandled pos value");
+                    SK_ABORT("unhandled pos value");
                 }
 
                 currentGlyph += count;

diff --git a/include/config/SkUserConfig.h b/include/config/SkUserConfig.h
index ff8a5f3..a0f6258 100644 (file)
--- a/include/config/SkUserConfig.h
+++ b/include/config/SkUserConfig.h
@@ -58,13 +58,6 @@
 //#define SK_DEBUG_GLYPH_CACHE
 //#define SK_DEBUG_PATH
 
-/*  If, in debugging mode, Skia needs to stop (presumably to invoke a debugger)
-    it will call SK_CRASH(). If this is not defined it, it is defined in
-    SkPostConfig.h to write to an illegal address
- */
-//#define SK_CRASH() *(int *)(uintptr_t)0 = 0
-
-
 /*  preconfig will have attempted to determine the endianness of the system,
     but you can change these mutually exclusive flags here.
  */

diff --git a/include/core/SkPostConfig.h b/include/core/SkPostConfig.h
index 387ea5b..68354a6 100644 (file)
--- a/include/core/SkPostConfig.h
+++ b/include/core/SkPostConfig.h
@@ -103,17 +103,6 @@
 // TODO(mdempsky): Move elsewhere as appropriate.
 #include <new>
 
-#ifndef SK_CRASH
-#  ifdef SK_BUILD_FOR_WIN
-#    define SK_CRASH() __debugbreak()
-#  else
-#    if 1   // set to 0 for infinite loop, which can help connecting gdb
-#      define SK_CRASH() do { SkNO_RETURN_HINT(); *(int *)(uintptr_t)0xbbadbeef = 0; } while (false)
-#    else
-#      define SK_CRASH() do { SkNO_RETURN_HINT(); } while (true)
-#    endif
-#  endif
-#endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -148,31 +137,21 @@
 #endif
 
 #if defined(GOOGLE3)
-    // Used as argument to DumpStackTrace in SK_ALWAYSBREAK.
     void SkDebugfForDumpStackTrace(const char* data, void* unused);
+    void DumpStackTrace(int skip_count, void w(const char*, void*), void* arg);
+#  define SK_DUMP_GOOGLE3_STACK() DumpStackTrace(0, SkDebugfForDumpStackTrace, nullptr)
+#else
+#  define SK_DUMP_GOOGLE3_STACK()
 #endif
 
-#ifndef SK_ALWAYSBREAK
-#  if defined(GOOGLE3)
-     void DumpStackTrace(int skip_count, void w(const char*, void*),
-                         void* arg);
-#    define SK_ALWAYSBREAK(cond) do { \
-              if (cond) break; \
-              SkNO_RETURN_HINT(); \
-              SkDebugf("%s:%d: failed assertion \"%s\"\n", __FILE__, __LINE__, #cond); \
-              DumpStackTrace(0, SkDebugfForDumpStackTrace, nullptr); \
-              SK_CRASH(); \
-        } while (false)
-#  elif defined(SK_DEBUG)
-#    define SK_ALWAYSBREAK(cond) do { \
-              if (cond) break; \
-              SkNO_RETURN_HINT(); \
-              SkDebugf("%s:%d: failed assertion \"%s\"\n", __FILE__, __LINE__, #cond); \
-              SK_CRASH(); \
-        } while (false)
-#  else
-#    define SK_ALWAYSBREAK(cond) do { if (cond) break; SK_CRASH(); } while (false)
-#  endif
+#ifndef SK_ABORT
+#  define SK_ABORT(msg) \
+    do { \
+       SkNO_RETURN_HINT(); \
+       SkDebugf("%s:%d: fatal error: \"%s\"\n", __FILE__, __LINE__, #msg); \
+       SK_DUMP_GOOGLE3_STACK(); \
+       sk_abort_no_print(); \
+    } while (false)
 #endif
 
 /**

diff --git a/include/core/SkTArray.h b/include/core/SkTArray.h
index 17cbc5c..bd255e3 100644 (file)
--- a/include/core/SkTArray.h
+++ b/include/core/SkTArray.h
@@ -507,7 +507,7 @@ void* operator new(size_t, SkTArray<T, MEM_COPY>* array, int SkDEBUGCODE(atIndex
 // exception.
 template <typename T, bool MEM_COPY>
 void operator delete(void*, SkTArray<T, MEM_COPY>* /*array*/, int /*atIndex*/) {
-    SK_CRASH();
+    SK_ABORT("Invalid Operation");
 }
 
 // Constructs a new object as the last element of an SkTArray.

diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index c112b03..0d31efc 100644 (file)
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -67,7 +67,7 @@ SK_API extern void sk_out_of_memory(void);
     The platform implementation must not return, but should either throw
     an exception or otherwise exit.
 */
-SK_API extern void sk_throw(void);
+SK_API extern void sk_abort_no_print(void);
 
 enum {
     SK_MALLOC_TEMP  = 0x01, //!< hint to sk_malloc that the requested memory will be freed in the scope of the stack frame
@@ -128,8 +128,10 @@ inline void operator delete(void* p) {
     SK_API void SkDebugf(const char format[], ...);
 #endif
 
+#define SkASSERT_RELEASE(cond)          if(!(cond)) { SK_ABORT(#cond); }
+
 #ifdef SK_DEBUG
-    #define SkASSERT(cond)              SK_ALWAYSBREAK(cond)
+    #define SkASSERT(cond)              SkASSERT_RELEASE(cond)
     #define SkDEBUGFAIL(message)        SkASSERT(false && message)
     #define SkDEBUGFAILF(fmt, ...)      SkASSERTF(false, fmt, ##__VA_ARGS__)
     #define SkDEBUGCODE(code)           code
@@ -150,7 +152,9 @@ inline void operator delete(void* p) {
     #define SkAssertResult(cond)        cond
 #endif
 
-#define SkFAIL(message)                 SK_ALWAYSBREAK(false && message)
+// Legacy macro names for SK_ABORT
+#define SkFAIL(message)                 SK_ABORT(message)
+#define sk_throw()                      SK_ABORT("sk_throw")
 
 // We want to evaluate cond only once, and inside the SkASSERT somewhere so we see its string form.
 // So we use the comma operator to make an SkDebugf that always returns false: we'll evaluate cond,

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 400b7ab..f3c2628 100644 (file)
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -275,7 +275,7 @@ bool SkPath::conservativelyContainsRect(const SkRect& rect) const {
                 orig.set(pts, iter.conicWeight());
                 SkPoint quadPts[5];
                 int count = orig.chopIntoQuadsPOW2(quadPts, 1);
-                SK_ALWAYSBREAK(2 == count);
+                SkASSERT_RELEASE(2 == count);
 
                 if (!check_edge_against_rect(quadPts[0], quadPts[2], rect, direction)) {
                     return false;

diff --git a/src/core/SkRegionPriv.h b/src/core/SkRegionPriv.h
index 160c55d..c9633c7 100644 (file)
--- a/src/core/SkRegionPriv.h
+++ b/src/core/SkRegionPriv.h
@@ -66,7 +66,7 @@ public:
         SkASSERT(count >= SkRegion::kRectRegionRuns);
 
         const int64_t size = sk_64_mul(count, sizeof(RunType)) + sizeof(RunHead);
-        if (count < 0 || !sk_64_isS32(size)) { SK_CRASH(); }
+        if (count < 0 || !sk_64_isS32(size)) { SK_ABORT("Invalid Size"); }
 
         RunHead* head = (RunHead*)sk_malloc_throw(size);
         head->fRefCnt = 1;

diff --git a/src/device/xps/SkXPSDevice.cpp b/src/device/xps/SkXPSDevice.cpp
index 48233bb..e4ac2aa 100644 (file)
--- a/src/device/xps/SkXPSDevice.cpp
+++ b/src/device/xps/SkXPSDevice.cpp
@@ -2044,7 +2044,7 @@ static int num_glyph_guess(SkPaint::TextEncoding encoding, const void* text, siz
     case SkPaint::kGlyphID_TextEncoding:
         return SkToInt(byteLength / 2);
     default:
-        SK_ALWAYSBREAK(true);
+        SK_ABORT("Invalid Text Encoding");
     }
     return 0;
 }
@@ -2254,7 +2254,6 @@ SkBaseDevice* SkXPSDevice::onCreateDevice(const CreateInfo& info, const SkPaint*
 #if 0
     if (SkBaseDevice::kGeneral_Usage == info.fUsage) {
         return nullptr;
-        SK_CRASH();
         //To what stream do we write?
         //SkXPSDevice* dev = new SkXPSDevice(this);
         //SkSize s = SkSize::Make(width, height);

diff --git a/src/gpu/GrAllocator.h b/src/gpu/GrAllocator.h
index c5d2dc3..3305fb0 100644 (file)
--- a/src/gpu/GrAllocator.h
+++ b/src/gpu/GrAllocator.h
@@ -389,7 +389,7 @@ template <typename T> void* operator new(size_t size, GrTAllocator<T>* allocator
 // to match the op new silences warnings about missing op delete when a constructor throws an
 // exception.
 template <typename T> void operator delete(void*, GrTAllocator<T>*) {
-    SK_CRASH();
+    SK_ABORT("Invalid Operation");
 }
 
 #define GrNEW_APPEND_TO_ALLOCATOR(allocator_ptr, type_name, args) \

diff --git a/src/gpu/GrTRecorder.h b/src/gpu/GrTRecorder.h
index a838ad2..c42d9e9 100644 (file)
--- a/src/gpu/GrTRecorder.h
+++ b/src/gpu/GrTRecorder.h
@@ -378,7 +378,7 @@ template <typename TBase, typename TAlign, typename TItem>
 void operator delete(void*, GrTRecorder<TBase, TAlign>&, const GrTRecorderAllocWrapper<TItem>&) {
     // We only provide an operator delete to work around compiler warnings that can come
     // up for an unmatched operator new when compiling with exceptions.
-    SK_CRASH();
+    SK_ABORT("Invalid Operation");
 }
 
 #define GrNEW_APPEND_TO_RECORDER(recorder, type_name, args) \

diff --git a/src/pathops/SkPathOpsDebug.cpp b/src/pathops/SkPathOpsDebug.cpp
index 6dd9edd..546771e 100644 (file)
--- a/src/pathops/SkPathOpsDebug.cpp
+++ b/src/pathops/SkPathOpsDebug.cpp
@@ -1094,13 +1094,13 @@ void SkOpAngle::debugValidateNext() const {
     const SkOpAngle* next = first;
     SkTDArray<const SkOpAngle*>(angles);
     do {
-//        SK_ALWAYSBREAK(next->fSegment->debugContains(next));
+//        SkASSERT_RELEASE(next->fSegment->debugContains(next));
         angles.push(next);
         next = next->next();
         if (next == first) {
             break;
         }
-        SK_ALWAYSBREAK(!angles.contains(next));
+        SkASSERT_RELEASE(!angles.contains(next));
         if (!next) {
             return;
         }

diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index 2e03a44..10476ad 100644 (file)
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -2176,7 +2176,7 @@ int LogFontTypeface::onCharsToGlyphs(const void* chars, Encoding encoding,
         break;
     }
     default:
-        SK_CRASH();
+        SK_ABORT("Invalid Text Encoding");
     }
 
     if (sc) {

diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp
index 6a40e50..bd6a5a2 100644 (file)
--- a/src/ports/SkFontMgr_fontconfig.cpp
+++ b/src/ports/SkFontMgr_fontconfig.cpp
@@ -113,7 +113,7 @@ template <typename T, T* (*C)(), void (*D)(T*)> class SkAutoFc
 public:
     SkAutoFc() : SkAutoTCallVProc<T, FcTDestroy<T, D> >(C()) {
         T* obj = this->operator T*();
-        SK_ALWAYSBREAK(nullptr != obj);
+        SkASSERT_RELEASE(nullptr != obj);
     }
     explicit SkAutoFc(T* obj) : SkAutoTCallVProc<T, FcTDestroy<T, D> >(obj) {}
 };

diff --git a/src/ports/SkFontMgr_win_dw.cpp b/src/ports/SkFontMgr_win_dw.cpp
index 585d809..ef09210 100644 (file)
--- a/src/ports/SkFontMgr_win_dw.cpp
+++ b/src/ports/SkFontMgr_win_dw.cpp
@@ -273,7 +273,7 @@ public:
         if (!SUCCEEDED(fFactory->QueryInterface(&fFactory2))) {
             // IUnknown::QueryInterface states that if it fails, punk will be set to nullptr.
             // http://blogs.msdn.com/b/oldnewthing/archive/2004/03/26/96777.aspx
-            SK_ALWAYSBREAK(nullptr == fFactory2.get());
+            SkASSERT_RELEASE(nullptr == fFactory2.get());
         }
 #endif
         memcpy(fLocaleName.get(), localeName, localeNameLength * sizeof(WCHAR));

diff --git a/src/ports/SkMemory_malloc.cpp b/src/ports/SkMemory_malloc.cpp
index 6ad01da..008352e 100644 (file)
--- a/src/ports/SkMemory_malloc.cpp
+++ b/src/ports/SkMemory_malloc.cpp
@@ -26,8 +26,7 @@ static inline void* throw_on_failure(size_t size, void* p) {
     return p;
 }
 
-void sk_throw() {
-    SkDEBUGFAIL("sk_throw");
+void sk_abort_no_print() {
     abort();
 }
 

diff --git a/src/ports/SkMemory_mozalloc.cpp b/src/ports/SkMemory_mozalloc.cpp
index f070085..f8a9963 100644 (file)
--- a/src/ports/SkMemory_mozalloc.cpp
+++ b/src/ports/SkMemory_mozalloc.cpp
@@ -12,9 +12,8 @@
 #include "mozilla/mozalloc_abort.h"
 #include "mozilla/mozalloc_oom.h"
 
-void sk_throw() {
-    SkDEBUGFAIL("sk_throw");
-    mozalloc_abort("Abort from sk_throw");
+void sk_abort_no_print() {
+    mozalloc_abort("Abort from sk_abort");
 }
 
 void sk_out_of_memory(void) {

diff --git a/src/ports/SkTypeface_win_dw.cpp b/src/ports/SkTypeface_win_dw.cpp
index 38d09e8..160c66e 100644 (file)
--- a/src/ports/SkTypeface_win_dw.cpp
+++ b/src/ports/SkTypeface_win_dw.cpp
@@ -111,7 +111,7 @@ int DWriteFontTypeface::onCharsToGlyphs(const void* chars, Encoding encoding,
         break;
     }
     default:
-        SK_CRASH();
+        SK_ABORT("Invalid Text Encoding");
     }
 
     for (int i = 0; i < glyphCount; ++i) {

diff --git a/src/ports/SkTypeface_win_dw.h b/src/ports/SkTypeface_win_dw.h
index 3d6fce0..cdbeeaa 100644 (file)
--- a/src/ports/SkTypeface_win_dw.h
+++ b/src/ports/SkTypeface_win_dw.h
@@ -55,7 +55,7 @@ private:
         if (!SUCCEEDED(fDWriteFontFace->QueryInterface(&fDWriteFontFace1))) {
             // IUnknown::QueryInterface states that if it fails, punk will be set to nullptr.
             // http://blogs.msdn.com/b/oldnewthing/archive/2004/03/26/96777.aspx
-            SK_ALWAYSBREAK(nullptr == fDWriteFontFace1.get());
+            SkASSERT_RELEASE(nullptr == fDWriteFontFace1.get());
         }
 #endif
     }

diff --git a/tests/CachedDataTest.cpp b/tests/CachedDataTest.cpp
index c3fc9a4..6c44161 100644 (file)
--- a/tests/CachedDataTest.cpp
+++ b/tests/CachedDataTest.cpp
@@ -30,7 +30,7 @@ static SkCachedData* make_data(size_t size, SkDiscardableMemoryPool* pool) {
     if (pool) {
         SkDiscardableMemory* dm = pool->create(size);
         // the pool "can" return null, but it shouldn't in these controlled conditions
-        SK_ALWAYSBREAK(dm);
+        SkASSERT_RELEASE(dm);
         return new SkCachedData(size, dm);
     } else {
         return new SkCachedData(sk_malloc_throw(size), size);

diff --git a/tests/PathOpsCubicLineIntersectionIdeas.cpp b/tests/PathOpsCubicLineIntersectionIdeas.cpp
index 6aec8b1..b23dd0c 100644 (file)
--- a/tests/PathOpsCubicLineIntersectionIdeas.cpp
+++ b/tests/PathOpsCubicLineIntersectionIdeas.cpp
@@ -194,13 +194,13 @@ DEF_TEST(PathOpsCubicLineRoots, reporter) {
             if (realRoots == 3) {
                 smallest = SkTMin(smallest, allRoots[2]);
             }
-            SK_ALWAYSBREAK(smallest < 0);
-            SK_ALWAYSBREAK(smallest >= -1);
+            SkASSERT_RELEASE(smallest < 0);
+            SkASSERT_RELEASE(smallest >= -1);
             largeBits = 0;
         } else {
             frexp(largest, &largeBits);
-            SK_ALWAYSBREAK(largeBits >= 0);
-            SK_ALWAYSBREAK(largeBits < 256);
+            SkASSERT_RELEASE(largeBits >= 0);
+            SkASSERT_RELEASE(largeBits < 256);
         }
         double step = 1e-6;
         if (largeBits > 21) {
@@ -222,7 +222,7 @@ DEF_TEST(PathOpsCubicLineRoots, reporter) {
                 break;
             }
             step *= 1.5;
-            SK_ALWAYSBREAK(step < 1);
+            SkASSERT_RELEASE(step < 1);
         } while (true);
         worstStep[largeBits] = SkTMax(worstStep[largeBits], diff);
 #if 0
@@ -256,11 +256,11 @@ static double testOneFailure(const CubicLineFailures& failure) {
     double allRoots[3] = {0}, validRoots[3] = {0};
     int realRoots = SkDCubic::RootsReal(A, B, C, D, allRoots);
     int valid = SkDQuad::AddValidTs(allRoots, realRoots, validRoots);
-    SK_ALWAYSBREAK(valid == 1);
-    SK_ALWAYSBREAK(realRoots != 1);
+    SkASSERT_RELEASE(valid == 1);
+    SkASSERT_RELEASE(realRoots != 1);
     double t = validRoots[0];
     SkDPoint calcPt = cubic.ptAtT(t);
-    SK_ALWAYSBREAK(!calcPt.approximatelyEqual(pt));
+    SkASSERT_RELEASE(!calcPt.approximatelyEqual(pt));
     int iters = 0;
     double newT = binary_search(cubic, 0.1, pt, t, &iters);
     return newT;
@@ -271,7 +271,7 @@ DEF_TEST(PathOpsCubicLineFailures, reporter) {
     for (int index = 0; index < cubicLineFailuresCount; ++index) {
         const CubicLineFailures& failure = cubicLineFailures[index];
         double newT = testOneFailure(failure);
-        SK_ALWAYSBREAK(newT >= 0);
+        SkASSERT_RELEASE(newT >= 0);
     }
 }
 
@@ -279,5 +279,5 @@ DEF_TEST(PathOpsCubicLineOneFailure, reporter) {
     return;  // disable for now
     const CubicLineFailures& failure = cubicLineFailures[1];
     double newT = testOneFailure(failure);
-    SK_ALWAYSBREAK(newT >= 0);
+    SkASSERT_RELEASE(newT >= 0);
 }

diff --git a/tools/Stats.h b/tools/Stats.h
index 12c1d35..557a2a5 100644 (file)
--- a/tools/Stats.h
+++ b/tools/Stats.h
@@ -62,7 +62,7 @@ struct Stats {
             s /= (max - min);
             s *= (SK_ARRAY_COUNT(kBars) - 1);
             const size_t bar = (size_t)(s + 0.5);
-            SK_ALWAYSBREAK(bar < SK_ARRAY_COUNT(kBars));
+            SkASSERT_RELEASE(bar < SK_ARRAY_COUNT(kBars));
             plot.append(kBars[bar]);
         }
     }

diff --git a/tools/sk_tool_utils_font.cpp b/tools/sk_tool_utils_font.cpp
index af52b37..0f8c96f 100644 (file)
--- a/tools/sk_tool_utils_font.cpp
+++ b/tools/sk_tool_utils_font.cpp
@@ -42,7 +42,7 @@ SkTypeface* create_font(const char* name, SkTypeface::Style style) {
         }
         if (!fontData) {
             // Once all legacy callers to portable fonts are converted, replace this with
-            // SK_CRASH();
+            // SK_ABORT();
             SkDebugf("missing %s %d\n", name, style);
             // If we called SkTypeface::CreateFromName() here we'd recurse infinitely,
             // so we reimplement its core logic here inline without the recursive aspect.