[MERGE chakra-core#4429 @Cellule] OS#15089337 WASM: Do not use StatementReader in IRBuilder

Cellule · Cellule · commit 2c67a8290303 · 2017-12-18T18:02:09.000-08:00
Merge pull request chakra-core#4429 from Cellule:statement_reader Fix usage of Statement Reader in IRBuilderAsmJs to make sure we don't use uninitialized values. Don't even allocate a statement reader for wasm since there are no statements to use.
diff --git a/lib/Backend/IRBuilderAsmJs.cpp b/lib/Backend/IRBuilderAsmJs.cpp
@@ -16,7 +16,7 @@ IRBuilderAsmJs::Build()
     m_tempAlloc = &localAlloc;
 
     uint32 offset;
-    uint32 statementIndex = m_statementReader.GetStatementIndex();
+    uint32 statementIndex = m_statementReader ? m_statementReader->GetStatementIndex() : Js::Constants::NoStatementIndex;
 
     m_argStack = JitAnew(m_tempAlloc, SListCounted<IR::Instr *>, m_tempAlloc);
     m_tempList = JitAnew(m_tempAlloc, SList<IR::Instr *>, m_tempAlloc);
@@ -124,7 +124,7 @@ IRBuilderAsmJs::Build()
         BuildImplicitArgIns();
     }
 
-    if (m_statementReader.AtStatementBoundary(&m_jnReader))
+    if (m_statementReader && m_statementReader->AtStatementBoundary(&m_jnReader))
     {
         statementIndex = AddStatementBoundary(statementIndex, offset);
     }
@@ -164,14 +164,14 @@ IRBuilderAsmJs::Build()
         }
         offset = m_jnReader.GetCurrentOffset();
 
-        if (m_statementReader.AtStatementBoundary(&m_jnReader))
+        if (m_statementReader && m_statementReader->AtStatementBoundary(&m_jnReader))
         {
             statementIndex = AddStatementBoundary(statementIndex, offset);
         }
 
     }
 
-    if (Js::Constants::NoStatementIndex != statementIndex)
+    if (m_statementReader && Js::Constants::NoStatementIndex != statementIndex)
     {
         statementIndex = AddStatementBoundary(statementIndex, Js::Constants::NoByteCodeOffset);
     }
