Skip to content

Commit 545b35e

Browse files
author
Irina Yatsenko
committed
[MERGE #4804 @irinayat-MS] OS#16244108: Small refactor of Parser::CreateCallNode 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.
2 parents d65ca8e + bb68776 commit 545b35e

File tree

4 files changed

+115
-131
lines changed

4 files changed

+115
-131
lines changed

‎lib/Parser/Parse.cpp

Lines changed: 41 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -693,50 +693,14 @@ static const int g_mpnopcbNode[] =
693693
#include "ptlist.h"
694694
};
695695

696-
const Js::RegSlot NoRegister = (Js::RegSlot)-1;
697-
const Js::RegSlot OneByteRegister = (Js::RegSlot_OneByte)-1;
698-
699-
void Parser::InitNode(OpCode nop,ParseNodePtr pnode) {
700-
pnode->nop = nop;
701-
pnode->grfpn = PNodeFlags::fpnNone;
702-
pnode->location = NoRegister;
703-
pnode->emitLabels = false;
704-
pnode->isUsed = true;
705-
pnode->notEscapedUse = false;
706-
pnode->isInList = false;
707-
pnode->isCallApplyTargetLoad = false;
708-
pnode->isSpecialName = false;
709-
}
710-
711696
// Create nodes using Arena
712697
ParseNodePtr
713698
Parser::StaticCreateBlockNode(ArenaAllocator* alloc, charcount_t ichMin , charcount_t ichLim, int blockId, PnodeBlockType blockType)
714699
{
715-
ParseNodePtr pnode = StaticCreateNodeT<knopBlock>(alloc, ichMin, ichLim);
716-
InitBlockNode(pnode, blockId, blockType);
717-
return pnode;
718-
}
700+
ParseNodeBlock* pnode = reinterpret_cast<ParseNodeBlock*>(StaticAllocNode<knopBlock>(alloc));
701+
pnode->Init(blockId, blockType, ichMin, ichLim);
719702

720-
void Parser::InitBlockNode(ParseNodePtr pnode, int blockId, PnodeBlockType blockType)
721-
{
722-
Assert(pnode->nop == knopBlock);
723-
pnode->AsParseNodeBlock()->pnodeScopes = nullptr;
724-
pnode->AsParseNodeBlock()->pnodeNext = nullptr;
725-
pnode->AsParseNodeBlock()->scope = nullptr;
726-
pnode->AsParseNodeBlock()->enclosingBlock = nullptr;
727-
pnode->AsParseNodeBlock()->pnodeLexVars = nullptr;
728-
pnode->AsParseNodeBlock()->pnodeStmt = nullptr;
729-
pnode->AsParseNodeBlock()->pnodeLastValStmt = nullptr;
730-
731-
pnode->AsParseNodeBlock()->callsEval = false;
732-
pnode->AsParseNodeBlock()->childCallsEval = false;
733-
pnode->AsParseNodeBlock()->blockType = blockType;
734-
pnode->AsParseNodeBlock()->blockId = blockId;
735-
736-
if (blockType != PnodeBlockType::Regular)
737-
{
738-
pnode->grfpn |= PNodeFlags::fpnSyntheticNode;
739-
}
703+
return pnode;
740704
}
741705

