Force crashWithInfo to be out of line.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 Jul 2017 06:24:44 +0000 (06:24 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 Jul 2017 06:24:44 +0000 (06:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174028

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

Update DFG_ASSERT macro to call CRASH_WITH_SECURITY_IMPLICATION_AND_INFO.

* dfg/DFGGraph.cpp:
(JSC::DFG::logDFGAssertionFailure):
(JSC::DFG::Graph::logAssertionFailure):
(JSC::DFG::crash): Deleted.
(JSC::DFG::Graph::handleAssertionFailure): Deleted.
* dfg/DFGGraph.h:

Source/WTF:

The first pass at making crashes hold information about why they
were crashing had the problem that it would inline the assertion.
This meant that clang could coalesce DFG_ASSERTS with other
assertion failures in the same function. This patch moves it out
of line to help fix that issue.

* wtf/Assertions.cpp:
(WTFCrashWithInfo):
* wtf/Assertions.h:
(WTF::isIntegralType):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219046 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/WTF/ChangeLog
Source/WTF/wtf/Assertions.cpp
Source/WTF/wtf/Assertions.h

index 8aed03e..4ba8148 100644 (file)
@@ -1,3 +1,19 @@
+2017-06-30  Keith Miller  <keith_miller@apple.com>
+
+        Force crashWithInfo to be out of line.
+        https://bugs.webkit.org/show_bug.cgi?id=174028
+
+        Reviewed by Filip Pizlo.
+
+        Update DFG_ASSERT macro to call CRASH_WITH_SECURITY_IMPLICATION_AND_INFO.
+
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::logDFGAssertionFailure):
+        (JSC::DFG::Graph::logAssertionFailure):
+        (JSC::DFG::crash): Deleted.
+        (JSC::DFG::Graph::handleAssertionFailure): Deleted.
+        * dfg/DFGGraph.h:
+
 2017-06-30  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Use AbstractMacroAssembler::random instead of holding WeakRandom in JIT
index 6a95720..9424e59 100644 (file)
@@ -1438,7 +1438,7 @@ void Graph::assertIsRegistered(Structure* structure)
     DFG_CRASH(*this, nullptr, toCString("Structure ", pointerDump(structure), " is watchable but isn't being watched.").data());
 }
 
