Record right offset with aligned wide instructions
authordinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Dec 2018 14:57:51 +0000 (14:57 +0000)
committerdinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Dec 2018 14:57:51 +0000 (14:57 +0000)
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:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h
Source/JavaScriptCore/generator/Opcode.rb

index 423af98..8e680f0 100644 (file)
@@ -1,3 +1,23 @@
+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>
 
         Align the metadata table on all platforms
index 828aed0..5f69aba 100644 (file)
@@ -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;
 }
 
index fc5166c..01e0769 100644 (file)
@@ -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);
index 523ff59..98555f8 100644 (file)
@@ -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"}