742706
// Create Node with limit
@@ -1060,7 +1024,7 @@ ParseNodePtr Parser::StaticCreateBinNode(OpCode nop,
10601024
{
10611025
DebugOnly(VerifyNodeSize(nopForSize, allocSize));
10621026
ParseNodePtr pnode = (ParseNodePtr)alloc->Alloc(allocSize);
1063-
InitNode(nop, pnode);
1027+
pnode->Init(nop, 0 /*ichMin*/, 0 /*ichLim*/);
10641028

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

10851049
// Create nodes using parser allocator
1050+
ParseNodePtr Parser::CreateBlockNode(PnodeBlockType blockType)
1051+
{
1052+
ParseNodePtr pnode = CreateNode(knopBlock);
1053+
pnode->AsParseNodeBlock()->Init(m_nextBlockId++, blockType, pnode->ichMin, pnode->ichLim);
1054+
return pnode;
1055+
}
10861056

10871057
ParseNodePtr Parser::CreateNode(OpCode nop, charcount_t ichMin)
10881058
{
@@ -1102,14 +1072,7 @@ ParseNodePtr Parser::CreateNode(OpCode nop, charcount_t ichMin)
11021072
*m_pCurrentAstSize += cb;
11031073
}
11041074

1105-
InitNode(nop,pnode);
1106-
1107-
// default - may be changed
1108-
pnode->ichMin = ichMin;
1109-
if (m_pscan!= nullptr) {
1110-
pnode->ichLim = m_pscan->IchLimTok();
1111-
}
1112-
else pnode->ichLim=0;
1075+
pnode->Init(nop, ichMin, (m_pscan != nullptr) ? m_pscan->IchLimTok() : 0 /*ichLim*/);
11131076

11141077
return pnode;
11151078
}
@@ -1123,7 +1086,7 @@ ParseNodePtr Parser::CreateUniNode(OpCode nop, ParseNodePtr pnode1)
11231086
Assert(m_pCurrentAstSize != nullptr);
11241087
*m_pCurrentAstSize += kcbPnUni;
11251088

1126-
InitNode(nop, pnode);
1089+
pnode->Init(nop, 0 /*ichMin*/, 0 /*ichLim*/);
11271090

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

1246-
ParseNodePtr
1247-
Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2,charcount_t ichMin,charcount_t ichLim)
1248-
{
1249-
Assert(!this->m_deferringAST);
1250-
DebugOnly(VerifyNodeSize(nop, kcbPnCall));
1251-
ParseNodePtr pnode = (ParseNodePtr)m_nodeAllocator.Alloc(kcbPnCall);
1252-
1253-
Assert(m_pCurrentAstSize != nullptr);
1254-
*m_pCurrentAstSize += kcbPnCall;
1255-
1256-
InitNode(nop, pnode);
1257-
1258-
pnode->AsParseNodeCall()->pnodeTarget = pnode1;
1259-
pnode->AsParseNodeCall()->pnodeArgs = pnode2;
1260-
pnode->AsParseNodeCall()->argCount = 0;
1261-
pnode->AsParseNodeCall()->spreadArgCount = 0;
1262-
pnode->AsParseNodeCall()->callOfConstants = false;
1263-
pnode->AsParseNodeCall()->isApplyCall = false;
1264-
pnode->AsParseNodeCall()->isEvalCall = false;
1265-
pnode->AsParseNodeCall()->isSuperCall = false;
1266-
pnode->AsParseNodeCall()->hasDestructuring = false;
1267-
pnode->ichMin = ichMin;
1268-
pnode->ichLim = ichLim;
1269-
1270-
return pnode;
1271-
}
1272-
12731209
ParseNodePtr Parser::CreateStrNode(IdentPtr pid)
12741210
{
12751211
Assert(!this->m_deferringAST);
@@ -1338,16 +1274,9 @@ ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePt
13381274
}
13391275
else
13401276
{
1341-
if (nullptr == pnode2)
1342-
{
1343-
ichMin = pnode1->ichMin;
1344-
ichLim = pnode1->ichLim;
1345-
}
1346-
else
1347-
{
1348-
ichMin = pnode1->ichMin;
1349-
ichLim = pnode2->ichLim;
1350-
}
1277+
ichMin = pnode1->ichMin;
1278+
ichLim = pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim;
1279+
13511280
if (pnode1->nop == knopDot || pnode1->nop == knopIndex)
13521281
{
13531282
this->CheckArguments(pnode1->AsParseNodeBin()->pnode1);
@@ -1356,33 +1285,39 @@ ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePt
13561285
return CreateCallNode(nop, pnode1, pnode2, ichMin, ichLim);
13571286
}
13581287

1288+
ParseNodePtr Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim)
1289+
{
1290+
Assert(!this->m_deferringAST);
1291+
1292+
// Classes, derived from ParseNodeCall, can be created here as well,
1293+
// as long as their size matches kcbPnCall (that is, they don't add
1294+
// any data members of their own).
1295+
DebugOnly(VerifyNodeSize(nop, kcbPnCall));
1296+
CompileAssert(kcbPnCall == sizeof(ParseNodeCall));
1297+
1298+
ParseNodeCall* pnode = reinterpret_cast<ParseNodeCall*>(m_nodeAllocator.Alloc(kcbPnCall));
1299+
pnode->Init(nop, pnode1, pnode2, ichMin, ichLim);
1300+
1301+
Assert(m_pCurrentAstSize != nullptr);
1302+
*m_pCurrentAstSize += kcbPnCall;
1303+
1304+
return pnode;
1305+
}
1306+
13591307
ParseNodePtr Parser::CreateSuperCallNode(ParseNodePtr pnode1, ParseNodePtr pnode2)
13601308
{
13611309
Assert(!this->m_deferringAST);
13621310
Assert(pnode1 && pnode1->isSpecialName && pnode1->AsParseNodeSpecialName()->isSuper);
13631311

13641312
DebugOnly(VerifyNodeSize(knopSuperCall, kcbPnSuperCall));
1365-
ParseNodePtr pnode = (ParseNodePtr)m_nodeAllocator.Alloc(kcbPnSuperCall);
1313+
CompileAssert(kcbPnSuperCall == sizeof(ParseNodeSuperCall));
1314+
1315+
ParseNodeSuperCall* pnode = reinterpret_cast<ParseNodeSuperCall*>(m_nodeAllocator.Alloc(kcbPnSuperCall));
1316+
pnode->Init(knopCall, pnode1, pnode2, pnode1->ichMin, pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim);
13661317

13671318
Assert(m_pCurrentAstSize != nullptr);
13681319
*m_pCurrentAstSize += kcbPnSuperCall;
13691320

1370-
InitNode(knopCall, pnode);
1371-
1372-
pnode->AsParseNodeCall()->pnodeTarget = pnode1;
1373-
pnode->AsParseNodeCall()->pnodeArgs = pnode2;
1374-
pnode->AsParseNodeCall()->argCount = 0;
1375-
pnode->AsParseNodeCall()->spreadArgCount = 0;
1376-
pnode->AsParseNodeCall()->callOfConstants = false;
1377-
pnode->AsParseNodeCall()->isApplyCall = false;
1378-
pnode->AsParseNodeCall()->isEvalCall = false;
1379-
pnode->AsParseNodeCall()->isSuperCall = true;
1380-
pnode->AsParseNodeSuperCall()->pnodeThis = nullptr;
1381-
pnode->AsParseNodeSuperCall()->pnodeNewTarget = nullptr;
1382-
1383-
pnode->ichMin = pnode1->ichMin;
1384-
pnode->ichLim = pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim;
1385-
13861321
return pnode;
13871322
}
13881323

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

