Changeset 239009 in webkit

Timestamp:

Dec 8, 2018 6:57:51 AM (5 years ago)

Author:

dinfuehr@igalia.com

Message:

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:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecompiler/BytecodeGenerator.cpp (modified) (3 diffs)
• bytecompiler/StaticPropertyAnalyzer.h (modified) (3 diffs)
• generator/Opcode.rb (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-12-08  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-07  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -1324 +1324 @@
 {
 #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
 }
@@ -2785 +2782 @@
 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;
@@ -2894 +2891 @@
 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;
 }

trunk/Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h

@@ -36 +36 @@
 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);
@@ -51 +51 @@
 };
 
-inline void StaticPropertyAnalyzer::createThis(RegisterID* dst, InstructionStream::MutableRef&& instructionRef)
+inline void StaticPropertyAnalyzer::createThis(RegisterID* dst, InstructionStream::MutableRef instructionRef)
 {
     AnalysisMap::AddResult addResult = m_analyses.add(
@@ -58 +58 @@
 }
 
-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));

trunk/Source/JavaScriptCore/generator/Opcode.rb

@@ -115 +115 @@
     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});
     }
 #{%{
@@ -129 +129 @@
     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);
@@ -138 +136 @@
 
 private:
-    template<OpcodeSize size>
+    template<OpcodeSize size, bool recordOpcode>
     static bool emitImpl(BytecodeGenerator* gen#{typed_args}#{metadata_param})
     {
@@ -145 +143 @@
         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}