Skip to content

Commit

Permalink
[MERGE #4804 @irinayat-MS] OS#16244108: Small refactor of Parser::Cre…
Browse files Browse the repository at this point in the history 
…ateCallNode and Parser::CreateSuperCallNode

Merge pull request #4804 from irinayat-MS:ParseNodeCall

https://microsoft.visualstudio.com/OS/_workitems/edit/16244108

The uninitialized field was introduced by #3917. The only read of the field is in EmitArgList, and if it ends up "true" instead of the default "false" an extra defensive load will be emitted for the constructor parameters so not a security/correctness concern.
  • Loading branch information
Irina Yatsenko committed Mar 12, 2018
2 parents d65ca8e + bb68776 commit 545b35e
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 131 deletions.
157 changes: 41 additions & 116 deletions lib/Parser/Parse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,50 +693,14 @@ static const int g_mpnopcbNode[] =
#include "ptlist.h"
};

const Js::RegSlot NoRegister = (Js::RegSlot)-1;
const Js::RegSlot OneByteRegister = (Js::RegSlot_OneByte)-1;

void Parser::InitNode(OpCode nop,ParseNodePtr pnode) {
pnode->nop = nop;
pnode->grfpn = PNodeFlags::fpnNone;
pnode->location = NoRegister;
pnode->emitLabels = false;
pnode->isUsed = true;
pnode->notEscapedUse = false;
pnode->isInList = false;
pnode->isCallApplyTargetLoad = false;
pnode->isSpecialName = false;
}

// Create nodes using Arena
ParseNodePtr
Parser::StaticCreateBlockNode(ArenaAllocator* alloc, charcount_t ichMin , charcount_t ichLim, int blockId, PnodeBlockType blockType)
{
ParseNodePtr pnode = StaticCreateNodeT<knopBlock>(alloc, ichMin, ichLim);
InitBlockNode(pnode, blockId, blockType);
return pnode;
}
ParseNodeBlock* pnode = reinterpret_cast<ParseNodeBlock*>(StaticAllocNode<knopBlock>(alloc));
pnode->Init(blockId, blockType, ichMin, ichLim);

void Parser::InitBlockNode(ParseNodePtr pnode, int blockId, PnodeBlockType blockType)
{
Assert(pnode->nop == knopBlock);
pnode->AsParseNodeBlock()->pnodeScopes = nullptr;
pnode->AsParseNodeBlock()->pnodeNext = nullptr;
pnode->AsParseNodeBlock()->scope = nullptr;
pnode->AsParseNodeBlock()->enclosingBlock = nullptr;
pnode->AsParseNodeBlock()->pnodeLexVars = nullptr;
pnode->AsParseNodeBlock()->pnodeStmt = nullptr;
pnode->AsParseNodeBlock()->pnodeLastValStmt = nullptr;

pnode->AsParseNodeBlock()->callsEval = false;
pnode->AsParseNodeBlock()->childCallsEval = false;
pnode->AsParseNodeBlock()->blockType = blockType;
pnode->AsParseNodeBlock()->blockId = blockId;

if (blockType != PnodeBlockType::Regular)
{
pnode->grfpn |= PNodeFlags::fpnSyntheticNode;
}
return pnode;
}