12215-
InitNode(nop,pnode);
12216-
12217-
pnode->ichMin = ichMin;
12218-
pnode->ichLim = ichLim;
12150+
pnode->Init(nop, ichMin, ichLim);
1221912151

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

12242-
InitNode(nop, pnode);
12243-
12174+
pnode->Init(nop, ichMin, ichLim);
1224412175
pnode->AsParseNodeUni()->pnode1 = pnode1;
1224512176

12246-
pnode->ichMin = ichMin;
12247-
pnode->ichLim = ichLim;
12248-
1224912177
return pnode;
1225012178
}
1225112179

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

12278-
InitNode(nop, pnode);
12206+
pnode->Init(nop, ichMin, ichLim);
1227912207

1228012208
pnode->AsParseNodeTri()->pnodeNext = NULL;
1228112209
pnode->AsParseNodeTri()->pnode1 = pnode1;
1228212210
pnode->AsParseNodeTri()->pnode2 = pnode2;
1228312211
pnode->AsParseNodeTri()->pnode3 = pnode3;
1228412212

12285-
pnode->ichMin = ichMin;
12286-
pnode->ichLim = ichLim;
12287-
1228812213
return pnode;
1228912214
}
1229012215

‎lib/Parser/Parse.h

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,7 @@ class Parser
251251
static ParseNodePtr StaticCreateNodeT(ArenaAllocator* alloc, charcount_t ichMin = 0, charcount_t ichLim = 0)
252252
{
253253
ParseNodePtr pnode = StaticAllocNode<nop>(alloc);
254-
InitNode(nop,pnode);
255-
// default - may be changed
256-
pnode->ichMin = ichMin;
257-
pnode->ichLim = ichLim;
254+
pnode->Init(nop, ichMin, ichLim);
258255

259256
return pnode;
260257
}
@@ -299,20 +296,15 @@ class Parser
299296
pnode->AsParseNodeSpecialName()->isSuper = false;
300297
return pnode;
301298
}
302-
ParseNodePtr CreateBlockNode(PnodeBlockType blockType = PnodeBlockType::Regular)
303-
{
304-
ParseNodePtr pnode = CreateNode(knopBlock);
305-
InitBlockNode(pnode, m_nextBlockId++, blockType);
306-
return pnode;
307-
}
308-
// Creating parse nodes.
309299

