Skip to content

Commit ded0397

Browse files
author
Jianchun Xu
committed
JIT: signed integer overflow and other fixes
Address CR comments. Refactored IntMath classes to share common code and use builtins when available. Cleaned up signed integer overflow cases. Added build.sh --sanity=... support to help future diagnostics. Turned off CLANG signed integer overflow optimization with `-fwrapv` switch. Fixed a ICU call bug (found in running Octane/typescript.js). Implemented another AsmJs thunk. (called by Octane/zlib.js). Synced to latest and fix merge errors.
1 parent 60d5d6b commit ded0397

28 files changed

+394
-470
lines changed

‎CMakeLists.txt

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,15 @@ if(CLR_CMAKE_PLATFORM_XPLAT)
253253
add_compile_options(
254254
-fasm-blocks
255255
-fms-extensions
256+
-fwrapv # Treat signed integer overflow as two's complement
256257
)
258+
259+
# Clang -fsanitize.
260+
if (CLANG_SANITIZE_SH)
261+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=${CLANG_SANITIZE_SH}")
262+
set(CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS} -fsanitize=${CLANG_SANITIZE_SH}")
263+
unset(CLANG_SANITIZE_SH CACHE) # don't cache
264+
endif()
257265
endif(CLR_CMAKE_PLATFORM_XPLAT)
258266

259267
if(CMAKE_BUILD_TYPE STREQUAL Debug)
@@ -286,16 +294,19 @@ if(CLR_CMAKE_PLATFORM_XPLAT)
286294
add_definitions(-DFEATURE_PAL)
287295
endif(CLR_CMAKE_PLATFORM_XPLAT)
288296