// Create Node with limit
Expand Down Expand Up @@ -1060,7 +1024,7 @@ ParseNodePtr Parser::StaticCreateBinNode(OpCode nop,
{
DebugOnly(VerifyNodeSize(nopForSize, allocSize));
ParseNodePtr pnode = (ParseNodePtr)alloc->Alloc(allocSize);
InitNode(nop, pnode);
pnode->Init(nop, 0 /*ichMin*/, 0 /*ichLim*/);

pnode->AsParseNodeBin()->pnodeNext = nullptr;
pnode->AsParseNodeBin()->pnode1 = pnode1;
Expand All @@ -1083,6 +1047,12 @@ ParseNodePtr Parser::StaticCreateBinNode(OpCode nop,
}

// Create nodes using parser allocator
ParseNodePtr Parser::CreateBlockNode(PnodeBlockType blockType)
{
ParseNodePtr pnode = CreateNode(knopBlock);
pnode->AsParseNodeBlock()->Init(m_nextBlockId++, blockType, pnode->ichMin, pnode->ichLim);
return pnode;
}

ParseNodePtr Parser::CreateNode(OpCode nop, charcount_t ichMin)
{
Expand All @@ -1102,14 +1072,7 @@ ParseNodePtr Parser::CreateNode(OpCode nop, charcount_t ichMin)
*m_pCurrentAstSize += cb;
}

InitNode(nop,pnode);

// default - may be changed
pnode->ichMin = ichMin;
if (m_pscan!= nullptr) {
pnode->ichLim = m_pscan->IchLimTok();
}
else pnode->ichLim=0;
pnode->Init(nop, ichMin, (m_pscan != nullptr) ? m_pscan->IchLimTok() : 0 /*ichLim*/);

return pnode;
}
Expand All @@ -1123,7 +1086,7 @@ ParseNodePtr Parser::CreateUniNode(OpCode nop, ParseNodePtr pnode1)
Assert(m_pCurrentAstSize != nullptr);
*m_pCurrentAstSize += kcbPnUni;

InitNode(nop, pnode);
pnode->Init(nop, 0 /*ichMin*/, 0 /*ichLim*/);

pnode->AsParseNodeUni()->pnode1 = pnode1;
if (nullptr == pnode1)
Expand Down Expand Up @@ -1243,33 +1206,6 @@ ParseNodePtr Parser::CreateBlockNode(charcount_t ichMin,charcount_t ichLim, Pnod
return StaticCreateBlockNode(&m_nodeAllocator, ichMin, ichLim, this->m_nextBlockId++, blockType);
}

ParseNodePtr
Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2,charcount_t ichMin,charcount_t ichLim)
{
Assert(!this->m_deferringAST);
DebugOnly(VerifyNodeSize(nop, kcbPnCall));
ParseNodePtr pnode = (ParseNodePtr)m_nodeAllocator.Alloc(kcbPnCall);

Assert(m_pCurrentAstSize != nullptr);
*m_pCurrentAstSize += kcbPnCall;

InitNode(nop, pnode);

pnode->AsParseNodeCall()->pnodeTarget = pnode1;
pnode->AsParseNodeCall()->pnodeArgs = pnode2;
pnode->AsParseNodeCall()->argCount = 0;
pnode->AsParseNodeCall()->spreadArgCount = 0;
pnode->AsParseNodeCall()->callOfConstants = false;
pnode->AsParseNodeCall()->isApplyCall = false;
pnode->AsParseNodeCall()->isEvalCall = false;
pnode->AsParseNodeCall()->isSuperCall = false;
pnode->AsParseNodeCall()->hasDestructuring = false;
pnode->ichMin = ichMin;
pnode->ichLim = ichLim;

return pnode;
}

ParseNodePtr Parser::CreateStrNode(IdentPtr pid)
{
Assert(!this->m_deferringAST);
Expand Down Expand Up @@ -1338,16 +1274,9 @@ ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePt
}
else
{
if (nullptr == pnode2)
{
ichMin = pnode1->ichMin;
ichLim = pnode1->ichLim;
}
else
{
ichMin = pnode1->ichMin;
ichLim = pnode2->ichLim;
}
ichMin = pnode1->ichMin;
ichLim = pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim;

if (pnode1->nop == knopDot || pnode1->nop == knopIndex)
{
this->CheckArguments(pnode1->AsParseNodeBin()->pnode1);
Expand All @@ -1356,33 +1285,39 @@ ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePt
return CreateCallNode(nop, pnode1, pnode2, ichMin, ichLim);
}

ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim)
{
Assert(!this->m_deferringAST);

// Classes, derived from ParseNodeCall, can be created here as well,
// as long as their size matches kcbPnCall (that is, they don't add
// any data members of their own).
DebugOnly(VerifyNodeSize(nop, kcbPnCall));
CompileAssert(kcbPnCall == sizeof(ParseNodeCall));

ParseNodeCall* pnode = reinterpret_cast<ParseNodeCall*>(m_nodeAllocator.Alloc(kcbPnCall));
pnode->Init(nop, pnode1, pnode2, ichMin, ichLim);

Assert(m_pCurrentAstSize != nullptr);
*m_pCurrentAstSize += kcbPnCall;

return pnode;
}

