Skip to content

Commit 9782d41

Browse files
MikeHolmanMikeHolman
committed
Merged PR 149294: avoid prop record lookup for external function creation, put more spectre stuff behind flags
avoid prop record lookup for external function creation, put more spectre stuff behind flags
2 parents e1460af + 581fe07 commit 9782d41

10 files changed

+57
-59
lines changed

‎lib/Backend/BackwardPass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3631,6 +3631,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
36313631
&& !block->GetFirstInstr()->m_func->GetJITFunctionBody()->IsWasmFunction()
36323632
&& !block->isDead
36333633
&& !block->isDeleted
3634+
&& CONFIG_FLAG_RELEASE(AddMaskingBlocks)
36343635
)
36353636
{
36363637
FOREACH_PREDECESSOR_BLOCK(blockPred, block)

‎lib/Common/CommonDefines.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@
340340
#endif
341341

342342
#ifdef NTBUILD
343+
#define ENABLE_SPECTRE_RUNTIME_MITIGATIONS
343344
#define ENABLE_PROJECTION
344345
#define ENABLE_FOUNDATION_OBJECT
345346
#define ENABLE_EXPERIMENTAL_FLAGS

‎lib/Common/ConfigFlagsList.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,8 @@ PHASE(All)
522522

523523
#define DEFAULT_CONFIG_MitigateSpectre (false)
524524

525+
#define DEFAULT_CONFIG_AddMaskingBlocks (false)
526+
525527
#define DEFAULT_CONFIG_PoisonVarArrayLoad (false)
526528
#define DEFAULT_CONFIG_PoisonIntArrayLoad (false)
527529
#define DEFAULT_CONFIG_PoisonFloatArrayLoad (false)
@@ -1306,6 +1308,8 @@ FLAGNR(Boolean, ForceMaxJitThreadCount, "Force the number of parallel jit thread
13061308

13071309
FLAGR(Boolean, MitigateSpectre, "Use mitigations for Spectre", DEFAULT_CONFIG_MitigateSpectre)
13081310

1311+
FLAGPR(Boolean, MitigateSpectre, AddMaskingBlocks, "Optimize Spectre mitigations by masking on loop out edges", DEFAULT_CONFIG_AddMaskingBlocks)
1312+
13091313
FLAGPR(Boolean, MitigateSpectre, PoisonVarArrayLoad, "Poison loads from Var arrays", DEFAULT_CONFIG_PoisonVarArrayLoad)
13101314
FLAGPR(Boolean, MitigateSpectre, PoisonIntArrayLoad, "Poison loads from Int arrays", DEFAULT_CONFIG_PoisonIntArrayLoad)
13111315
FLAGPR(Boolean, MitigateSpectre, PoisonFloatArrayLoad, "Poison loads from Float arrays", DEFAULT_CONFIG_PoisonFloatArrayLoad)

‎lib/Runtime/Language/JavascriptOperators.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3881,7 +3881,9 @@ using namespace Js;
38813881

38823882
Var JavascriptOperators::OP_GetElementI(Var instance, Var index, ScriptContext* scriptContext)
38833883
{
3884+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
38843885
instance = BreakSpeculation(instance);
3886+
#endif
38853887
if (TaggedInt::Is(index))
38863888
{
38873889
return GetElementIIntIndex(instance, index, scriptContext);

‎lib/Runtime/Library/JavascriptExternalFunction.cpp

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -314,24 +314,6 @@ namespace Js
314314
END_LEAVE_SCRIPT(scriptContext);
315315
#endif
316316

317-
bool marshallingMayBeNeeded = false;
318-
if (result != nullptr)
319-
{
320-
marshallingMayBeNeeded = Js::VarIs<Js::RecyclableObject>(result);
321-
if (marshallingMayBeNeeded)
322-
{
323-
Js::RecyclableObject * obj = Js::VarTo<Js::RecyclableObject>(result);
324-
325-
// For JSRT, we could get result marshalled in different context.
326-
bool isJSRT = scriptContext->GetThreadContext()->IsJSRT();
327-
marshallingMayBeNeeded = obj->GetScriptContext() != scriptContext;
328-
if (!isJSRT && marshallingMayBeNeeded)
329-
{
330-
Js::Throw::InternalError();
331-
}
332-
}
333-
}
334-
335317
if (scriptContext->HasRecordedException())
336318
{
337319
bool considerPassingToDebugger = false;
@@ -354,7 +336,7 @@ namespace Js
354336
{
355337
result = scriptContext->GetLibrary()->GetUndefined();
356338
}
357-
else if (marshallingMayBeNeeded)
339+
else
358340
{
359341
result = CrossSite::MarshalVar(scriptContext, result);
360342
}

‎lib/Runtime/Library/JavascriptLibrary.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5911,20 +5911,8 @@ namespace Js
59115911

59125912
JavascriptExternalFunction* JavascriptLibrary::CreateStdCallExternalFunction(StdCallJavascriptMethod entryPoint, Var name, void *callbackState)
59135913
{
5914-
Var functionNameOrId = name;
5915-
if (VarIs<JavascriptString>(name))
5916-
{
5917-
JavascriptString * functionName = VarTo<JavascriptString>(name);
5918-
const char16 * functionNameBuffer = functionName->GetString();
5919-
int functionNameBufferLength = functionName->GetLengthAsSignedInt();
5920-
5921-
PropertyId functionNamePropertyId = scriptContext->GetOrAddPropertyIdTracked(functionNameBuffer, functionNameBufferLength);
5922-
functionNameOrId = TaggedInt::ToVarUnchecked(functionNamePropertyId);
5923-
}
5924-
5925-
AssertOrFailFast(TaggedInt::Is(functionNameOrId));
59265914
JavascriptExternalFunction* function = this->CreateIdMappedExternalFunction(entryPoint, stdCallFunctionWithDeferredPrototypeType);
5927-
function->SetFunctionNameId(functionNameOrId);
5915+
function->SetFunctionNameId(name);
59285916
function->SetCallbackState(callbackState);
59295917
return function;
59305918
}

‎lib/Runtime/Library/JavascriptObject.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ JavascriptArray* JavascriptObject::CreateKeysHelper(RecyclableObject* object, Sc
12091209
AssertMsg(includeStringProperties || includeSymbolProperties, "Should either get string or symbol properties.");
12101210

12111211
JavascriptStaticEnumerator enumerator;
1212-
EnumeratorFlags flags = EnumeratorFlags::UseCache;
1212+
EnumeratorFlags flags = EnumeratorFlags::UseCache;
12131213
JavascriptArray* newArr = scriptContext->GetLibrary()->CreateArray(0);
12141214
if (includeNonEnumerable)
12151215
{
@@ -1219,8 +1219,8 @@ JavascriptArray* JavascriptObject::CreateKeysHelper(RecyclableObject* object, Sc
12191219
{
12201220
flags |= EnumeratorFlags::EnumSymbols;
12211221
}
1222-
EnumeratorCache* cache = scriptContext->GetLibrary()->GetCreateKeysCache(object->GetType());
1223-
if (!object->GetEnumerator(&enumerator, flags, scriptContext, cache))
1222+
EnumeratorCache* cache = scriptContext->GetLibrary()->GetCreateKeysCache(object->GetType());
1223+
if (!object->GetEnumerator(&enumerator, flags, scriptContext, cache))
12241224
{
12251225
return newArr; // Return an empty array if we don't have an enumerator
12261226
}
@@ -1231,7 +1231,7 @@ JavascriptArray* JavascriptObject::CreateKeysHelper(RecyclableObject* object, Sc
12311231
uint32 symbolIndex = 0;
12321232
const PropertyRecord* propertyRecord;
12331233
JavascriptSymbol* symbol;
1234-
JavascriptArray* newArrForSymbols = nullptr;
1234+
JavascriptArray* newArrForSymbols = nullptr;
12351235
while ((propertyName = enumerator.MoveAndGetNext(propertyId)) != NULL)
12361236
{
12371237
if (propertyName)
@@ -1244,10 +1244,10 @@ JavascriptArray* JavascriptObject::CreateKeysHelper(RecyclableObject* object, Sc
12441244
{
12451245
symbol = scriptContext->GetSymbol(propertyRecord);
12461246
// no need to marshal symbol because it is created from scriptContext
1247-
if (!newArrForSymbols)
1248-
{
1249-
newArrForSymbols = scriptContext->GetLibrary()->CreateArray(0);
1250-
}
1247+
if (!newArrForSymbols)
1248+
{
1249+
newArrForSymbols = scriptContext->GetLibrary()->CreateArray(0);
1250+
}
12511251
newArrForSymbols->DirectSetItemAt(symbolIndex++, symbol);
12521252
continue;
12531253
}
@@ -1270,14 +1270,14 @@ JavascriptArray* JavascriptObject::CreateKeysHelper(RecyclableObject* object, Sc
12701270
}
12711271
}
12721272

1273-
if (newArrForSymbols)
1274-
{
1273+
if (newArrForSymbols)
1274+
{
12751275
// Append all the symbols at the end of list
12761276
uint32 totalSymbols = newArrForSymbols->GetLength();
12771277
for (uint32 symIndex = 0; symIndex < totalSymbols; symIndex++)
12781278
{
12791279
newArr->DirectSetItemAt(propertyIndex++, newArrForSymbols->DirectGetItem(symIndex));
1280-
}
1280+
}
12811281
}
12821282

12831283
return newArr;

‎lib/Runtime/Library/JavascriptString.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,9 @@ namespace Js
733733
Var value;
734734
if (pThis->GetItemAt(idxPosition, &value))
735735
{
736+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
736737
value = BreakSpeculation(value);
738+
#endif
737739
return value;
738740
}
739741
else
@@ -782,7 +784,11 @@ namespace Js
782784
return scriptContext->GetLibrary()->GetNaN();
783785
}
784786

785-
return BreakSpeculation(TaggedInt::ToVarUnchecked(pThis->GetItem(idxPosition)));
787+
Var charCode = TaggedInt::ToVarUnchecked(pThis->GetItem(idxPosition));
788+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
789+
charCode = BreakSpeculation(charCode);
790+
#endif
791+
return charCode;
786792
}
787793

788794
Var JavascriptString::EntryCodePointAt(RecyclableObject* function, CallInfo callInfo, ...)
@@ -1837,7 +1843,9 @@ namespace Js
18371843
idxEnd = idxStart;
18381844
}
18391845

1846+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
18401847
pThis = (JavascriptString*)BreakSpeculation(pThis);
1848+
#endif
18411849

18421850
return SubstringCore(pThis, idxStart, idxEnd - idxStart, scriptContext);
18431851
}
@@ -1958,7 +1966,9 @@ namespace Js
19581966
return pThis;
19591967
}
19601968

1969+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
19611970
pThis = (JavascriptString*)BreakSpeculation(pThis);
1971+
#endif
19621972

19631973
return SubstringCore(pThis, idxStart, idxEnd - idxStart, scriptContext);
19641974
}
@@ -2016,7 +2026,9 @@ namespace Js
20162026
return pThis;
20172027
}
20182028

2029+
#ifdef ENABLE_SPECTRE_RUNTIME_MITIGATIONS
20192030
pThis = (JavascriptString*)BreakSpeculation(pThis);
2031+
#endif
20202032

20212033
Assert(0 <= idxStart && idxStart <= idxEnd && idxEnd <= len);
20222034
return SubstringCore(pThis, idxStart, idxEnd - idxStart, scriptContext);

‎lib/Runtime/Library/RuntimeFunction.cpp

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
namespace Js
88
{
99
RuntimeFunction::RuntimeFunction(DynamicType * type)
10-
: JavascriptFunction(type), functionNameId(nullptr)
10+
: JavascriptFunction(type), isDisplayString(false), functionNameId(nullptr)
1111
{}
1212

1313
RuntimeFunction::RuntimeFunction(DynamicType * type, FunctionInfo * functionInfo)
14-
: JavascriptFunction(type, functionInfo), functionNameId(nullptr)
14+
: JavascriptFunction(type, functionInfo), isDisplayString(false), functionNameId(nullptr)
1515
{}
1616

1717
RuntimeFunction::RuntimeFunction(DynamicType * type, FunctionInfo * functionInfo, ConstructorCache* cache)
18-
: JavascriptFunction(type, functionInfo, cache), functionNameId(nullptr)
18+
: JavascriptFunction(type, functionInfo, cache), isDisplayString(false), functionNameId(nullptr)
1919
{}
2020

2121
JavascriptString *
@@ -24,33 +24,39 @@ namespace Js
2424
JavascriptLibrary* library = this->GetLibrary();
2525
ScriptContext * scriptContext = library->GetScriptContext();
2626
JavascriptString * retStr = nullptr;
27+
if (this->isDisplayString)
28+
{
29+
return VarTo<JavascriptString>(this->functionNameId);
30+
}
31+
2732
if (this->functionNameId == nullptr)
2833
{
2934
retStr = library->GetFunctionDisplayString();
30-
this->functionNameId = retStr;
3135
}
3236
else
3337
{
38+
if (this->GetTypeHandler()->IsDeferredTypeHandler())
39+
{
40+
JavascriptString* functionName = nullptr;
41+
DebugOnly(bool status = ) this->GetFunctionName(&functionName);
42+
Assert(status);
43+
this->SetPropertyWithAttributes(PropertyIds::name, functionName, PropertyConfigurable, nullptr);
44+
}
3445
if (TaggedInt::Is(this->functionNameId))
3546
{
36-
if (this->GetTypeHandler()->IsDeferredTypeHandler())
37-
{
38-
JavascriptString* functionName = nullptr;
39-
DebugOnly(bool status = ) this->GetFunctionName(&functionName);
40-
Assert(status);
41-
this->SetPropertyWithAttributes(PropertyIds::name, functionName, PropertyConfigurable, nullptr);
42-
}
43-
4447
// This has a side-effect where any other code (such as debugger) that uses functionNameId value will now get the value like "function foo() { native code }"
4548
// instead of just "foo". Alternative ways will need to be devised; if it's not desirable to use this full display name value in those cases.
46-
retStr = GetNativeFunctionDisplayString(scriptContext, scriptContext->GetPropertyString(TaggedInt::ToInt32(this->functionNameId)));
47-
this->functionNameId = retStr;
49+
retStr = GetNativeFunctionDisplayString(scriptContext, scriptContext->GetPropertyString(TaggedInt::ToInt32(this->functionNameId)));
4850
}
4951
else
5052
{
51-
retStr = VarTo<JavascriptString>(this->functionNameId);
53+
retStr = GetNativeFunctionDisplayString(scriptContext, VarTo<JavascriptString>(this->functionNameId));
5254
}
5355
}
56+
57+
this->functionNameId = retStr;
58+
this->isDisplayString = true;
59+
5460
return retStr;
5561
}
5662

@@ -62,6 +68,7 @@ namespace Js
6268

6369
// We are only reference the propertyId, it needs to be tracked to stay alive
6470
Assert(!TaggedInt::Is(nameId) || this->GetScriptContext()->IsTrackedPropertyId(TaggedInt::ToInt32(nameId)));
71+
this->isDisplayString = false;
6572
this->functionNameId = nameId;
6673
}
6774

‎lib/Runtime/Library/RuntimeFunction.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ namespace Js
2525
// NOTE: This has a side-effect that after toString() is called for the first time on a built-in function the functionNameId gets replaced with a string like "function foo() { native code }".
2626
// As a result any code like debugger(F12) that shows the functionNameId to the user will need to pre-process this string as it may not be desirable to use this as-is in some cases.
2727
// See RuntimeFunction::EnsureSourceString() for details.
28+
Field(bool) isDisplayString;
2829
Field(Var) functionNameId;
2930
virtual Var GetSourceString() const { return functionNameId; }
3031
virtual JavascriptString * EnsureSourceString();

0 commit comments

Comments
 (0)