-NO_RETURN_DUE_TO_CRASH static void crash(
+static void logDFGAssertionFailure(
     Graph& graph, const CString& whileText, const char* file, int line, const char* function,
     const char* assertion)
 {
@@ -1452,25 +1452,25 @@ NO_RETURN_DUE_TO_CRASH static void crash(
     dataLog("\n");
     dataLog("DFG ASSERTION FAILED: ", assertion, "\n");
     dataLog(file, "(", line, ") : ", function, "\n");
-    CRASH_WITH_SECURITY_IMPLICATION();
+    WTFReportBacktrace();
 }
 
-void Graph::handleAssertionFailure(
+void Graph::logAssertionFailure(
     std::nullptr_t, const char* file, int line, const char* function, const char* assertion)
 {
-    crash(*this, "", file, line, function, assertion);
+    logDFGAssertionFailure(*this, "", file, line, function, assertion);
 }
 
-void Graph::handleAssertionFailure(
+void Graph::logAssertionFailure(
     Node* node, const char* file, int line, const char* function, const char* assertion)
 {
-    crash(*this, toCString("While handling node ", node, "\n\n"), file, line, function, assertion);
+    logDFGAssertionFailure(*this, toCString("While handling node ", node, "\n\n"), file, line, function, assertion);
 }
 
-void Graph::handleAssertionFailure(
+void Graph::logAssertionFailure(
     BasicBlock* block, const char* file, int line, const char* function, const char* assertion)
 {
-    crash(*this, toCString("While handling block ", pointerDump(block), "\n\n"), file, line, function, assertion);
+    logDFGAssertionFailure(*this, toCString("While handling block ", pointerDump(block), "\n\n"), file, line, function, assertion);
 }
 
 Dominators& Graph::ensureDominators()
index e34beb3..e7e2906 100644 (file)
@@ -93,16 +93,18 @@ template<typename> class FlowMap;
         }                                                               \
     } while (false)
 
-#define DFG_ASSERT(graph, node, assertion) do {                         \
+#define DFG_ASSERT(graph, node, assertion, ...) do {                    \
         if (!!(assertion))                                              \
             break;                                                      \
-        (graph).handleAssertionFailure(                                 \
+        (graph).logAssertionFailure(                                    \
             (node), __FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \
+        CRASH_WITH_SECURITY_IMPLICATION_AND_INFO(__VA_ARGS__);          \
     } while (false)
 
-#define DFG_CRASH(graph, node, reason) do {                             \
-        (graph).handleAssertionFailure(                                 \
+#define DFG_CRASH(graph, node, reason, ...) do {                        \
+        (graph).logAssertionFailure(                                    \
             (node), __FILE__, __LINE__, WTF_PRETTY_FUNCTION, (reason)); \
+        CRASH_WITH_SECURITY_IMPLICATION_AND_INFO(__VA_ARGS__);          \
     } while (false)
 
 struct InlineVariableData {
@@ -887,13 +889,13 @@ public:
     
     void visitChildren(SlotVisitor&) override;
     
-    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+    void logAssertionFailure(
         std::nullptr_t, const char* file, int line, const char* function,
         const char* assertion);
-    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+    void logAssertionFailure(
         Node*, const char* file, int line, const char* function,
         const char* assertion);
-    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+    void logAssertionFailure(
         BasicBlock*, const char* file, int line, const char* function,
         const char* assertion);
 
index 8dc0783..54f3289 100644 (file)
@@ -1,3 +1,21 @@
+2017-06-30  Keith Miller  <keith_miller@apple.com>
+
+        Force crashWithInfo to be out of line.
+        https://bugs.webkit.org/show_bug.cgi?id=174028
+
+        Reviewed by Filip Pizlo.
+
+        The first pass at making crashes hold information about why they
+        were crashing had the problem that it would inline the assertion.
+        This meant that clang could coalesce DFG_ASSERTS with other
+        assertion failures in the same function. This patch moves it out
+        of line to help fix that issue.
+
+        * wtf/Assertions.cpp:
+        (WTFCrashWithInfo):
+        * wtf/Assertions.h:
+        (WTF::isIntegralType):
+
 2017-06-30  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Drop SymbolRegistry::keyForSymbol
index 9142238..8e104cb 100644 (file)
@@ -559,6 +559,87 @@ void WTFInitializeLogChannelStatesFromString(WTFLogChannel* channels[], size_t c
 
 } // extern "C"
 
+#if OS(DARWIN) && (CPU(X64_64) || CPU(ARM64))
+#if CPU(X86_64)
+#define STUFF_REGISTER_FOR_CRASH(reg, info) __asm__ volatile ("movq %0, %%" reg : : "r" (static_cast<uint64_t>(info)) : reg)
+
+// This ordering was chosen to be consistent with JSC's JIT asserts. We probably shouldn't change this ordering
+// since it would make tooling crash reports much harder. If, for whatever reason, we decide to change the ordering
+// here we should update the abortWithuint64_t functions.
+#define STUFF_FOR_CRASH_REGISTER1 "r11"
+#define STUFF_FOR_CRASH_REGISTER2 "r10"
+#define STUFF_FOR_CRASH_REGISTER3 "r9"
+#define STUFF_FOR_CRASH_REGISTER4 "r8"
+#define STUFF_FOR_CRASH_REGISTER5 "r15"
+
+#elif CPU(ARM64) // CPU(X86_64)
+#define STUFF_REGISTER_FOR_CRASH(reg, info) __asm__ volatile ("mov " reg ", %0" : : "r" (static_cast<uint64_t>(info)) : reg)
+
+// See comment above on the ordering.
+#define STUFF_FOR_CRASH_REGISTER1 "x16"
+#define STUFF_FOR_CRASH_REGISTER2 "x17"
+#define STUFF_FOR_CRASH_REGISTER3 "x18"
+#define STUFF_FOR_CRASH_REGISTER4 "x19"
+#define STUFF_FOR_CRASH_REGISTER5 "x20"
+
+#endif // CPU(ARM64)
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t reason, uint64_t misc1, uint64_t misc2, uint64_t misc3, uint64_t misc4)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER2, misc1);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER3, misc2);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER4, misc3);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER5, misc4);
+    CRASH();
+}
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t reason, uint64_t misc1, uint64_t misc2, uint64_t misc3)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER2, misc1);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER3, misc2);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER4, misc3);
+    CRASH();
+}
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t reason, uint64_t misc1, uint64_t misc2)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER2, misc1);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER3, misc2);
+    CRASH();
+}
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t reason, uint64_t misc1)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER2, misc1);
+    CRASH();
+}
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t reason)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    CRASH();
+}
+
+void WTFCrashWithInfo(int, const char*, const char*, int)
+{
+    CRASH();
+}
+
+#else
+
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t, uint64_t, uint64_t, uint64_t, uint64_t) { CRASH(); }
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t, uint64_t, uint64_t, uint64_t) { CRASH(); }
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t, uint64_t, uint64_t) { CRASH(); }
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t, uint64_t) { CRASH(); }
+void WTFCrashWithInfo(int, const char*, const char*, int, uint64_t) { CRASH(); }
+void WTFCrashWithInfo(int, const char*, const char*, int) { CRASH(); }
+
+#endif // OS(DARWIN) && (CPU(X64_64) || CPU(ARM64))
+
 namespace WTF {
 
 void resetAccumulatedLogs()
index 30e0e26..535b3c6 100644 (file)
 #include <os/log.h>
 #endif
 
+#ifdef __cplusplus
+#include <type_traits>
+#endif
+
 #ifdef NDEBUG
 /* Disable ASSERT* macros in release mode. */
 #define ASSERTIONS_DISABLED_DEFAULT 1
@@ -465,5 +469,42 @@ static inline void UNREACHABLE_FOR_PLATFORM()
 #define UNREACHABLE_FOR_PLATFORM() RELEASE_ASSERT_NOT_REACHED()
 #endif
 
+#ifdef __cplusplus
+
+// The combination of line, file, function, and counter should be a unique number per call to this crash. This forces the compiler to not coalesce calls to WTFCrashWithInfo.
+// The easiest way to fill these values per translation unit is to pass __LINE__, __FILE__, WTF_PRETTY_FUNCTION, and __COUNTER__.
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, uint64_t reason, uint64_t misc1, uint64_t misc2, uint64_t misc3, uint64_t misc4);
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, uint64_t reason, uint64_t misc1, uint64_t misc2, uint64_t misc3);
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, uint64_t reason, uint64_t misc1, uint64_t misc2);
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, uint64_t reason, uint64_t misc1);
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, uint64_t reason);
+WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithInfo(int line, const char* file, const char* function, int counter);
+
+
+namespace WTF {
+inline void isIntegralType() { }
+
+template<typename T, typename... Types>
+void isIntegralType(T, Types... types)
+{
+    static_assert(std::is_integral<T>::value || std::is_enum<T>::value, "All types need to be integral bitwise_cast to integral type for logging");
+    isIntegralType(types...);
+}
+}
+
+#ifndef INLINE_CRASH_WITH_SECURITY_IMPLICATION_AND_INFO
+// This is useful if you are going to stuff data into registers before crashing. Like the crashWithInfo functions below...
+// GCC doesn't like the ##__VA_ARGS__ here since this macro is called from another macro so we just CRASH instead there.
+#if COMPILER(CLANG) || COMPILER(MSVC)
+#define CRASH_WITH_SECURITY_IMPLICATION_AND_INFO(...) do { \
+        WTF::isIntegralType(__VA_ARGS__); \
+        WTFCrashWithInfo(__LINE__, __FILE__, WTF_PRETTY_FUNCTION, __COUNTER__, ##__VA_ARGS__); \
+    } while (false)
+#else
+#define CRASH_WITH_SECURITY_IMPLICATION_AND_INFO(...) CRASH()
+#endif
+#endif // INLINE_CRASH_WITH_SECURITY_IMPLICATION_AND_INFO
+
+#endif // __cplusplus
 
 #endif /* WTF_Assertions_h */