ParseNodePtr Parser::CreateSuperCallNode(ParseNodePtr pnode1, ParseNodePtr pnode2)
{
Assert(!this->m_deferringAST);
Assert(pnode1 && pnode1->isSpecialName && pnode1->AsParseNodeSpecialName()->isSuper);

DebugOnly(VerifyNodeSize(knopSuperCall, kcbPnSuperCall));
ParseNodePtr pnode = (ParseNodePtr)m_nodeAllocator.Alloc(kcbPnSuperCall);
CompileAssert(kcbPnSuperCall == sizeof(ParseNodeSuperCall));

ParseNodeSuperCall* pnode = reinterpret_cast<ParseNodeSuperCall*>(m_nodeAllocator.Alloc(kcbPnSuperCall));
pnode->Init(knopCall, pnode1, pnode2, pnode1->ichMin, pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim);

Assert(m_pCurrentAstSize != nullptr);
*m_pCurrentAstSize += kcbPnSuperCall;

InitNode(knopCall, pnode);

pnode->AsParseNodeCall()->pnodeTarget = pnode1;
pnode->AsParseNodeCall()->pnodeArgs = pnode2;
pnode->AsParseNodeCall()->argCount = 0;
pnode->AsParseNodeCall()->spreadArgCount = 0;
pnode->AsParseNodeCall()->callOfConstants = false;
pnode->AsParseNodeCall()->isApplyCall = false;
pnode->AsParseNodeCall()->isEvalCall = false;
pnode->AsParseNodeCall()->isSuperCall = true;
pnode->AsParseNodeSuperCall()->pnodeThis = nullptr;
pnode->AsParseNodeSuperCall()->pnodeNewTarget = nullptr;

pnode->ichMin = pnode1->ichMin;
pnode->ichLim = pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim;

return pnode;
}

Expand Down Expand Up @@ -12212,10 +12147,7 @@ ParseNodePtr Parser::CreateNode(OpCode nop, charcount_t ichMin, charcount_t ichL
Assert(m_pCurrentAstSize != NULL);
*m_pCurrentAstSize += cb;

InitNode(nop,pnode);

pnode->ichMin = ichMin;
pnode->ichLim = ichLim;
pnode->Init(nop, ichMin, ichLim);

return pnode;
}
Expand All @@ -12239,13 +12171,9 @@ ParseNodePtr Parser::CreateUniNode(OpCode nop, ParseNodePtr pnode1, charcount_t
Assert(m_pCurrentAstSize != NULL);
*m_pCurrentAstSize += kcbPnUni;

InitNode(nop, pnode);

pnode->Init(nop, ichMin, ichLim);
pnode->AsParseNodeUni()->pnode1 = pnode1;

pnode->ichMin = ichMin;
pnode->ichLim = ichLim;

return pnode;
}

Expand Down Expand Up @@ -12275,16 +12203,13 @@ ParseNodePtr Parser::CreateTriNode(OpCode nop, ParseNodePtr pnode1,
Assert(m_pCurrentAstSize != NULL);
*m_pCurrentAstSize += kcbPnTri;

InitNode(nop, pnode);
pnode->Init(nop, ichMin, ichLim);

pnode->AsParseNodeTri()->pnodeNext = NULL;
pnode->AsParseNodeTri()->pnode1 = pnode1;
pnode->AsParseNodeTri()->pnode2 = pnode2;
pnode->AsParseNodeTri()->pnode3 = pnode3;

pnode->ichMin = ichMin;
pnode->ichLim = ichLim;

return pnode;
}

Expand Down
17 changes: 3 additions & 14 deletions lib/Parser/Parse.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,7 @@ class Parser
static ParseNodePtr StaticCreateNodeT(ArenaAllocator* alloc, charcount_t ichMin = 0, charcount_t ichLim = 0)
{
ParseNodePtr pnode = StaticAllocNode<nop>(alloc);
InitNode(nop,pnode);
// default - may be changed
pnode->ichMin = ichMin;
pnode->ichLim = ichLim;
pnode->Init(nop, ichMin, ichLim);

return pnode;
}
Expand Down Expand Up @@ -299,20 +296,15 @@ class Parser
pnode->AsParseNodeSpecialName()->isSuper = false;
return pnode;
}
ParseNodePtr CreateBlockNode(PnodeBlockType blockType = PnodeBlockType::Regular)
{
ParseNodePtr pnode = CreateNode(knopBlock);
InitBlockNode(pnode, m_nextBlockId++, blockType);
return pnode;
}
// Creating parse nodes.

