WebKit Bugzilla
Attachment 356822 Details for
Bug 192006
: Record right offset with aligned wide instructions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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"}
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192006
:
355724
|
355725
|
355728
|
355734
|
355858
|
356798
| 356822