Skip to content

Commit e9293b1

Browse files
committed
[MERGE #4812 @akroshg] ChakraCore 2018-03 Security updates
Merge pull request #4812 from akroshg:test1803_1 Pushing 18-03 changes.
2 parents fed06e6 + 66bba57 commit e9293b1

33 files changed

+270
-257
lines changed

Build/Common.Build.props

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@
7878
<!-- Separate global variable for linker -->
7979
<AdditionalOptions>%(AdditionalOptions) /Gw</AdditionalOptions>
8080

81+
<!-- Enable warnings not included in W4 by default -->
82+
<AdditionalOptions>%(AdditionalOptions) /w44242 /w44254</AdditionalOptions>
83+
8184
<ProgramDataBaseFileName Condition="'$(ConfigurationType)'=='StaticLibrary'">$(IntDir)$(TargetName).pdb</ProgramDataBaseFileName>
8285
<ProgramDataBaseFileName Condition="'$(ConfigurationType)'!='StaticLibrary'">$(IntDir)</ProgramDataBaseFileName>
8386

Build/NuGet/.pack-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.8.1
1+
1.8.2

bin/NativeTests/NativeTests.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#define CATCH_CONFIG_RUNNER
88
#pragma warning(push)
99
// conversion from 'int' to 'char', possible loss of data
10+
#pragma warning(disable:4242)
1011
#pragma warning(disable:4244)
1112
#include "catch.hpp"
1213
#pragma warning(pop)

lib/Backend/BailOut.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1301,8 +1301,11 @@ BailOutRecord::BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, Bail
13011301
InlineeFrameRecord* inlineeFrameRecord = entryPointInfo->FindInlineeFrame(returnAddress);
13021302
if (inlineeFrameRecord)
13031303
{
1304+
// While bailing out, RestoreFrames should box all Vars on the stack. If there are multiple Vars pointing to the same
1305+
// object, the cached version (that was previously boxed) will be reused to maintain pointer identity and correctness
1306+
// after the transition to the interpreter.
13041307
InlinedFrameLayout* outerMostFrame = (InlinedFrameLayout *)(((uint8 *)Js::JavascriptCallStackLayout::ToFramePointer(layout)) - entryPointInfo->frameHeight);
1305-
inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout, false /* deepCopy */);
1308+
inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout, true /* boxArgs */);
13061309
}
13071310
}
13081311

lib/Backend/GlobOpt.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,8 +2481,11 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
24812481
CurrentBlockData()->KillStateForGeneratorYield();
24822482
}
24832483

2484-
// Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
2485-
this->TryReplaceLdLen(instr);
2484+
if (!IsLoopPrePass())
2485+
{
2486+
// Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
2487+
this->TryReplaceLdLen(instr);
2488+
}
24862489

24872490
// Consider: Do we ever get post-op bailout here, and if so is the FillBailOutInfo call in the right place?
24882491
if (instr->HasBailOutInfo() && !this->IsLoopPrePass())
@@ -13440,6 +13443,9 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef)
1344013443
return;
1344113444
}
1344213445

13446+
const bool isLikelyVirtualTypedArray = baseValueType.IsLikelyOptimizedVirtualTypedArray();
13447+
Assert(!(isLikelyJsArray && isLikelyVirtualTypedArray));
13448+
1344313449
ValueType newBaseValueType(baseValueType.ToDefiniteObject());
1344413450
if(isLikelyJsArray && newBaseValueType.HasNoMissingValues() && !DoArrayMissingValueCheckHoist())
1344513451
{
@@ -13770,7 +13776,7 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef)
1377013776
{
1377113777
const JsArrayKills loopKills(loop->jsArrayKills);
1377213778
Value *baseValueInLoopLandingPad = nullptr;
13773-
if((isLikelyJsArray && loopKills.KillsValueType(newBaseValueType)) ||
13779+
if(((isLikelyJsArray || isLikelyVirtualTypedArray) && loopKills.KillsValueType(newBaseValueType)) ||
1377413780
!OptIsInvariant(baseOpnd->m_sym, currentBlock, loop, baseValue, true, true, &baseValueInLoopLandingPad) ||
1377513781
!(doArrayChecks || baseValueInLoopLandingPad->GetValueInfo()->IsObject()))
1377613782
{
@@ -17384,7 +17390,9 @@ GlobOpt::DoArrayCheckHoist(const ValueType baseValueType, Loop* loop, IR::Instr
1738417390
return false;
1738517391
}
1738617392

17387-
if(!baseValueType.IsLikelyArrayOrObjectWithArray() ||
17393+
// This includes typed arrays, but not virtual typed arrays, whose vtable can change if the buffer goes away.
17394+
// Note that in the virtual case the vtable check is the only way to catch this, since there's no bound check.
17395+
if(!(baseValueType.IsLikelyArrayOrObjectWithArray() || baseValueType.IsLikelyOptimizedVirtualTypedArray()) ||
1738817396
(loop ? ImplicitCallFlagsAllowOpts(loop) : ImplicitCallFlagsAllowOpts(func)))
1738917397
{
1739017398
return true;

lib/Backend/GlobOpt.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,16 @@ class JsArrayKills
332332
public:
333333
bool KillsValueType(const ValueType valueType) const
334334
{
335-
Assert(valueType.IsArrayOrObjectWithArray());
335+
Assert(valueType.IsArrayOrObjectWithArray() || valueType.IsOptimizedVirtualTypedArray());
336336

337337
return
338338
killsAllArrays ||
339-
(killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) ||
340-
(killsNativeArrays && !valueType.HasVarElements());
339+
(valueType.IsArrayOrObjectWithArray() &&
340+
(
341+
(killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) ||
342+
(killsNativeArrays && !valueType.HasVarElements())
343+
)
344+
);
341345
}
342346

343347
bool AreSubsetOf(const JsArrayKills &other) const

lib/Backend/InlineeFrameInfo.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,14 @@ void InlineeFrameRecord::Finalize(Func* inlinee, uint32 currentOffset)
199199
Assert(this->inlineDepth != 0);
200200
}
201201

202-
void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const
202+
void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout, bool boxValues) const
203203
{
204204
Assert(this->inlineDepth != 0);
205205
Assert(inlineeStartOffset != 0);
206206

207207
BAILOUT_VERBOSE_TRACE(functionBody, _u("Restore function object: "));
208208
// No deepCopy needed for just the function
209-
Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody, /*deepCopy*/ false);
209+
Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody, boxValues);
210210
Assert(Js::ScriptFunction::Is(varFunction));
211211

212212
Js::ScriptFunction* function = Js::ScriptFunction::FromVar(varFunction);
@@ -222,9 +222,9 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
222222

223223
// Forward deepCopy flag for the arguments in case their data must be guaranteed
224224
// to have its own lifetime
225-
Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody, deepCopy);
225+
Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody, boxValues);
226226
#if DBG
227-
if (!Js::TaggedNumber::Is(var))
227+
if (boxValues && !Js::TaggedNumber::Is(var))
228228
{
229229
Js::RecyclableObject *const recyclableObject = Js::RecyclableObject::FromVar(var);
230230
Assert(!ThreadContext::IsOnStack(recyclableObject));
@@ -236,7 +236,10 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
236236
BAILOUT_FLUSH(functionBody);
237237
}
238238

