Attachment 356822 Details for Bug 192006 – Patch

[patch] Patch

bug-192006-20181207194502.patch (text/plain), 6.86 KB, created by Dominik Inführ on 2018-12-07 10:45:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Dominik Inführ

Created: 2018-12-07 10:45:03 PST

Size: 6.86 KB

patch

obsolete

>Subversion Revision: 238951
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index ac53c4cf7c1eb23870742f6de68a16f3b43f3bbd..00f0bf3361bf9d9bacfb99aa59ab33c49d4266e1 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-12-07  Dominik Infuehr  <dinfuehr@igalia.com>
>+
>+        Record right offset with aligned wide instructions
>+        https://bugs.webkit.org/show_bug.cgi?id=192006
>+
>+        Reviewed by Yusuke Suzuki.
>+
>+        Aligning bytecode instructions inserts nops into the instruction stream.
>+        Emitting an instruction did not record the actual start of the instruction with
>+        aligned instructions, but the nop just before the actual instruction. This was
>+        problematic with the StaticPropertyAnalyzer that used the wrong instruction offset.
>+
>+        * bytecode/InstructionStream.h:
>+        (JSC::InstructionStream::MutableRef::clone):
>+        * bytecompiler/BytecodeGenerator.cpp:
>+        (JSC::BytecodeGenerator::alignWideOpcode):
>+        (JSC::BytecodeGenerator::emitCreateThis):
>+        (JSC::BytecodeGenerator::emitNewObject):
>+        * generator/Opcode.rb:
>+
> 2018-12-05  Mark Lam  <mark.lam@apple.com>
> 
>         speculationFromCell() should speculate non-Identifier strings as SpecString instead of SpecStringVar.
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>index 828aed0469d9fbd81f7a78ff1c106787f1b90bd7..5f69aba5dc9ea65a6421ee4f226e2b192b166a2e 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>@@ -1323,11 +1323,8 @@ void BytecodeGenerator::recordOpcode(OpcodeID opcodeID)
> void BytecodeGenerator::alignWideOpcode()
> {
> #if CPU(NEEDS_ALIGNED_ACCESS)
>-    OpcodeID lastOpcodeID = m_lastOpcodeID;
>-    m_lastOpcodeID = op_end;
>     while ((m_writer.position() + 1) % OpcodeSize::Wide)
>         OpNop::emit<OpcodeSize::Narrow>(this);
>-    recordOpcode(lastOpcodeID);
> #endif
> }
> 
>@@ -2784,9 +2781,9 @@ RegisterID* BytecodeGenerator::emitGetArgument(RegisterID* dst, int32_t index)
> 
> RegisterID* BytecodeGenerator::emitCreateThis(RegisterID* dst)
> {
>-    m_staticPropertyAnalyzer.createThis(dst, m_writer.ref());
>-
>     OpCreateThis::emit(this, dst, dst, 0);
>+    m_staticPropertyAnalyzer.createThis(dst, m_lastInstruction);
>+
>     m_codeBlock->addPropertyAccessInstruction(m_lastInstruction.offset());
>     return dst;
> }
>@@ -2893,9 +2890,9 @@ void BytecodeGenerator::restoreTDZStack(const BytecodeGenerator::PreservedTDZSta
> 
> RegisterID* BytecodeGenerator::emitNewObject(RegisterID* dst)
> {
>-    m_staticPropertyAnalyzer.newObject(dst, m_writer.ref());
>-
>     OpNewObject::emit(this, dst, 0);
>+    m_staticPropertyAnalyzer.newObject(dst, m_lastInstruction);
>+
>     return dst;
> }
> 
>diff --git a/Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h b/Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h
>index fc5166c965015d98c2410ec82f2aef01cdb059c4..01e0769cf6e313bced993115befb84945bf4861f 100644
>--- a/Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h
>+++ b/Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h
>@@ -35,8 +35,8 @@ namespace JSC {
> // is understood to be lossy, and it's OK if it turns out to be wrong sometimes.
> class StaticPropertyAnalyzer {
> public:
>-    void createThis(RegisterID* dst, InstructionStream::MutableRef&& instructionRef);
>-    void newObject(RegisterID* dst, InstructionStream::MutableRef&& instructionRef);
>+    void createThis(RegisterID* dst, InstructionStream::MutableRef instructionRef);
>+    void newObject(RegisterID* dst, InstructionStream::MutableRef instructionRef);
>     void putById(RegisterID* dst, unsigned propertyIndex); // propertyIndex is an index into a uniqued set of strings.
>     void mov(RegisterID* dst, RegisterID* src);
> 
>@@ -50,14 +50,14 @@ private:
>     AnalysisMap m_analyses;
> };
> 
>-inline void StaticPropertyAnalyzer::createThis(RegisterID* dst, InstructionStream::MutableRef&& instructionRef)
>+inline void StaticPropertyAnalyzer::createThis(RegisterID* dst, InstructionStream::MutableRef instructionRef)
> {
>     AnalysisMap::AddResult addResult = m_analyses.add(
>         dst->index(), StaticPropertyAnalysis::create(WTFMove(instructionRef)));
>     ASSERT_UNUSED(addResult, addResult.isNewEntry); // Can't have two 'this' in the same constructor.
> }
> 
>-inline void StaticPropertyAnalyzer::newObject(RegisterID* dst, InstructionStream::MutableRef&& instructionRef)
>+inline void StaticPropertyAnalyzer::newObject(RegisterID* dst, InstructionStream::MutableRef instructionRef)
> {
>     RefPtr<StaticPropertyAnalysis> analysis = StaticPropertyAnalysis::create(WTFMove(instructionRef));
>     AnalysisMap::AddResult addResult = m_analyses.add(dst->index(), analysis);
>diff --git a/Source/JavaScriptCore/generator/Opcode.rb b/Source/JavaScriptCore/generator/Opcode.rb
>index 523ff593c7ead27e89afb675ee3817e7efa5aa84..98555f86b28e7ac691d2b966636688def79fab4c 100644
>--- a/Source/JavaScriptCore/generator/Opcode.rb
>+++ b/Source/JavaScriptCore/generator/Opcode.rb
>@@ -114,9 +114,9 @@ EOF
>         <<-EOF.chomp
>     static void emit(BytecodeGenerator* gen#{typed_args})
>     {
>-        gen->recordOpcode(opcodeID);#{@metadata.create_emitter_local}
>-        emit<OpcodeSize::Narrow, NoAssert, false>(gen#{untyped_args}#{metadata_arg})
>-            || emit<OpcodeSize::Wide, Assert, false>(gen#{untyped_args}#{metadata_arg});
>+        #{@metadata.create_emitter_local}
>+        emit<OpcodeSize::Narrow, NoAssert, true>(gen#{untyped_args}#{metadata_arg})
>+            || emit<OpcodeSize::Wide, Assert, true>(gen#{untyped_args}#{metadata_arg});
>     }
> #{%{
>     template<OpcodeSize size, FitsAssertion shouldAssert = Assert>
>@@ -128,22 +128,22 @@ EOF
>     template<OpcodeSize size, FitsAssertion shouldAssert = Assert, bool recordOpcode = true>
>     static bool emit(BytecodeGenerator* gen#{typed_args}#{metadata_param})
>     {
>-        if (recordOpcode)
>-            gen->recordOpcode(opcodeID);
>-        bool didEmit = emitImpl<size>(gen#{untyped_args}#{metadata_arg});
>+        bool didEmit = emitImpl<size, recordOpcode>(gen#{untyped_args}#{metadata_arg});
>         if (shouldAssert == Assert)
>             ASSERT(didEmit);
>         return didEmit;
>     }
> 
> private:
>-    template<OpcodeSize size>
>+    template<OpcodeSize size, bool recordOpcode>
>     static bool emitImpl(BytecodeGenerator* gen#{typed_args}#{metadata_param})
>     {
>         if (size == OpcodeSize::Wide)
>             gen->alignWideOpcode();
>         if (#{map_fields_with_size("", "size", &:fits_check).join "\n            && "}
>             && (size == OpcodeSize::Wide ? #{op_wide.fits_check(Size::Narrow)} : true)) {
>+            if (recordOpcode)
>+                gen->recordOpcode(opcodeID);
>             if (size == OpcodeSize::Wide)
>                 #{op_wide.fits_write Size::Narrow}
> #{map_fields_with_size("            ", "size", &:fits_write).join "\n"}

Actions: View | Formatted Diff | Diff

Attachments on bug 192006: 355724 | 355725 | 355728 | 355734 | 355858 | 356798 | 356822