Attachment 348899 Details for Bug 189300 – proposed patch.

[patch] proposed patch.

bug-189300.patch (text/plain), 6.84 KB, created by Mark Lam on 2018-09-05 00:04:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-09-05 00:04:47 PDT

Size: 6.84 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 235659)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,38 @@
>+2018-09-04  Mark Lam  <mark.lam@apple.com>
>+
>+        Fix DeferredSourceDump to capture the caller bytecodeIndex instead of CodeOrigin.
>+        https://bugs.webkit.org/show_bug.cgi?id=189300
>+        <rdar://problem/39681779>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        At the time a DeferredSourceDump is instantiated, it captures a CodeOrigin value
>+        which points to a InlineCallFrame in the DFG::Plan's m_inlineCallFrames set.  The
>+        DeferredSourceDump is later used to dump source even if the compilation fails.
>+        This is intentional so that we can use this tool to see what source fails to
>+        compile as well.
>+
>+        The DFG::Plan may have been destructed by then, and since the compilation failed,
>+        the InlineCallFrame is also destructed.  This means DeferredSourceDump::dump()
>+        may be end up accessing freed memory.
>+
>+        DeferredSourceDump doesn't really need a CodeOrigin.  All it wants is the caller
>+        bytecodeIndex for the call to an inlined function.  Hence, we can fix this issue
>+        by changing DeferredSourceDump to capture the caller bytecodeIndex instead.
>+
>+        In this patch, we also change DeferredSourceDump's m_codeBlock and m_rootCodeBlock
>+        to be Strong references to ensure that the CodeBlocks are kept alive until they
>+        can be dumped.
>+
>+        * bytecode/DeferredCompilationCallback.cpp:
>+        (JSC::DeferredCompilationCallback::dumpCompiledSourcesIfNeeded):
>+        * bytecode/DeferredSourceDump.cpp:
>+        (JSC::DeferredSourceDump::DeferredSourceDump):
>+        (JSC::DeferredSourceDump::dump):
>+        * bytecode/DeferredSourceDump.h:
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::parseCodeBlock):
>+
> 2018-09-04  Michael Saboff  <msaboff@apple.com>
> 
>         Unreviewed indentations change.
>Index: Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp	(revision 235627)
>+++ Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -65,6 +65,7 @@ void DeferredCompilationCallback::dumpCo
>         dataLog("[", ++index, "] ");
>         info.dump();
>     }
>+    dataLog("\n");
> }
> 
> } // JSC
>Index: Source/JavaScriptCore/bytecode/DeferredSourceDump.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/DeferredSourceDump.cpp	(revision 235627)
>+++ Source/JavaScriptCore/bytecode/DeferredSourceDump.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2015 Apple Inc. All rights reserved.
>+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -28,21 +28,21 @@
> 
> #include "CodeBlock.h"
> #include "CodeBlockWithJITType.h"
>+#include "StrongInlines.h"
> 
> namespace JSC {
> 
> DeferredSourceDump::DeferredSourceDump(CodeBlock* codeBlock)
>-    : m_codeBlock(codeBlock)
>-    , m_rootCodeBlock(nullptr)
>+    : m_codeBlock(*codeBlock->vm(), codeBlock)
>     , m_rootJITType(JITCode::None)
> {
> }
> 
>-DeferredSourceDump::DeferredSourceDump(CodeBlock* codeBlock, CodeBlock* rootCodeBlock, JITCode::JITType rootJITType, CodeOrigin callerCodeOrigin)
>-    : m_codeBlock(codeBlock)
>-    , m_rootCodeBlock(rootCodeBlock)
>+DeferredSourceDump::DeferredSourceDump(CodeBlock* codeBlock, CodeBlock* rootCodeBlock, JITCode::JITType rootJITType, unsigned callerBytecodeIndex)
>+    : m_codeBlock(*codeBlock->vm(), codeBlock)
>+    , m_rootCodeBlock(*codeBlock->vm(), rootCodeBlock)
>     , m_rootJITType(rootJITType)
>-    , m_callerCodeOrigin(callerCodeOrigin)
>+    , m_callerBytecodeIndex(callerBytecodeIndex)
> {
> }
> 
>@@ -56,7 +56,7 @@ void DeferredSourceDump::dump()
>     dataLog(*m_codeBlock);
> 
>     if (isInlinedFrame)
>-        dataLog(" at ", CodeBlockWithJITType(m_rootCodeBlock, m_rootJITType), " ", m_callerCodeOrigin);
>+        dataLog(" at ", CodeBlockWithJITType(*m_rootCodeBlock, m_rootJITType), " ", "bc#", m_callerBytecodeIndex);
> 
>     dataLog("\n'''");
>     m_codeBlock->dumpSource();
>Index: Source/JavaScriptCore/bytecode/DeferredSourceDump.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/DeferredSourceDump.h	(revision 235627)
>+++ Source/JavaScriptCore/bytecode/DeferredSourceDump.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2015 Apple Inc. All rights reserved.
>+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -27,6 +27,7 @@
> 
> #include "CodeOrigin.h"
> #include "JITCode.h"
>+#include "Strong.h"
> 
> namespace JSC {
> 
>@@ -35,15 +36,15 @@ class CodeBlock;
> class DeferredSourceDump {
> public:
>     DeferredSourceDump(CodeBlock*);
>-    DeferredSourceDump(CodeBlock*, CodeBlock* rootCodeBlock, JITCode::JITType rootJITType, CodeOrigin callerCodeOrigin);
>+    DeferredSourceDump(CodeBlock*, CodeBlock* rootCodeBlock, JITCode::JITType rootJITType, unsigned callerBytecodeIndex);
> 
>     void dump();
> 
> private:
>-    CodeBlock* m_codeBlock;
>-    CodeBlock* m_rootCodeBlock;
>+    Strong<CodeBlock> m_codeBlock;
>+    Strong<CodeBlock> m_rootCodeBlock;
>     JITCode::JITType m_rootJITType;
>-    CodeOrigin m_callerCodeOrigin;
>+    unsigned m_callerBytecodeIndex { UINT_MAX };
> };
> 
> } // namespace JSC
>Index: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp	(revision 235627)
>+++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp	(working copy)
>@@ -6952,7 +6952,7 @@ void ByteCodeParser::parseCodeBlock()
>     if (UNLIKELY(Options::dumpSourceAtDFGTime())) {
>         Vector<DeferredSourceDump>& deferredSourceDump = m_graph.m_plan.callback()->ensureDeferredSourceDump();
>         if (inlineCallFrame()) {
>-            DeferredSourceDump dump(codeBlock->baselineVersion(), m_codeBlock, JITCode::DFGJIT, inlineCallFrame()->directCaller);
>+            DeferredSourceDump dump(codeBlock->baselineVersion(), m_codeBlock, JITCode::DFGJIT, inlineCallFrame()->directCaller.bytecodeIndex);
>             deferredSourceDump.append(dump);
>         } else
>             deferredSourceDump.append(DeferredSourceDump(codeBlock->baselineVersion()));

Actions: View | Formatted Diff | Diff

Attachments on bug 189300: 348898 | 348899