Skip to content

Commit 25dbe89

Browse files
CelluleCellule
committed
[MERGE chakra-core#1361 @Cellule] MemSet incorrect copy for native float array with -1
Merge pull request chakra-core#1361 from Cellule:memop_setsamebytes We used to check if the memset value is -1 to determine if we can do a byte level memset. This was valid ints but not for floats. Changed the check to really see if the bytes are all the same.
2 parents 9321f6f + e961167 commit 25dbe89

4 files changed

Lines changed: 64 additions & 35 deletions

File tree

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21446,7 +21446,7 @@ GlobOpt::EmitMemop(Loop * loop, LoopCount *loopCount, const MemOpEmitData* emitD
2144621446
case TyInt16:
2144721447
case TyInt32:
2144821448
case TyInt64:
21449-
_snwprintf_s(constBuf, constBufSize, sizeof(IntConstType) == 8 ? _u("lld%") : _u("%d"), candidate->constant.u.intConst.value);
21449+
_snwprintf_s(constBuf, constBufSize, sizeof(IntConstType) == 8 ? _u("%lld") : _u("%d"), candidate->constant.u.intConst.value);
2145021450
break;
2145121451
case TyFloat32:
2145221452
case TyFloat64:

‎lib/Runtime/Library/JavascriptArray.cpp‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3956,6 +3956,62 @@ namespace Js
39563956
return -1;
39573957
}
39583958

3959+
template<typename T>
3960+
bool AreAllBytesEqual(T value)
3961+
{
3962+
byte* bValue = (byte*)&value;
3963+
byte firstByte = *bValue++;
3964+
for (int i = 1; i < sizeof(T); ++i)
3965+
{
3966+
if (*bValue++ != firstByte)
3967+
{
3968+
return false;
3969+
}
3970+
}
3971+
return true;
3972+
}
3973+
3974+
template<>
3975+
void JavascriptArray::CopyValueToSegmentBuferNoCheck(double* buffer, uint32 length, double value)
3976+
{
3977+
if (JavascriptNumber::IsZero(value) && !JavascriptNumber::IsNegZero(value))
3978+
{
3979+
memset(buffer, 0, sizeof(double) * length);
3980+
}
3981+
else
3982+
{
3983+
for (uint32 i = 0; i < length; i++)
3984+
{
3985+
buffer[i] = value;
3986+
}
3987+
}
3988+
}
3989+
3990+
template<>
3991+
void JavascriptArray::CopyValueToSegmentBuferNoCheck(int32* buffer, uint32 length, int32 value)
3992+
{
3993+
if (value == 0 || AreAllBytesEqual(value))
3994+
{
3995+
memset(buffer, *(byte*)&value, sizeof(int32)* length);
3996+
}
3997+
else
3998+
{
3999+
for (uint32 i = 0; i < length; i++)
4000+
{
4001+
buffer[i] = value;
4002+
}
4003+
}
4004+
}
4005+
4006+
template<>
4007+
void JavascriptArray::CopyValueToSegmentBuferNoCheck(Js::Var* buffer, uint32 length, Js::Var value)
4008+
{
4009+
for (uint32 i = 0; i < length; i++)
4010+
{
4011+
buffer[i] = value;
4012+
}
4013+
}
4014+
39594015
int32 JavascriptNativeIntArray::HeadSegmentIndexOfHelper(Var search, uint32 &fromIndex, uint32 toIndex, bool includesAlgorithm, ScriptContext * scriptContext)
39604016
{
39614017
// We proceed largely in the same manner as in JavascriptArray's version of this method (see comments there for more information),

‎lib/Runtime/Library/JavascriptArray.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ namespace Js
169169
#if ENABLE_PROFILE_INFO
170170
template<typename T> inline void DirectProfiledSetItemInHeadSegmentAt(const uint32 offset, const T newValue, StElemInfo *const stElemInfo);
171171
#endif
172+
template<typename T> static void CopyValueToSegmentBuferNoCheck(T* buffer, uint32 length, T value);
172173
template<typename T> void DirectSetItem_Full(uint32 itemIndex, T newValue);
173174
template<typename T> SparseArraySegment<T>* PrepareSegmentForMemOp(uint32 startIndex, uint32 length);
174175
template<typename T> bool DirectSetItemAtRange(uint32 startIndex, uint32 length, T newValue);

‎lib/Runtime/Library/JavascriptArray.inl‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,7 @@ SECOND_PASS:
10141014
#endif
10151015
return true;
10161016
}
1017+
10171018
template<typename T>
10181019
bool JavascriptArray::DirectSetItemAtRange(uint32 startIndex, uint32 length, T newValue)
10191020
{
@@ -1033,18 +1034,7 @@ SECOND_PASS:
10331034

10341035
if (startIndex == 0 && head != EmptySegment && length < head->size)
10351036
{
1036-
if (newValue == (T)0 || newValue == (T)(-1))
1037-
{
1038-
memset(((Js::SparseArraySegment<T>*)head)->elements, ((int)(intptr_t)newValue), sizeof(T)* length);
1039-
}
1040-
else
1041-
{
1042-
Js::SparseArraySegment<T>* headSegment = ((Js::SparseArraySegment<T>*)head);
1043-
for (uint32 i = 0; i < length; i++)
1044-
{
1045-
headSegment->elements[i] = newValue;
1046-
}
1047-
}
1037+
CopyValueToSegmentBuferNoCheck(((Js::SparseArraySegment<T>*)head)->elements, length, newValue);
10481038

10491039
if (length > this->length)
10501040
{
@@ -1082,17 +1072,7 @@ SECOND_PASS:
10821072

10831073
SetHasNoMissingValues(true);
10841074

1085-
if (newValue == (T)0 || newValue == (T)(-1))
1086-
{
1087-
memset(((Js::SparseArraySegment<T>*)current)->elements, ((int)(intptr_t)newValue), sizeof(T)* length);
1088-
}
1089-
else
1090-
{
1091-
for (uint32 i = 0; i < length; i++)
1092-
{
1093-
((Js::SparseArraySegment<T>*)current)->elements[i] = newValue;
1094-
}
1095-
}
1075+
CopyValueToSegmentBuferNoCheck(((Js::SparseArraySegment<T>*)current)->elements, length, newValue);
10961076
this->SetLastUsedSegment(current);
10971077
}
10981078
else
@@ -1116,17 +1096,9 @@ SECOND_PASS:
11161096
{
11171097
return false;
11181098
}
1119-
if (newValue == (T)0 || newValue == (T)(-1))
1120-
{
1121-
memset((((Js::SparseArraySegment<T>*)current)->elements + (startIndex - current->left)), ((int)(intptr_t)newValue), sizeof(T)* length);
1122-
}
1123-
else
1124-
{
1125-
for (uint32 i = 0; i < length; i++)
1126-
{
1127-
((Js::SparseArraySegment<T>*)current)->elements[startIndex - current->left + i] = newValue;
1128-
}
1129-
}
1099+
Assert(current->left + current->length >= startIndex + length);
1100+
T* segmentCopyStart = current->elements + (startIndex - current->left);
1101+
CopyValueToSegmentBuferNoCheck(segmentCopyStart, length, newValue);
11301102
this->SetLastUsedSegment(current);
11311103
#if DBG
11321104
if (Js::Configuration::Global.flags.MemOpMissingValueValidate)

0 commit comments

Comments
 (0)