DFG_ASSERT should allow stuffing registers before trapping.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jun 2017 08:23:18 +0000 (08:23 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jun 2017 08:23:18 +0000 (08:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174005

Reviewed by Mark Lam.

Source/JavaScriptCore:

DFG_ASSERT currently prints error data to stderr before crashing,
which is nice for local development. In the wild, however, we
can't see this information in crash logs. This patch enables
stuffing some of the most useful information from DFG_ASSERTS into
up to five registers right before crashing. The values stuffed
should not impact any logging during local development.

* assembler/AbortReason.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::verifyEdge):
* dfg/DFGGraph.cpp:
(JSC::DFG::logForCrash):
(JSC::DFG::Graph::logAssertionFailure):
(JSC::DFG::crash): Deleted.
(JSC::DFG::Graph::handleAssertionFailure): Deleted.
* dfg/DFGGraph.h:

Source/WTF:

Add new template functions that enable stuffing up to five values
into registers before crashing.

* wtf/Assertions.h:
(CRASH_WITH_INFO):

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

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

index 53fd9ff..9bf9067 100644 (file)
@@ -1,3 +1,27 @@
+2017-06-30  Keith Miller  <keith_miller@apple.com>
+
+        DFG_ASSERT should allow stuffing registers before trapping.
+        https://bugs.webkit.org/show_bug.cgi?id=174005
+
+        Reviewed by Mark Lam.
+
+        DFG_ASSERT currently prints error data to stderr before crashing,
+        which is nice for local development. In the wild, however, we
+        can't see this information in crash logs. This patch enables
+        stuffing some of the most useful information from DFG_ASSERTS into
+        up to five registers right before crashing. The values stuffed
+        should not impact any logging during local development.
+
+        * assembler/AbortReason.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::verifyEdge):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::logForCrash):
+        (JSC::DFG::Graph::logAssertionFailure):
+        (JSC::DFG::crash): Deleted.
+        (JSC::DFG::Graph::handleAssertionFailure): Deleted.
+        * dfg/DFGGraph.h:
+
 2017-06-29  Saam Barati  <sbarati@apple.com>
 
         Calculating postCapacity in unshiftCountSlowCase is wrong
index 32ae086..e932e6a 100644 (file)
@@ -76,4 +76,9 @@ enum AbortReason {
     YARRNoInputConsumed                               = 340,
 };
 
+// See above for comment on numbering. This enum is for crashes during compilation.
+enum CompilationAbortReason {
+    AIEdgeVerificationFailed                          = 10,
+};
+
 } // namespace JSC
index c73a278..af86f00 100644 (file)
@@ -130,8 +130,8 @@ void AbstractInterpreter<AbstractStateType>::verifyEdge(Node* node, Edge edge)
 {
     if (!(forNode(edge).m_type & ~typeFilterFor(edge.useKind())))
         return;
-    
-    DFG_CRASH(m_graph, node, toCString("Edge verification error: ", node, "->", edge, " was expected to have type ", SpeculationDump(typeFilterFor(edge.useKind())), " but has type ", SpeculationDump(forNode(edge).m_type), " (", forNode(edge).m_type, ")").data());
+
+    DFG_CRASH(m_graph, node, toCString("Edge verification error: ", node, "->", edge, " was expected to have type ", SpeculationDump(typeFilterFor(edge.useKind())), " but has type ", SpeculationDump(forNode(edge).m_type), " (", forNode(edge).m_type, ")").data(), AIEdgeVerificationFailed, node->op(), edge->op(), forNode(edge).m_type, typeFilterFor(edge.useKind()));
 }
 
 template<typename AbstractStateType>
index 6a95720..5aff704 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 logForCrash(
     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);
+    logForCrash(*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);
+    logForCrash(*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);
+    logForCrash(*this, toCString("While handling block ", pointerDump(block), "\n\n"), file, line, function, assertion);
 }
 
 Dominators& Graph::ensureDominators()
index e34beb3..7a868ce 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_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_INFO(__VA_ARGS__);                                       \
     } while (false)
 
 struct InlineVariableData {
@@ -886,14 +888,15 @@ public:
     void registerFrozenValues();
     
     void visitChildren(SlotVisitor&) override;
-    
-    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+
+    // These should not be called directly. Instead they should be called through the DFG_CRASH/DFG_ASSERT macros.
+    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 b314440..750f799 100644 (file)
@@ -1,3 +1,16 @@
+2017-06-30  Keith Miller  <keith_miller@apple.com>
+
+        DFG_ASSERT should allow stuffing registers before trapping.
+        https://bugs.webkit.org/show_bug.cgi?id=174005
+
+        Reviewed by Mark Lam.
+
+        Add new template functions that enable stuffing up to five values
+        into registers before crashing.
+
+        * wtf/Assertions.h:
+        (CRASH_WITH_INFO):
+
 2017-06-28  Brent Fulgham  <bfulgham@apple.com>
 
         Teach ResourceLoadStatistics to recognize changes in the file system
index 30e0e26..7c3fdfc 100644 (file)
@@ -465,5 +465,102 @@ static inline void UNREACHABLE_FOR_PLATFORM()
 #define UNREACHABLE_FOR_PLATFORM() RELEASE_ASSERT_NOT_REACHED()
 #endif
 
+#ifndef INLINE_CRASH_WITH_SECURITY_IMPLICATION
+// This is useful if you are going to stuff data into registers before crashing. Like the CRASH_WITH_INFO functions below...
+#define INLINE_CRASH_WITH_SECURITY_IMPLICATION() CRASH()
+#endif
+
+#ifdef __cplusplus
+
+#if OS(DARWIN) && (CPU(X64_64) || CPU(ARM64))
+#if CPU(X86_64)
+#define STUFF_REGISTER_FOR_CRASH(reg, value) \
+    static_assert(std::is_integral<decltype(value)>::value, "STUFF_REGISTERS_FOR_CRASH expects an integral input"); \
+    __asm__ volatile ("movq %0, %%" reg : : "r" (static_cast<uint64_t>(value)) : reg)
+
+// This ordering was chosen to be consistant 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 abortWithReason 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, value) \
+    static_assert(std::is_integral<decltype(value)>::value, "STUFF_REGISTERS_FOR_CRASH expects an integral input"); \
+    __asm__ volatile ("mov " reg ", %0" : : "r" (static_cast<uint64_t>(value)) : 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)
+
+template<typename Reason, typename A, typename B, typename C, typename D>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Reason reason, A misc1, B misc2, C misc3, D 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);
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+template<typename Reason, typename A, typename B, typename C>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Reason reason, A misc1, B misc2, C 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);
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+template<typename Reason, typename A, typename B>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Reason reason, A misc1, B 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);
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+template<typename Reason, typename A>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Reason reason, A misc1)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER2, misc1);
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+template<typename Reason>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Reason reason)
+{
+    STUFF_REGISTER_FOR_CRASH(STUFF_FOR_CRASH_REGISTER1, reason);
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO()
+{
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+#else // OS(DARWIN) && (CPU(X64_64) || CPU(ARM64))
+
+template<typename... Types>
+ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH_WITH_INFO(Types...)
+{
+    INLINE_CRASH_WITH_SECURITY_IMPLICATION();
+}
+
+#endif // OS(DARWIN) && (CPU(X64_64) || CPU(ARM64))
+
+#endif // __cplusplus
 
 #endif /* WTF_Assertions_h */