// Creating parse nodes.
ParseNodePtr CreateNode(OpCode nop, charcount_t ichMin);
ParseNodePtr CreateTriNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ParseNodePtr pnode3);
ParseNodePtr CreateIntNode(int32 lw);
ParseNodePtr CreateStrNode(IdentPtr pid);

ParseNodePtr CreateUniNode(OpCode nop, ParseNodePtr pnodeOp);
ParseNodePtr CreateBlockNode(PnodeBlockType blockType = PnodeBlockType::Regular);
ParseNodePtr CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
ParseNodePtr CreateSuperReferenceNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
ParseNodePtr CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
Expand Down Expand Up @@ -380,9 +372,6 @@ class Parser
ParseNodePtr CreateIntNodeWithScanner(int32 lw);
ParseNodePtr CreateProgNodeWithScanner(bool isModuleSource);

static void InitNode(OpCode nop,ParseNodePtr pnode);
static void InitBlockNode(ParseNodePtr pnode, int blockId, PnodeBlockType blockType);

private:
ParseNodePtr m_currentNodeNonLambdaFunc; // current function or NULL
ParseNodePtr m_currentNodeNonLambdaDeferredFunc; // current function or NULL
Expand Down
64 changes: 63 additions & 1 deletion lib/Parser/ptree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@
//-------------------------------------------------------------------------------------------------------
#include "ParserPch.h"

void ParseNode::Init(OpCode nop, charcount_t ichMin, charcount_t ichLim)
{
this->nop = nop;
this->grfpn = PNodeFlags::fpnNone;
this->location = Js::Constants::NoRegister;
this->emitLabels = false;
this->isUsed = true;
this->notEscapedUse = false;
this->isInList = false;
this->isCallApplyTargetLoad = false;
this->isSpecialName = false;
this->ichMin = ichMin;
this->ichLim = ichLim;
}

ParseNodeUni * ParseNode::AsParseNodeUni()
{
Assert(((this->Grfnop() & fnopUni) && this->nop != knopParamPattern) || this->nop == knopThrow);
Expand Down Expand Up @@ -251,4 +266,51 @@ ParseNodePtr ParseNode::GetFormalNext()
pnodeNext = this->AsParseNodeVar()->pnodeNext;
}
return pnodeNext;
}
}

void ParseNodeCall::Init(OpCode nop, ParseNodePtr pnodeTarget, ParseNodePtr pnodeArgs, charcount_t ichMin, charcount_t ichLim)
{
__super::Init(nop, ichMin, ichLim);

this->pnodeTarget = pnodeTarget;
this->pnodeArgs = pnodeArgs;
this->argCount = 0;
this->spreadArgCount = 0;
this->callOfConstants = false;
this->isApplyCall = false;
this->isEvalCall = false;
this->isSuperCall = false;
this->hasDestructuring = false;
}

void ParseNodeSuperCall::Init(OpCode nop, ParseNodePtr pnodeTarget, ParseNodePtr pnodeArgs, charcount_t ichMin, charcount_t ichLim)
{
__super::Init(nop, pnodeTarget, pnodeArgs, ichMin, ichLim);

this->isSuperCall = true;
this->pnodeThis = nullptr;
this->pnodeNewTarget = nullptr;
}

void ParseNodeBlock::Init(int blockId, PnodeBlockType blockType, charcount_t ichMin, charcount_t ichLim)
{
__super::Init(knopBlock, ichMin, ichLim);

this->pnodeScopes = nullptr;
this->pnodeNext = nullptr;
this->scope = nullptr;
this->enclosingBlock = nullptr;
this->pnodeLexVars = nullptr;
this->pnodeStmt = nullptr;
this->pnodeLastValStmt = nullptr;

this->callsEval = false;
this->childCallsEval = false;
this->blockType = blockType;
this->blockId = blockId;

if (blockType != PnodeBlockType::Regular)
{
this->grfpn |= PNodeFlags::fpnSyntheticNode;
}
}
Loading

0 comments on commit 545b35e

Please sign in to comment.