Revert of Consolidate SK_CRASH and sk_throw into SK_ABORT (patchset #5 id:80001 of https://codereview.chromium.org/1610823002/ )
Reason for revert:
Chrome is calling SK_CRASH
Original issue's description:
> Consolidate SK_CRASH and sk_throw into SK_ABORT
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1610823002
>
> Committed: https://skia.googlesource.com/skia/+/4c5cd7d527ed29aabfa72aa47b23a4496eeda357
TBR=reed@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1620333002
diff --git a/bench/ChecksumBench.cpp b/bench/ChecksumBench.cpp
index bac7bdd..92e7da6 100644
--- a/bench/ChecksumBench.cpp
+++ b/bench/ChecksumBench.cpp
@@ -45,7 +45,7 @@
case kSHA1_ChecksumType: return "compute_sha1";
case kMurmur3_ChecksumType: return "compute_murmur3";
- default: SK_ABORT("Invalid Type"); return "";
+ default: SK_CRASH(); return "";
}
}
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index b461a63..4fc4044 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -947,7 +947,7 @@
log->configOption("clip",
SkStringPrintf("%d %d %d %d", fClip.fLeft, fClip.fTop,
fClip.fRight, fClip.fBottom).c_str());
- SkASSERT_RELEASE(fCurrentScale < fScales.count()); // debugging paranoia
+ SK_ALWAYSBREAK(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 02f3c87..a382156 100644
--- a/gm/textblob.cpp
+++ b/gm/textblob.cpp
@@ -170,7 +170,7 @@
memcpy(buf.pos, pos.begin(), count * sizeof(SkScalar) * 2);
} break;
default:
- SK_ABORT("unhandled pos value");
+ SkFAIL("unhandled pos value");
}
currentGlyph += count;
diff --git a/include/config/SkUserConfig.h b/include/config/SkUserConfig.h
index a0f6258..ff8a5f3 100644
--- a/include/config/SkUserConfig.h
+++ b/include/config/SkUserConfig.h
@@ -58,6 +58,13 @@
//#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 68354a6..387ea5b 100644
--- a/include/core/SkPostConfig.h
+++ b/include/core/SkPostConfig.h
@@ -103,6 +103,17 @@
// 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
///////////////////////////////////////////////////////////////////////////////
@@ -137,21 +148,31 @@
#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_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)
+#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
#endif
/**
diff --git a/include/core/SkTArray.h b/include/core/SkTArray.h
index bd255e3..17cbc5c 100644
--- a/include/core/SkTArray.h
+++ b/include/core/SkTArray.h
@@ -507,7 +507,7 @@
// exception.
template <typename T, bool MEM_COPY>
void operator delete(void*, SkTArray<T, MEM_COPY>* /*array*/, int /*atIndex*/) {
- SK_ABORT("Invalid Operation");
+ SK_CRASH();
}
// Constructs a new object as the last element of an SkTArray.
diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 0d31efc..c112b03 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -67,7 +67,7 @@
The platform implementation must not return, but should either throw
an exception or otherwise exit.
*/
-SK_API extern void sk_abort_no_print(void);
+SK_API extern void sk_throw(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,10 +128,8 @@
SK_API void SkDebugf(const char format[], ...);
#endif
-#define SkASSERT_RELEASE(cond) if(!(cond)) { SK_ABORT(#cond); }
-
#ifdef SK_DEBUG
- #define SkASSERT(cond) SkASSERT_RELEASE(cond)
+ #define SkASSERT(cond) SK_ALWAYSBREAK(cond)
#define SkDEBUGFAIL(message) SkASSERT(false && message)
#define SkDEBUGFAILF(fmt, ...) SkASSERTF(false, fmt, ##__VA_ARGS__)
#define SkDEBUGCODE(code) code
@@ -152,9 +150,7 @@
#define SkAssertResult(cond) cond
#endif
-// Legacy macro names for SK_ABORT
-#define SkFAIL(message) SK_ABORT(message)
-#define sk_throw() SK_ABORT("sk_throw")
+#define SkFAIL(message) SK_ALWAYSBREAK(false && message)
// 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 33b7451..c77a25b 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -275,7 +275,7 @@
orig.set(pts, iter.conicWeight());
SkPoint quadPts[5];
int count = orig.chopIntoQuadsPOW2(quadPts, 1);
- SkASSERT_RELEASE(2 == count);
+ SK_ALWAYSBREAK(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 c9633c7..160c55d 100644
--- a/src/core/SkRegionPriv.h
+++ b/src/core/SkRegionPriv.h
@@ -66,7 +66,7 @@
SkASSERT(count >= SkRegion::kRectRegionRuns);
const int64_t size = sk_64_mul(count, sizeof(RunType)) + sizeof(RunHead);
- if (count < 0 || !sk_64_isS32(size)) { SK_ABORT("Invalid Size"); }
+ if (count < 0 || !sk_64_isS32(size)) { SK_CRASH(); }
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 e4ac2aa..48233bb 100644
--- a/src/device/xps/SkXPSDevice.cpp
+++ b/src/device/xps/SkXPSDevice.cpp
@@ -2044,7 +2044,7 @@
case SkPaint::kGlyphID_TextEncoding:
return SkToInt(byteLength / 2);
default:
- SK_ABORT("Invalid Text Encoding");
+ SK_ALWAYSBREAK(true);
}
return 0;
}
@@ -2254,6 +2254,7 @@
#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 3305fb0..c5d2dc3 100644
--- a/src/gpu/GrAllocator.h
+++ b/src/gpu/GrAllocator.h
@@ -389,7 +389,7 @@
// 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_ABORT("Invalid Operation");
+ SK_CRASH();
}
#define GrNEW_APPEND_TO_ALLOCATOR(allocator_ptr, type_name, args) \
diff --git a/src/gpu/GrTRecorder.h b/src/gpu/GrTRecorder.h
index c42d9e9..a838ad2 100644
--- a/src/gpu/GrTRecorder.h
+++ b/src/gpu/GrTRecorder.h
@@ -378,7 +378,7 @@
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_ABORT("Invalid Operation");
+ SK_CRASH();
}
#define GrNEW_APPEND_TO_RECORDER(recorder, type_name, args) \
diff --git a/src/pathops/SkPathOpsDebug.cpp b/src/pathops/SkPathOpsDebug.cpp
index 546771e..6dd9edd 100644
--- a/src/pathops/SkPathOpsDebug.cpp
+++ b/src/pathops/SkPathOpsDebug.cpp
@@ -1094,13 +1094,13 @@
const SkOpAngle* next = first;
SkTDArray<const SkOpAngle*>(angles);
do {
-// SkASSERT_RELEASE(next->fSegment->debugContains(next));
+// SK_ALWAYSBREAK(next->fSegment->debugContains(next));
angles.push(next);
next = next->next();
if (next == first) {
break;
}
- SkASSERT_RELEASE(!angles.contains(next));
+ SK_ALWAYSBREAK(!angles.contains(next));
if (!next) {
return;
}
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index 10476ad..2e03a44 100644
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -2176,7 +2176,7 @@
break;
}
default:
- SK_ABORT("Invalid Text Encoding");
+ SK_CRASH();
}
if (sc) {
diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp
index bd6a5a2..6a40e50 100644
--- a/src/ports/SkFontMgr_fontconfig.cpp
+++ b/src/ports/SkFontMgr_fontconfig.cpp
@@ -113,7 +113,7 @@
public:
SkAutoFc() : SkAutoTCallVProc<T, FcTDestroy<T, D> >(C()) {
T* obj = this->operator T*();
- SkASSERT_RELEASE(nullptr != obj);
+ SK_ALWAYSBREAK(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 ef09210..585d809 100644
--- a/src/ports/SkFontMgr_win_dw.cpp
+++ b/src/ports/SkFontMgr_win_dw.cpp
@@ -273,7 +273,7 @@
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
- SkASSERT_RELEASE(nullptr == fFactory2.get());
+ SK_ALWAYSBREAK(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 008352e..6ad01da 100644
--- a/src/ports/SkMemory_malloc.cpp
+++ b/src/ports/SkMemory_malloc.cpp
@@ -26,7 +26,8 @@
return p;
}
-void sk_abort_no_print() {
+void sk_throw() {
+ SkDEBUGFAIL("sk_throw");
abort();
}
diff --git a/src/ports/SkMemory_mozalloc.cpp b/src/ports/SkMemory_mozalloc.cpp
index f8a9963..f070085 100644
--- a/src/ports/SkMemory_mozalloc.cpp
+++ b/src/ports/SkMemory_mozalloc.cpp
@@ -12,8 +12,9 @@
#include "mozilla/mozalloc_abort.h"
#include "mozilla/mozalloc_oom.h"
-void sk_abort_no_print() {
- mozalloc_abort("Abort from sk_abort");
+void sk_throw() {
+ SkDEBUGFAIL("sk_throw");
+ mozalloc_abort("Abort from sk_throw");
}
void sk_out_of_memory(void) {
diff --git a/src/ports/SkTypeface_win_dw.cpp b/src/ports/SkTypeface_win_dw.cpp
index 160c66e..38d09e8 100644
--- a/src/ports/SkTypeface_win_dw.cpp
+++ b/src/ports/SkTypeface_win_dw.cpp
@@ -111,7 +111,7 @@
break;
}
default:
- SK_ABORT("Invalid Text Encoding");
+ SK_CRASH();
}
for (int i = 0; i < glyphCount; ++i) {
diff --git a/src/ports/SkTypeface_win_dw.h b/src/ports/SkTypeface_win_dw.h
index cdbeeaa..3d6fce0 100644
--- a/src/ports/SkTypeface_win_dw.h
+++ b/src/ports/SkTypeface_win_dw.h
@@ -55,7 +55,7 @@
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
- SkASSERT_RELEASE(nullptr == fDWriteFontFace1.get());
+ SK_ALWAYSBREAK(nullptr == fDWriteFontFace1.get());
}
#endif
}
diff --git a/tests/CachedDataTest.cpp b/tests/CachedDataTest.cpp
index 6c44161..c3fc9a4 100644
--- a/tests/CachedDataTest.cpp
+++ b/tests/CachedDataTest.cpp
@@ -30,7 +30,7 @@
if (pool) {
SkDiscardableMemory* dm = pool->create(size);
// the pool "can" return null, but it shouldn't in these controlled conditions
- SkASSERT_RELEASE(dm);
+ SK_ALWAYSBREAK(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 b23dd0c..6aec8b1 100644
--- a/tests/PathOpsCubicLineIntersectionIdeas.cpp
+++ b/tests/PathOpsCubicLineIntersectionIdeas.cpp
@@ -194,13 +194,13 @@
if (realRoots == 3) {
smallest = SkTMin(smallest, allRoots[2]);
}
- SkASSERT_RELEASE(smallest < 0);
- SkASSERT_RELEASE(smallest >= -1);
+ SK_ALWAYSBREAK(smallest < 0);
+ SK_ALWAYSBREAK(smallest >= -1);
largeBits = 0;
} else {
frexp(largest, &largeBits);
- SkASSERT_RELEASE(largeBits >= 0);
- SkASSERT_RELEASE(largeBits < 256);
+ SK_ALWAYSBREAK(largeBits >= 0);
+ SK_ALWAYSBREAK(largeBits < 256);
}
double step = 1e-6;
if (largeBits > 21) {
@@ -222,7 +222,7 @@
break;
}
step *= 1.5;
- SkASSERT_RELEASE(step < 1);
+ SK_ALWAYSBREAK(step < 1);
} while (true);
worstStep[largeBits] = SkTMax(worstStep[largeBits], diff);
#if 0
@@ -256,11 +256,11 @@
double allRoots[3] = {0}, validRoots[3] = {0};
int realRoots = SkDCubic::RootsReal(A, B, C, D, allRoots);
int valid = SkDQuad::AddValidTs(allRoots, realRoots, validRoots);
- SkASSERT_RELEASE(valid == 1);
- SkASSERT_RELEASE(realRoots != 1);
+ SK_ALWAYSBREAK(valid == 1);
+ SK_ALWAYSBREAK(realRoots != 1);
double t = validRoots[0];
SkDPoint calcPt = cubic.ptAtT(t);
- SkASSERT_RELEASE(!calcPt.approximatelyEqual(pt));
+ SK_ALWAYSBREAK(!calcPt.approximatelyEqual(pt));
int iters = 0;
double newT = binary_search(cubic, 0.1, pt, t, &iters);
return newT;
@@ -271,7 +271,7 @@
for (int index = 0; index < cubicLineFailuresCount; ++index) {
const CubicLineFailures& failure = cubicLineFailures[index];
double newT = testOneFailure(failure);
- SkASSERT_RELEASE(newT >= 0);
+ SK_ALWAYSBREAK(newT >= 0);
}
}
@@ -279,5 +279,5 @@
return; // disable for now
const CubicLineFailures& failure = cubicLineFailures[1];
double newT = testOneFailure(failure);
- SkASSERT_RELEASE(newT >= 0);
+ SK_ALWAYSBREAK(newT >= 0);
}
diff --git a/tools/Stats.h b/tools/Stats.h
index 557a2a5..12c1d35 100644
--- a/tools/Stats.h
+++ b/tools/Stats.h
@@ -62,7 +62,7 @@
s /= (max - min);
s *= (SK_ARRAY_COUNT(kBars) - 1);
const size_t bar = (size_t)(s + 0.5);
- SkASSERT_RELEASE(bar < SK_ARRAY_COUNT(kBars));
+ SK_ALWAYSBREAK(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 0f8c96f..af52b37 100644
--- a/tools/sk_tool_utils_font.cpp
+++ b/tools/sk_tool_utils_font.cpp
@@ -42,7 +42,7 @@
}
if (!fontData) {
// Once all legacy callers to portable fonts are converted, replace this with
- // SK_ABORT();
+ // SK_CRASH();
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.