300+
// Creating parse nodes.
310301
ParseNodePtr CreateNode(OpCode nop, charcount_t ichMin);
311302
ParseNodePtr CreateTriNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ParseNodePtr pnode3);
312303
ParseNodePtr CreateIntNode(int32 lw);
313304
ParseNodePtr CreateStrNode(IdentPtr pid);
314305

315306
ParseNodePtr CreateUniNode(OpCode nop, ParseNodePtr pnodeOp);
307+
ParseNodePtr CreateBlockNode(PnodeBlockType blockType = PnodeBlockType::Regular);
316308
ParseNodePtr CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
317309
ParseNodePtr CreateSuperReferenceNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
318310
ParseNodePtr CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2);
@@ -380,9 +372,6 @@ class Parser
380372
ParseNodePtr CreateIntNodeWithScanner(int32 lw);
381373
ParseNodePtr CreateProgNodeWithScanner(bool isModuleSource);
382374

383-
static void InitNode(OpCode nop,ParseNodePtr pnode);
384-
static void InitBlockNode(ParseNodePtr pnode, int blockId, PnodeBlockType blockType);
385-
386375
private:
387376
ParseNodePtr m_currentNodeNonLambdaFunc; // current function or NULL
388377
ParseNodePtr m_currentNodeNonLambdaDeferredFunc; // current function or NULL

‎lib/Parser/ptree.cpp

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,21 @@
44
//-------------------------------------------------------------------------------------------------------
55
#include "ParserPch.h"
66

7+
void ParseNode::Init(OpCode nop, charcount_t ichMin, charcount_t ichLim)
8+
{
9+
this->nop = nop;
10+
this->grfpn = PNodeFlags::fpnNone;
11+
this->location = Js::Constants::NoRegister;
12+
this->emitLabels = false;
13+
this->isUsed = true;
14+
this->notEscapedUse = false;
15+
this->isInList = false;
16+
this->isCallApplyTargetLoad = false;
17+
this->isSpecialName = false;
18+
this->ichMin = ichMin;
19+
this->ichLim = ichLim;
20+
}
21+
722
ParseNodeUni * ParseNode::AsParseNodeUni()
823
{
924
Assert(((this->Grfnop() & fnopUni) && this->nop != knopParamPattern) || this->nop == knopThrow);
@@ -251,4 +266,51 @@ ParseNodePtr ParseNode::GetFormalNext()
251266
pnodeNext = this->AsParseNodeVar()->pnodeNext;
252267
}
253268
return pnodeNext;
254-
}
269+
}
270+
271+
void ParseNodeCall::Init(OpCode nop, ParseNodePtr pnodeTarget, ParseNodePtr pnodeArgs, charcount_t ichMin, charcount_t ichLim)
272+
{
273+
__super::Init(nop, ichMin, ichLim);
274+
275+
this->pnodeTarget = pnodeTarget;
276+
this->pnodeArgs = pnodeArgs;
277+
this->argCount = 0;
278+
this->spreadArgCount = 0;
279+
this->callOfConstants = false;
280+
this->isApplyCall = false;
281+
this->isEvalCall = false;
282+
this->isSuperCall = false;
283+
this->hasDestructuring = false;
284+
}
285+
286+
void ParseNodeSuperCall::Init(OpCode nop, ParseNodePtr pnodeTarget, ParseNodePtr pnodeArgs, charcount_t ichMin, charcount_t ichLim)
287+
{
288+
__super::Init(nop, pnodeTarget, pnodeArgs, ichMin, ichLim);
289+
290+
this->isSuperCall = true;
291+
this->pnodeThis = nullptr;
292+
this->pnodeNewTarget = nullptr;
293+
}
294+
295+
void ParseNodeBlock::Init(int blockId, PnodeBlockType blockType, charcount_t ichMin, charcount_t ichLim)
296+
{
297+
__super::Init(knopBlock, ichMin, ichLim);
298+
299+
this->pnodeScopes = nullptr;
300+
this->pnodeNext = nullptr;
301+
this->scope = nullptr;
302+
this->enclosingBlock = nullptr;
303+
this->pnodeLexVars = nullptr;
304+
this->pnodeStmt = nullptr;
305+
this->pnodeLastValStmt = nullptr;
306+
307+
this->callsEval = false;
308+
this->childCallsEval = false;
309+
this->blockType = blockType;
310+
this->blockId = blockId;
311+
312+
if (blockType != PnodeBlockType::Regular)
313+
{
314+
this->grfpn |= PNodeFlags::fpnSyntheticNode;
315+
}
316+
}

0 commit comments

Comments
 (0)