@@ -422,14 +422,11 @@ IRBuilderAsmJs::BuildFieldSym(Js::RegSlot reg, Js::PropertyId propertyId, Proper
 uint
 IRBuilderAsmJs::AddStatementBoundary(uint statementIndex, uint offset)
 {
-    if (m_func->GetJITFunctionBody()->IsWasmFunction())
-    {
-        return 0;
-    }
+    AssertOrFailFast(m_statementReader);
     IR::PragmaInstr* pragmaInstr = IR::PragmaInstr::New(Js::OpCode::StatementBoundary, statementIndex, m_func);
     this->AddInstr(pragmaInstr, offset);
 
-    return m_statementReader.MoveNextStatementBoundary();
+    return m_statementReader->MoveNextStatementBoundary();
 }
 
 uint32 IRBuilderAsmJs::GetTypedRegFromRegSlot(Js::RegSlot reg, WAsmJs::Types type)
diff --git a/lib/Backend/IRBuilderAsmJs.h b/lib/Backend/IRBuilderAsmJs.h
@@ -81,7 +81,11 @@ class IRBuilderAsmJs
         , m_switchAdapter(this)
         , m_switchBuilder(&m_switchAdapter)
     {
-        func->m_workItem->InitializeReader(&m_jnReader, &m_statementReader, func->m_alloc);
+        if (!m_func->GetJITFunctionBody()->IsWasmFunction())
+        {
+            m_statementReader = Anew(func->m_alloc, Js::StatementReader<Js::FunctionBody::ArenaStatementMapList>);
+        }
+        func->m_workItem->InitializeReader(&m_jnReader, m_statementReader, func->m_alloc);
         m_asmFuncInfo = m_func->GetJITFunctionBody()->GetAsmJsInfo();
 #if 0
         // templatized JIT loop body
@@ -217,7 +221,7 @@ class IRBuilderAsmJs
     IR::Instr *             m_lastInstr;
     IR::Instr **            m_offsetToInstruction;
     Js::ByteCodeReader      m_jnReader;
-    Js::StatementReader<Js::FunctionBody::ArenaStatementMapList> m_statementReader;
+    Js::StatementReader<Js::FunctionBody::ArenaStatementMapList>* m_statementReader = nullptr;
     SListCounted<IR::Instr *> *m_argStack;
     SList<IR::Instr *> *    m_tempList;
     SList<int32> *          m_argOffsetStack;
diff --git a/lib/Backend/JITTimeWorkItem.cpp b/lib/Backend/JITTimeWorkItem.cpp
@@ -114,7 +114,10 @@ JITTimeWorkItem::InitializeReader(
 #endif
     bool hasSpanSequenceMap = m_jitBody.InitializeStatementMap(&m_statementMap, alloc);
     Js::SmallSpanSequence * spanSeq = hasSpanSequenceMap ? &m_statementMap : nullptr;
-    statementReader->Create(m_jitBody.GetByteCodeBuffer(), startOffset, spanSeq, m_fullStatementList);
+    if (statementReader)
+    {
+        statementReader->Create(m_jitBody.GetByteCodeBuffer(), startOffset, spanSeq, m_fullStatementList);
+    }
 }
 
 JITTimeFunctionBody *
diff --git a/lib/Runtime/ByteCode/StatementReader.h b/lib/Runtime/ByteCode/StatementReader.h
@@ -10,14 +10,14 @@ namespace Js
     class StatementReader
     {
     private:
-        const byte* m_startLocation;
-        SmallSpanSequence* m_statementMap;
+        const byte* m_startLocation = nullptr;
+        SmallSpanSequence* m_statementMap = nullptr;
         SmallSpanSequenceIter m_statementMapIter;
 
-        TStatementMapList * m_fullstatementMap;
-        const byte* m_nextStatementBoundary;
-        int m_statementIndex;
-        bool m_startOfStatement;
+        TStatementMapList * m_fullstatementMap = nullptr;
+        const byte* m_nextStatementBoundary = nullptr;
+        int m_statementIndex = 0;
+        bool m_startOfStatement = true;
 
     public:
         void Create(FunctionBody* functionRead, uint startOffset = 0);

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ IRBuilderAsmJs::Build()`
`16`	`16`	`m_tempAlloc = &localAlloc;`
`17`	`17`
`18`	`18`	`uint32 offset;`
`19`		`- uint32 statementIndex = m_statementReader.GetStatementIndex();`
	`19`	`+ uint32 statementIndex = m_statementReader ? m_statementReader->GetStatementIndex() : Js::Constants::NoStatementIndex;`
`20`	`20`
`21`	`21`	`m_argStack = JitAnew(m_tempAlloc, SListCounted<IR::Instr *>, m_tempAlloc);`
`22`	`22`	`m_tempList = JitAnew(m_tempAlloc, SList<IR::Instr *>, m_tempAlloc);`
`@@ -124,7 +124,7 @@ IRBuilderAsmJs::Build()`
`124`	`124`	`BuildImplicitArgIns();`
`125`	`125`	`}`
`126`	`126`
`127`		`- if (m_statementReader.AtStatementBoundary(&m_jnReader))`
	`127`	`+ if (m_statementReader && m_statementReader->AtStatementBoundary(&m_jnReader))`
`128`	`128`	`{`
`129`	`129`	`statementIndex = AddStatementBoundary(statementIndex, offset);`
`130`	`130`	`}`
`@@ -164,14 +164,14 @@ IRBuilderAsmJs::Build()`
`164`	`164`	`}`
`165`	`165`	`offset = m_jnReader.GetCurrentOffset();`
`166`	`166`
`167`		`- if (m_statementReader.AtStatementBoundary(&m_jnReader))`
	`167`	`+ if (m_statementReader && m_statementReader->AtStatementBoundary(&m_jnReader))`
`168`	`168`	`{`
`169`	`169`	`statementIndex = AddStatementBoundary(statementIndex, offset);`
`170`	`170`	`}`
`171`	`171`
`172`	`172`	`}`
`173`	`173`
`174`		`- if (Js::Constants::NoStatementIndex != statementIndex)`
	`174`	`+ if (m_statementReader && Js::Constants::NoStatementIndex != statementIndex)`
`175`	`175`	`{`
`176`	`176`	`statementIndex = AddStatementBoundary(statementIndex, Js::Constants::NoByteCodeOffset);`
`177`	`177`	`}`
`@@ -422,14 +422,11 @@ IRBuilderAsmJs::BuildFieldSym(Js::RegSlot reg, Js::PropertyId propertyId, Proper`
`422`	`422`	`uint`
`423`	`423`	`IRBuilderAsmJs::AddStatementBoundary(uint statementIndex, uint offset)`
`424`	`424`	`{`
`425`		`- if (m_func->GetJITFunctionBody()->IsWasmFunction())`
`426`		`- {`
`427`		`- return 0;`
`428`		`- }`
	`425`	`+ AssertOrFailFast(m_statementReader);`
`429`	`426`	`IR::PragmaInstr* pragmaInstr = IR::PragmaInstr::New(Js::OpCode::StatementBoundary, statementIndex, m_func);`
`430`	`427`	`this->AddInstr(pragmaInstr, offset);`
`431`	`428`
`432`		`- return m_statementReader.MoveNextStatementBoundary();`
	`429`	`+ return m_statementReader->MoveNextStatementBoundary();`
`433`	`430`	`}`
`434`	`431`
`435`	`432`	`uint32 IRBuilderAsmJs::GetTypedRegFromRegSlot(Js::RegSlot reg, WAsmJs::Types type)`