289-
if(NO_JIT
297+
if(NO_JIT_SH
290298
OR CLR_CMAKE_PLATFORM_DARWIN) # TODO: JIT for OSX
299+
unset(NO_JIT_SH CACHE) # don't cache
300+
unset(BuildJIT CACHE) # also clear it just in case
291301
add_definitions(-DDISABLE_JIT=1)
292302
else()
293303
set(BuildJIT 1)
294304
endif()
295305

296-
if(WITHOUT_FEATURES)
297-
add_definitions(${WITHOUT_FEATURES})
298-
endif(WITHOUT_FEATURES)
306+
if(WITHOUT_FEATURES_SH)
307+
unset(WITHOUT_FEATURES_SH CACHE) # don't cache
308+
add_definitions(${WITHOUT_FEATURES_SH})
309+
endif(WITHOUT_FEATURES_SH)
299310

300311
enable_language(ASM)
301312

‎build.sh

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ PRINT_USAGE() {
4242
echo " --xcode Generate XCode project"
4343
echo " -t, --test-build Test build (by default Release build)"
4444
echo " --static Build as static library (by default shared library)"
45+
echo " --sanitize=CHECKS Build with clang -fsanitize checks,"
46+
echo " e.g. undefined,signed-integer-overflow"
4547
echo " -v, --verbose Display verbose output including all options"
4648
echo " --create-deb=V Create .deb package with given V version"
4749
echo " --without=FEATURE,FEATURE,..."
@@ -66,6 +68,7 @@ MULTICORE_BUILD=""
6668
NO_JIT=
6769
ICU_PATH="-DICU_SETTINGS_RESET=1"
6870
STATIC_LIBRARY="-DSHARED_LIBRARY_SH=1"
71+
SANITIZE=
6972
WITHOUT_FEATURES=""
7073
CREATE_DEB=0
7174
ARCH="-DCC_TARGETS_AMD64_SH=1"
@@ -194,7 +197,7 @@ while [[ $# -gt 0 ]]; do
194197
;;
195198

196199
--no-jit)
197-
NO_JIT="-DNO_JIT=1"
200+
NO_JIT="-DNO_JIT_SH=1"
198201
;;
199202

200203
--xcode)
@@ -211,13 +214,19 @@ while [[ $# -gt 0 ]]; do
211214
STATIC_LIBRARY="-DSTATIC_LIBRARY_SH=1"
212215
;;
213216

217+
--sanitize=*)
218+
SANITIZE=$1
219+
SANITIZE=${SANITIZE:11} # value after --sanitize=
220+
SANITIZE="-DCLANG_SANITIZE_SH=${SANITIZE}"
221+
;;
222+
214223
--without=*)
215224
FEATURES=$1
216225
FEATURES=${FEATURES:10} # value after --without=
217226
for x in ${FEATURES//,/ } # replace comma with space then split
218227
do
219228
if [[ "$WITHOUT_FEATURES" == "" ]]; then
220-
WITHOUT_FEATURES="-DWITHOUT_FEATURES="
229+
WITHOUT_FEATURES="-DWITHOUT_FEATURES_SH="
221230
else
222231
WITHOUT_FEATURES="$WITHOUT_FEATURES;"
223232
fi
@@ -312,7 +321,8 @@ else
312321
fi
313322

314323
echo Generating $BUILD_TYPE makefiles
315-
cmake $CMAKE_GEN $CC_PREFIX $ICU_PATH $STATIC_LIBRARY $ARCH -DCMAKE_BUILD_TYPE=$BUILD_TYPE $NO_JIT $WITHOUT_FEATURES ../..
324+
cmake $CMAKE_GEN $CC_PREFIX $ICU_PATH $STATIC_LIBRARY $ARCH \
325+
-DCMAKE_BUILD_TYPE=$BUILD_TYPE $SANITIZE $NO_JIT $WITHOUT_FEATURES ../..
316326

317327
_RET=$?
318328
if [[ $? == 0 ]]; then

‎lib/Backend/EhFrame.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ void EhFrame::Entry::Begin()
119119

120120
void EhFrame::Entry::End()
121121
{
122+
Assert(beginOffset != -1); // Must have called Begin()
123+
122124
// padding
123125
size_t padding = (MachPtr - writer->Count() % MachPtr) % MachPtr;
124126
for (size_t i = 0; i < padding; i++)

‎lib/Backend/Func.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1723,7 +1723,7 @@ IR::IndirOpnd * Func::GetConstantAddressIndirOpnd(intptr_t address, IR::AddrOpnd
17231723
{
17241724
Assert(regOpnd->m_sym->IsSingleDef());
17251725
void * curr = regOpnd->m_sym->m_instrDef->GetSrc1()->AsAddrOpnd()->m_address;
1726-
ptrdiff_t diff = (intptr_t)address - (intptr_t)curr;
1726+
ptrdiff_t diff = (uintptr_t)address - (uintptr_t)curr;
17271727
if (!Math::FitsInDWord(diff))
17281728
{
17291729
return false;

‎lib/Backend/GlobOpt.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8480,7 +8480,7 @@ GlobOpt::PropagateIntRangeBinary(IR::Instr *instr, int32 min1, int32 max1,
84808480
{
84818481
// Turn values like 0x1010 into 0x1111
84828482
max = 1 << Math::Log2(max);
8483-
max = (max << 1) - 1;
8483+
max = (uint32)(max << 1) - 1;
84848484
min = 0;
84858485
}
84868486

@@ -8566,7 +8566,7 @@ GlobOpt::PropagateIntRangeBinary(IR::Instr *instr, int32 min1, int32 max1,
85668566
if (max1)
85678567
{
85688568
max1 = 1 << Math::Log2(max1);
8569-
max1 = (max1 << 1) - 1;
8569+
max1 = (uint32)(max1 << 1) - 1;
85708570
}
85718571

85728572
if (max1 > 0)

‎lib/Backend/IntBounds.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,8 @@ bool IntBounds::IsGreaterThanOrEqualTo(const int constantValue, const int consta
304304
if(offset == 1)
305305
return constantValue > constantBoundBase;
306306

307-
const int constantBound = constantBoundBase + offset;
307+
// use unsigned to avoid signed int overflow
308+
const int constantBound = (unsigned)constantBoundBase + (unsigned)offset;
308309
return
309310
offset >= 0
310311
? constantBound >= constantBoundBase && constantValue >= constantBound
@@ -318,7 +319,8 @@ bool IntBounds::IsLessThanOrEqualTo(const int constantValue, const int constantB
318319
if(offset == -1)
319320
return constantValue < constantBoundBase;
320321

321-
const int constantBound = constantBoundBase + offset;
322+
// use unsigned to avoid signed int overflow
323+
const int constantBound = (unsigned)constantBoundBase + (unsigned)offset;
322324
return
323325
offset >= 0
324326
? constantBound < constantBoundBase || constantValue <= constantBound

‎lib/Backend/IntConstantBounds.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class IntConstantBounds
6565
IntConstantBounds And_0x1f() const
6666
{
6767
const int32 mask = 0x1f;
68-
if(static_cast<UIntConstType>(upperBound - lowerBound) >= static_cast<UIntConstType>(mask) ||
68+
if(static_cast<UIntConstType>(upperBound) - static_cast<UIntConstType>(lowerBound) >= static_cast<UIntConstType>(mask) ||
6969
(lowerBound & mask) > (upperBound & mask))
7070
{
7171
// The range contains all items in the set {0-mask}, or the range crosses a boundary of {0-mask}. Since we cannot

‎lib/Backend/Lower.cpp

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
254254
case Js::OpCode::InvalCachedScope:
255255
this->LowerBinaryHelper(instr, IR::HelperOP_InvalidateCachedScope);
256256
break;
257-
257+
258258
case Js::OpCode::InitCachedScope:
259259
instrPrev = this->LowerInitCachedScope(instr);
260260
break;
@@ -3166,7 +3166,7 @@ Lowerer::LoadOptimizationOverridesValueOpnd(IR::Instr *instr, OptimizationOverri
31663166

31673167
IR::Opnd *
31683168
Lowerer::LoadNumberAllocatorValueOpnd(IR::Instr *instr, NumberAllocatorValue valueType)
3169-
{
3169+
{
31703170
ScriptContextInfo *scriptContext = instr->m_func->GetScriptContextInfo();
31713171
bool allowNativeCodeBumpAllocation = scriptContext->GetRecyclerAllowNativeCodeBumpAllocation();
31723172

@@ -3513,7 +3513,7 @@ Lowerer::LowerNewScObjectLiteral(IR::Instr *newObjInstr)
35133513
propertyArrayOpnd = IR::AddrOpnd::New(propArrayAddr, IR::AddrOpndKindDynamicMisc, this->m_func);
35143514

35153515
//#if 0 TODO: OOP JIT, obj literal types
3516-
// should pass in isShared bit through RPC, enable for in-proc jit to see perf impact
3516+
// should pass in isShared bit through RPC, enable for in-proc jit to see perf impact
35173517
Js::DynamicType * literalType = func->IsOOPJIT() || !CONFIG_FLAG(OOPJITMissingOpts) ? nullptr : *(Js::DynamicType **)literalTypeRef;
35183518

35193519
if (literalType == nullptr || !literalType->GetIsShared())
@@ -6832,7 +6832,7 @@ Lowerer::GenerateStFldWithCachedType(IR::Instr *instrStFld, bool* continueAsHelp
68326832

68336833
if (hasTypeCheckBailout)
68346834
{
6835-
AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !propertySymOpnd->IsTypeDead(),
6835+
AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !propertySymOpnd->IsTypeDead(),
68366836
"Why does a field store have a type check bailout, if its type is dead?");
68376837

68386838
if (instrStFld->GetBailOutInfo()->bailOutInstr != instrStFld)
@@ -8590,7 +8590,7 @@ Lowerer::LowerMemset(IR::Instr * instr, IR::RegOpnd * helperRet)
85908590
m_lowererMD.LoadHelperArgument(instr, baseOpnd);
85918591
m_lowererMD.ChangeToHelperCall(instr, helperMethod);
85928592
dst->Free(m_func);
8593-
8593+
85948594
return instrPrev;
85958595
}
85968596

@@ -8865,10 +8865,10 @@ Lowerer::GenerateFastBrBReturn(IR::Instr * instr)
88658865
// TEST firstPrototypeOpnd, firstPrototypeOpnd
88668866
// JNE $helper
88678867
IR::RegOpnd * firstPrototypeOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
8868-
InsertMove(firstPrototypeOpnd,
8868+
InsertMove(firstPrototypeOpnd,
88698869
IR::IndirOpnd::New(forInEnumeratorOpnd, Js::ForInObjectEnumerator::GetOffsetOfFirstPrototype(), TyMachPtr, this->m_func), instr);
88708870
InsertTestBranch(firstPrototypeOpnd, firstPrototypeOpnd, Js::OpCode::BrNeq_A, labelHelper, instr);
8871-
8871+
88728872
// MOV currentEnumeratorOpnd, forInEnumerator->enumerator.currentEnumerator
88738873
// TEST currentEnumeratorOpnd, currentEnumeratorOpnd
88748874
// JNE $helper
@@ -8883,7 +8883,7 @@ Lowerer::GenerateFastBrBReturn(IR::Instr * instr)
88838883
IR::RegOpnd * objectOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
88848884
InsertMove(objectOpnd,
88858885
IR::IndirOpnd::New(forInEnumeratorOpnd, Js::ForInObjectEnumerator::GetOffsetOfEnumeratorObject(), TyMachPtr, this->m_func), instr);
8886-
InsertTestBranch(objectOpnd, objectOpnd, Js::OpCode::BrEq_A, labelHelper, instr);
8886+
InsertTestBranch(objectOpnd, objectOpnd, Js::OpCode::BrEq_A, labelHelper, instr);
88878887

88888888
// MOV initialTypeOpnd, forInEnumerator->enumerator.initialType
88898889
// CMP initialTypeOpnd, objectOpnd->type
@@ -8907,7 +8907,7 @@ Lowerer::GenerateFastBrBReturn(IR::Instr * instr)
89078907
InsertCompareBranch(enumeratedCountOpnd,
89088908
IR::IndirOpnd::New(cachedDataOpnd, Js::DynamicObjectPropertyEnumerator::GetOffsetOfCachedDataCachedCount(), TyUint32, this->m_func),
89098909
Js::OpCode::BrGe_A, labelHelper, instr);
8910-
8910+
89118911
// MOV propertyAttributesOpnd, cachedData->attributes
89128912
// MOV objectPropertyAttributesOpnd, propertyAttributesOpnd[enumeratedCount]
89138913
// CMP objectPropertyAttributesOpnd & PropertyEnumerable, PropertyEnumerable
@@ -8923,7 +8923,7 @@ Lowerer::GenerateFastBrBReturn(IR::Instr * instr)
89238923

89248924
InsertCompareBranch(andPropertyEnumerableInstr->GetDst(), IR::IntConstOpnd::New(PropertyEnumerable, TyUint8, this->m_func),
89258925
Js::OpCode::BrNeq_A, labelHelper, instr);
8926-
8926+
89278927
IR::Opnd * opndDst = instr->GetDst(); // ForIn result propertyString
89288928
Assert(opndDst->IsRegOpnd());
89298929

@@ -9932,7 +9932,7 @@ Lowerer::LowerArgIn(IR::Instr *instrArgIn)
99329932
// ...
99339933
// s2 = assign param2
99349934
// $done:
9935-
9935+
99369936
AnalysisAssert(instrArgIn);
99379937

99389938
IR::Opnd *restDst = nullptr;
@@ -10086,7 +10086,7 @@ Lowerer::LowerArgIn(IR::Instr *instrArgIn)
1008610086

1008710087

1008810088
BVSparse<JitArenaAllocator> *formalsBv = JitAnew(this->m_alloc, BVSparse<JitArenaAllocator>, this->m_alloc);
10089-
10089+
1009010090
while (currArgInCount > 0)
1009110091
{
1009210092
dstOpnd = instrArgIn->GetDst();
@@ -10104,9 +10104,9 @@ Lowerer::LowerArgIn(IR::Instr *instrArgIn)
1010410104
// BrEq_A $Ln-1
1010510105

1010610106
currArgInCount--;
10107-
10107+
1010810108
labelInitNext = IR::LabelInstr::New(Js::OpCode::Label, this->m_func);
10109-
10109+
1011010110

1011110111
// And insert the "normal" initialization before the "done" label
1011210112

@@ -10206,7 +10206,7 @@ Lowerer::LowerArgIn(IR::Instr *instrArgIn)
1020610206
this->m_lowererMD.ChangeToAssign(instrArgIn);
1020710207
}
1020810208

10209-
JitAdelete(this->m_alloc, formalsBv);
10209+
JitAdelete(this->m_alloc, formalsBv);
1021010210

1021110211
return instrResume;
1021210212
}
@@ -10248,8 +10248,8 @@ Lowerer::LoadGeneratorObject(IR::Instr * instrInsert)
1024810248
instrInsert->m_func->SetArgOffset(generatorSym, LowererMD::GetFormalParamOffset() * MachPtr);
1024910249
IR::SymOpnd * generatorSymOpnd = IR::SymOpnd::New(generatorSym, TyMachPtr, instrInsert->m_func);
1025010250
IR::RegOpnd * generatorRegOpnd = IR::RegOpnd::New(TyMachPtr, instrInsert->m_func);
10251-
instrInsert->m_func->SetHasImplicitParamLoad();
10252-
return LowererMD::CreateAssign(generatorRegOpnd, generatorSymOpnd, instrInsert);
10251+
instrInsert->m_func->SetHasImplicitParamLoad();
10252+
return LowererMD::CreateAssign(generatorRegOpnd, generatorSymOpnd, instrInsert);
1025310253
}
1025410254

1025510255
IR::Instr *
@@ -10904,7 +10904,7 @@ Lowerer::LoadCallInfo(IR::Instr * instrInsert)
1090410904
{
1090510905
// Generator function arguments and ArgumentsInfo are not on the stack. Instead they
1090610906
// are accessed off the generator object (which is prm1).
10907-
IR::Instr *genLoadInstr = LoadGeneratorObject(instrInsert);
10907+
IR::Instr *genLoadInstr = LoadGeneratorObject(instrInsert);
1090810908
IR::RegOpnd * generatorRegOpnd = genLoadInstr->GetDst()->AsRegOpnd();
1090910909

1091010910
IR::IndirOpnd * indirOpnd = IR::IndirOpnd::New(generatorRegOpnd, Js::JavascriptGenerator::GetCallInfoOffset(), TyMachPtr, func);
@@ -12543,7 +12543,7 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
1254312543
else
1254412544
{
1254512545
indexOpnd = IR::MemRefOpnd::New((BYTE*)bailOutInfo->bailOutRecord + BailOutRecord::GetOffsetOfPolymorphicCacheIndex(), TyUint32, this->m_func);
12546-
}
12546+
}
1254712547

1254812548
m_lowererMD.CreateAssign(
1254912549
indexOpnd, IR::IntConstOpnd::New(bailOutInfo->polymorphicCacheIndex, TyUint32, this->m_func), instr);
@@ -14694,7 +14694,7 @@ Lowerer::GenerateFastElemIIntIndexCommon(
1469414694
const bool needBailOutOnInvalidLength = !!(bailOutKind & (IR::BailOutOnInvalidatedArrayHeadSegment));
1469514695
const bool needBailOutToHelper = !!(bailOutKind & (IR::BailOutOnArrayAccessHelperCall | IR::BailOutOnInvalidatedArrayLength));
1469614696
const bool needBailOutOnSegmentLengthCompare = needBailOutToHelper || needBailOutOnInvalidLength;
14697-
14697+
1469814698
if(indexIsLessThanHeadSegmentLength || needBailOutOnSegmentLengthCompare)
1469914699
{
1470014700
if (needBailOutOnSegmentLengthCompare)
@@ -18522,7 +18522,7 @@ Lowerer::GenerateFastArgumentsLdElemI(IR::Instr* ldElem, IR::LabelInstr *labelFa
1852218522
ldElem->InsertBefore(labelCreateHeapArgs);
1852318523
emittedFastPath = true;
1852418524
}
18525-
18525+
1852618526
if (!emittedFastPath)
1852718527
{
1852818528
throw Js::RejitException(RejitReason::DisableStackArgOpt);
@@ -20633,7 +20633,7 @@ Lowerer::GenerateLdHomeObjProto(IR::Instr* instr)
2063320633
//
2063420634
// $Err:
2063520635
// ThrowRuntimeReferenceError(JSERR_BadSuperReference);
20636-
//
20636+
//
2063720637
// $NoErr:
2063820638
// instance = ((RecyclableObject*)instance)->GetPrototype();
2063920639
// if (instance == nullptr) goto $Done;

0 commit comments

Comments
 (0)