239-
void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack, bool deepCopy)
239+
// Note: the boxValues parameter should be true when this is called from a Bailout codepath to ensure that multiple vars to
240+
// the same object reuse the cached value during the transition to the interpreter.
241+
// Otherwise, this parameter should be false as the values are not required to be moved to the heap to restore the frame.
242+
void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack, bool boxValues)
240243
{
241244
InlineeFrameRecord* innerMostRecord = this;
242245
class AutoReverse
@@ -274,7 +277,7 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr
274277

275278
while (currentRecord)
276279
{
277-
currentRecord->Restore(functionBody, currentFrame, callstack, deepCopy);
280+
currentRecord->Restore(functionBody, currentFrame, callstack, boxValues);
278281
currentRecord = currentRecord->parent;
279282
currentFrame = currentFrame->Next();
280283
}
@@ -283,10 +286,10 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr
283286
currentFrame->callInfo.Count = 0;
284287
}
285288

286-
Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const
289+
Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool boxValue) const
287290
{
288291
Js::Var value;
289-
bool boxStackInstance = true;
292+
bool boxStackInstance = boxValue;
290293
double dblValue;
291294
if (offset >= 0)
292295
{
@@ -324,8 +327,11 @@ Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js
324327
BAILOUT_VERBOSE_TRACE(functionBody, _u(", value: 0x%p"), value);
325328
if (boxStackInstance)
326329
{
330+
// Do not deepCopy in this call to BoxStackInstance because this should be used for
331+
// bailing out, where a shallow copy that is cached is needed to ensure that multiple
332+
// vars pointing to the same boxed object reuse the new boxed value.
327333
Js::Var oldValue = value;
328-
value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true, deepCopy);
334+
value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true, false /* deepCopy */);
329335

330336
#if ENABLE_DEBUG_CONFIG_OPTIONS
331337
if (oldValue != value)

lib/Backend/InlineeFrameInfo.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ struct InlineeFrameRecord
108108
}
109109

110110
void PopulateParent(Func* func);
111-
void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack, bool deepCopy);
111+
void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack, bool boxValues);
112112
void Finalize(Func* inlinee, uint currentOffset);
113113
#if DBG_DUMP
114114
void Dump() const;
@@ -123,8 +123,8 @@ struct InlineeFrameRecord
123123
}
124124

125125
private:
126-
void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const;
127-
Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const;
126+
void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout, bool boxValues) const;
127+
Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool boxValue) const;
128128
InlineeFrameRecord* Reverse();
129129
};
130130

lib/Common/ChakraCoreVersion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// ChakraCore version number definitions (used in ChakraCore binary metadata)
1818
#define CHAKRA_CORE_MAJOR_VERSION 1
1919
#define CHAKRA_CORE_MINOR_VERSION 8
20-
#define CHAKRA_CORE_PATCH_VERSION 1
20+
#define CHAKRA_CORE_PATCH_VERSION 2
2121
#define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0.
2222

2323
// -------------

lib/Common/CommonDefines.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@
324324

325325
#ifndef NTBUILD
326326
#define DELAYLOAD_SECTIONAPI 1
327+
#define DELAYLOAD_UNLOCKMEMORY 1
327328
#endif
328329

329330
#ifdef NTBUILD

0 commit comments

Comments
 (0)