From f2b340fc885ad2a12d2d73974eff9c8f4c94192c Mon Sep 17 00:00:00 2001 From: djsollen Date: Fri, 29 Jan 2016 08:51:04 -0800 Subject: 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 --- bench/ChecksumBench.cpp | 2 +- bench/nanobench.cpp | 2 +- gm/textblob.cpp | 2 +- include/config/SkUserConfig.h | 7 ----- include/core/SkPostConfig.h | 45 ++++++++--------------------- include/core/SkTArray.h | 2 +- include/core/SkTypes.h | 10 +++++-- src/core/SkPath.cpp | 2 +- src/core/SkRegionPriv.h | 2 +- src/device/xps/SkXPSDevice.cpp | 3 +- src/gpu/GrAllocator.h | 2 +- src/gpu/GrTRecorder.h | 2 +- src/pathops/SkPathOpsDebug.cpp | 4 +-- src/ports/SkFontHost_win.cpp | 2 +- src/ports/SkFontMgr_fontconfig.cpp | 2 +- src/ports/SkFontMgr_win_dw.cpp | 2 +- src/ports/SkMemory_malloc.cpp | 3 +- src/ports/SkMemory_mozalloc.cpp | 5 ++-- src/ports/SkTypeface_win_dw.cpp | 2 +- src/ports/SkTypeface_win_dw.h | 2 +- tests/CachedDataTest.cpp | 2 +- tests/PathOpsCubicLineIntersectionIdeas.cpp | 20 ++++++------- tools/Stats.h | 2 +- tools/sk_tool_utils_font.cpp | 2 +- 24 files changed, 51 insertions(+), 78 deletions(-) diff --git a/bench/ChecksumBench.cpp b/bench/ChecksumBench.cpp index 92e7da662f..bac7bddc81 100644 --- 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 5a00952145..492cb0b828 100644 --- 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 a38215653d..02f3c878a3 100644 --- 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 ff8a5f3154..a0f6258281 100644 --- 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 387ea5b154..68354a6156 100644 --- a/include/core/SkPostConfig.h +++ b/include/core/SkPostConfig.h @@ -103,17 +103,6 @@ // TODO(mdempsky): Move elsewhere as appropriate. #include -#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 17cbc5c05c..bd255e3868 100644 --- a/include/core/SkTArray.h +++ b/include/core/SkTArray.h @@ -507,7 +507,7 @@ void* operator new(size_t, SkTArray* array, int SkDEBUGCODE(atIndex // exception. template void operator delete(void*, SkTArray* /*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 c112b0364f..0d31efc6cc 100644 --- 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 400b7ab86c..f3c2628c4f 100644 --- 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 160c55d555..c9633c7ad6 100644 --- 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 48233bb554..e4ac2aa742 100644 --- 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 c5d2dc3d38..3305fb00c2 100644 --- a/src/gpu/GrAllocator.h +++ b/src/gpu/GrAllocator.h @@ -389,7 +389,7 @@ template void* operator new(size_t size, GrTAllocator* allocator // to match the op new silences warnings about missing op delete when a constructor throws an // exception. template void operator delete(void*, GrTAllocator*) { - 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 a838ad2955..c42d9e9751 100644 --- a/src/gpu/GrTRecorder.h +++ b/src/gpu/GrTRecorder.h @@ -378,7 +378,7 @@ template void operator delete(void*, GrTRecorder&, const GrTRecorderAllocWrapper&) { // 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 6dd9edd64e..546771e84c 100644 --- a/src/pathops/SkPathOpsDebug.cpp +++ b/src/pathops/SkPathOpsDebug.cpp @@ -1094,13 +1094,13 @@ void SkOpAngle::debugValidateNext() const { const SkOpAngle* next = first; SkTDArray(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 2e03a44578..10476ad434 100644 --- 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 6a40e5080d..bd6a5a2a2a 100644 --- a/src/ports/SkFontMgr_fontconfig.cpp +++ b/src/ports/SkFontMgr_fontconfig.cpp @@ -113,7 +113,7 @@ template class SkAutoFc public: SkAutoFc() : SkAutoTCallVProc >(C()) { T* obj = this->operator T*(); - SK_ALWAYSBREAK(nullptr != obj); + SkASSERT_RELEASE(nullptr != obj); } explicit SkAutoFc(T* obj) : SkAutoTCallVProc >(obj) {} }; diff --git a/src/ports/SkFontMgr_win_dw.cpp b/src/ports/SkFontMgr_win_dw.cpp index 585d809406..ef09210e38 100644 --- 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 6ad01daca0..008352ec4a 100644 --- 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 f070085893..f8a996307e 100644 --- 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 38d09e85ec..160c66ed0e 100644 --- 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 3d6fce0a32..cdbeeaa785 100644 --- 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 c3fc9a47bf..6c44161707 100644 --- 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 6aec8b148a..b23dd0ceaf 100644 --- 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 12c1d35e1f..557a2a51a2 100644 --- 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 af52b37a4e..0f8c96f617 100644 --- 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. -- cgit v1.2.3