Skip to content

Commit a562259

Browse files
leirocksleirocks
committed
some write barrier fixes
- false poisitive on EntryPointInfo->jsMethod after native address reuse in recycler - false poisitive in arena allocator cacheBlockEnd reused in leaf block in recycler (sharing same page allocator) - overload ChangeToAssign in JIT code, check all posible assign for write barrier - missing barriers in various location
1 parent f3e5eaf commit a562259

28 files changed

+140
-98
lines changed

‎lib/Backend/Lower.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -102,30 +102,30 @@ Lowerer::Lower()
102102
this->LowerRange(m_func->m_headInstr, m_func->m_tailInstr, defaultDoFastPath, loopFastPath);
103103

104104
#if DBG
105-
// TODO: (leish)(swb) implement for arm
105+
// TODO: (leish)(swb) implement for arm
106106
#if defined(_M_IX86) || defined(_M_AMD64)
107-
if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(RecyclerVerifyMark))
108-
{
109-
// find out all write barrier setting instr, call Recycler::WBSetBit for verification purpose
110-
// should do this in LowererMD::GenerateWriteBarrier, however, can't insert call instruction there
111-
FOREACH_INSTR_EDITING(instr, instrNext, m_func->m_headInstr)
112-
if (instr->m_src1 && instr->m_src1->IsAddrOpnd())
113-
{
114-
IR::AddrOpnd* addrOpnd = instr->m_src1->AsAddrOpnd();
115-
if (addrOpnd->GetAddrOpndKind() == IR::AddrOpndKindWriteBarrierCardTable)
116-
{
117-
auto& leaInstr = instr->m_prev->m_prev;
118-
auto& shrInstr = instr->m_prev;
119-
Assert(leaInstr->m_opcode == Js::OpCode::LEA);
120-
Assert(shrInstr->m_opcode == Js::OpCode::SHR);
121-
m_lowererMD.LoadHelperArgument(shrInstr, leaInstr->m_dst);
122-
IR::Instr* instrCall = IR::Instr::New(Js::OpCode::Call, m_func);
123-
shrInstr->InsertBefore(instrCall);
124-
m_lowererMD.ChangeToHelperCall(instrCall, IR::HelperWriteBarrierSetVerifyBit);
125-
}
126-
}
127-
NEXT_INSTR_EDITING
128-
}
107+
if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(RecyclerVerifyMark))
108+
{
109+
// find out all write barrier setting instr, call Recycler::WBSetBit for verification purpose
110+
// should do this in LowererMD::GenerateWriteBarrier, however, can't insert call instruction there
111+
FOREACH_INSTR_EDITING(instr, instrNext, m_func->m_headInstr)
112+
if (instr->m_src1 && instr->m_src1->IsAddrOpnd())
113+
{
114+
IR::AddrOpnd* addrOpnd = instr->m_src1->AsAddrOpnd();
115+
if (addrOpnd->GetAddrOpndKind() == IR::AddrOpndKindWriteBarrierCardTable)
116+
{
117+
auto& leaInstr = instr->m_prev->m_prev;
118+
auto& shrInstr = instr->m_prev;
119+
Assert(leaInstr->m_opcode == Js::OpCode::LEA);
120+
Assert(shrInstr->m_opcode == Js::OpCode::SHR);
121+
m_lowererMD.LoadHelperArgument(shrInstr, leaInstr->m_dst);
122+
IR::Instr* instrCall = IR::Instr::New(Js::OpCode::Call, m_func);
123+
shrInstr->InsertBefore(instrCall);
124+
m_lowererMD.ChangeToHelperCall(instrCall, IR::HelperWriteBarrierSetVerifyBit);
125+
}
126+
}
127+
NEXT_INSTR_EDITING
128+
}
129129
#endif
130130
#endif
131131

@@ -14558,7 +14558,7 @@ IR::Instr *Lowerer::InsertMove(IR::Opnd *dst, IR::Opnd *src, IR::Instr *const in
1455814558
}
1455914559
else
1456014560
{
14561-
LowererMD::ChangeToAssign(instr);
14561+
LowererMD::ChangeToAssignNoBarrierCheck(instr);
1456214562
}
1456314563

1456414564
return instr;

‎lib/Backend/LowerMDShared.cpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -756,11 +756,17 @@ IR::Instr* LowererMD::ChangeToHelperCallMem(IR::Instr * instr, IR::JnHelperMeth
756756
///----------------------------------------------------------------------------
757757

758758
IR::Instr *
759-
LowererMD::ChangeToAssign(IR::Instr * instr)
759+
LowererMD::ChangeToAssignNoBarrierCheck(IR::Instr * instr)
760760
{
761761
return ChangeToAssign(instr, instr->GetDst()->GetType());
762762
}
763763

764+
IR::Instr *
765+
LowererMD::ChangeToAssign(IR::Instr * instr)
766+
{
767+
return ChangeToWriteBarrierAssign(instr, instr->m_func);
768+
}
769+
764770
IR::Instr *
765771
LowererMD::ChangeToAssign(IR::Instr * instr, IRType type)
766772
{
@@ -4712,7 +4718,7 @@ LowererMD::GenerateLoadPolymorphicInlineCacheSlot(IR::Instr * instrLdSt, IR::Reg
47124718
instrLdSt->InsertBefore(instr);
47134719
}
47144720

4715-
void
4721+
IR::Instr *
47164722
LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
47174723
{
47184724
#ifdef RECYCLER_WRITE_BARRIER_JIT
@@ -4747,7 +4753,7 @@ LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
47474753
}
47484754
#endif
47494755

4750-
ChangeToAssign(assignInstr);
4756+
IR::Instr * instr = ChangeToAssignNoBarrierCheck(assignInstr);
47514757

47524758
// Now insert write barrier if necessary
47534759
#ifdef RECYCLER_WRITE_BARRIER_JIT
@@ -4758,6 +4764,8 @@ LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
47584764
LowererMD::GenerateWriteBarrier(assignInstr);
47594765
}
47604766
#endif
4767+
4768+
return instr;
47614769
}
47624770

47634771
void

‎lib/Backend/LowerMDShared.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class LowererMD
8585
static IR::Instr * CreateAssign(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instrInsertPt, bool generateWriteBarrier = true);
8686

8787
static IR::Instr * ChangeToAssign(IR::Instr * instr);
88+
static IR::Instr * ChangeToAssignNoBarrierCheck(IR::Instr * instr);
8889
static IR::Instr * ChangeToAssign(IR::Instr * instr, IRType type);
8990
static IR::Instr * ChangeToLea(IR::Instr *const instr, bool postRegAlloc = false);
9091
static void ImmedSrcToReg(IR::Instr * instr, IR::Opnd * newOpnd, int srcNum);
@@ -305,7 +306,7 @@ class LowererMD
305306

306307
void GenerateWriteBarrierAssign(IR::IndirOpnd * opndDst, IR::Opnd * opndSrc, IR::Instr * insertBeforeInstr);
307308
void GenerateWriteBarrierAssign(IR::MemRefOpnd * opndDst, IR::Opnd * opndSrc, IR::Instr * insertBeforeInstr);
308-
static void ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
309+
static IR::Instr * ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
309310

310311
static IR::BranchInstr * GenerateLocalInlineCacheCheck(IR::Instr * instrLdSt, IR::RegOpnd * opndType, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelNext, bool checkTypeWithoutProperty = false);
311312
static IR::BranchInstr * GenerateProtoInlineCacheCheck(IR::Instr * instrLdSt, IR::RegOpnd * opndType, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelNext);

‎lib/Backend/arm/LowerMD.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,11 +2343,18 @@ IR::Instr* LowererMD::ChangeToHelperCallMem(IR::Instr * instr, IR::JnHelperMeth
23432343
///----------------------------------------------------------------------------
23442344

23452345
IR::Instr *
2346-
LowererMD::ChangeToAssign(IR::Instr * instr)
2346+
LowererMD::ChangeToAssignNoBarrierCheck(IR::Instr * instr)
23472347
{
23482348
return ChangeToAssign(instr, instr->GetDst()->GetType());
23492349
}
23502350

2351+
IR::Instr *
2352+
LowererMD::ChangeToAssign(IR::Instr * instr)
2353+
{
2354+
// TODO: (swb)(leish) assert that the dest is not indir or memref
2355+
return ChangeToWriteBarrierAssign(instr, instr->m_func);
2356+
}
2357+
23512358
IR::Instr *
23522359
LowererMD::ChangeToAssign(IR::Instr * instr, IRType type)
23532360
{
@@ -2381,13 +2388,13 @@ LowererMD::ChangeToAssign(IR::Instr * instr, IRType type)
23812388
return instr;
23822389
}
23832390

2384-
void
2391+
IR::Instr *
23852392
LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
23862393
{
23872394
#ifdef RECYCLER_WRITE_BARRIER_JIT
23882395
// WriteBarrier-TODO- Implement ARM JIT
23892396
#endif
2390-
ChangeToAssign(assignInstr);
2397+
return ChangeToAssign(assignInstr);
23912398
}
23922399

23932400
///----------------------------------------------------------------------------

‎lib/Backend/arm/LowerMD.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class LowererMD
7070
IR::Instr * ChangeToHelperCallMem(IR::Instr * instr, IR::JnHelperMethod helperMethod);
7171
static IR::Instr * CreateAssign(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instrInsertPt, bool generateWriteBarrier = true);
7272
static IR::Instr * ChangeToAssign(IR::Instr * instr);
73+
static IR::Instr * ChangeToAssignNoBarrierCheck(IR::Instr * instr);
7374
static IR::Instr * ChangeToAssign(IR::Instr * instr, IRType type);
7475
static IR::Instr * ChangeToLea(IR::Instr *const instr, bool postRegAlloc = false);
7576
static IR::Instr * ForceDstToReg(IR::Instr *instr);
@@ -270,7 +271,7 @@ class LowererMD
270271
static void GenerateStFldFromLocalInlineCache(IR::Instr * instrStFld, IR::RegOpnd * opndBase, IR::Opnd * opndSrc, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelFallThru, bool isInlineSlot);
271272
void GenerateFunctionObjectTest(IR::Instr * callInstr, IR::RegOpnd *functionOpnd, bool isHelper, IR::LabelInstr* continueAfterExLabel = nullptr);
272273

273-
static void ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
274+
static IR::Instr * ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
274275

275276
int GetHelperArgsCount() { return this->helperCallArgsCount; }
276277
void ResetHelperArgsCount() { this->helperCallArgsCount = 0; }

‎lib/Backend/arm64/LowerMD.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ class LowererMD
264264
static void GenerateLdLocalFldFromFlagInlineCache(IR::Instr * instrLdFld, IR::RegOpnd * opndBase, IR::Opnd * opndDst, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelFallThru, bool isInlineSlot) { __debugbreak(); }
265265
static void GenerateStFldFromLocalInlineCache(IR::Instr * instrStFld, IR::RegOpnd * opndBase, IR::Opnd * opndSrc, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelFallThru, bool isInlineSlot) { __debugbreak(); }
266266

267-
static void ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func) { __debugbreak(); }
267+
static IR::Instr * ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func) { __debugbreak(); }
268268

269269
int GetHelperArgsCount() { __debugbreak(); return 0; }
270270
void ResetHelperArgsCount() { __debugbreak(); }

‎lib/Common/Memory/ArenaAllocator.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ ArenaAllocatorBase<TFreeListPolicy, ObjectAlignmentBitShiftArg, RequireObjectAli
7070
ArenaMemoryTracking::ReportFreeAll(this);
7171
ArenaMemoryTracking::ArenaDestroyed(this);
7272

73+
#if DBG
74+
// tag the fields in case the address is reused in recycler and create a false positive
75+
this->cacheBlockEnd = (char*)((intptr_t)this->cacheBlockEnd | 1);
76+
#else
77+
this->cacheBlockEnd = nullptr;
78+
#endif
79+
7380
if (!pageAllocator->IsClosed())
7481
{
7582
ReleasePageMemory();
@@ -79,6 +86,7 @@ ArenaAllocatorBase<TFreeListPolicy, ObjectAlignmentBitShiftArg, RequireObjectAli
7986
#ifdef PROFILE_MEM
8087
LogEnd();
8188
#endif
89+
8290
}
8391

8492
template <class TFreeListPolicy, size_t ObjectAlignmentBitShiftArg, bool RequireObjectAlignment, size_t MaxObjectSize>

‎lib/Common/Memory/Recycler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7599,7 +7599,7 @@ Recycler::TrackAllocCore(void * object, size_t size, const TrackAllocData& track
75997599
{
76007600
size_t stackTraceSize = 16 * sizeof(void*);
76017601
item = NoCheckHeapNewPlus(stackTraceSize, TrackerItem, typeInfo);
7602-
StackBackTrace::Capture((char*)&item[1], stackTraceSize, 0);
7602+
StackBackTrace::Capture((char*)&item[1], stackTraceSize, 7);
76037603
}
76047604
else
76057605
{

‎lib/Common/Memory/RecyclerPointers.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,13 +539,13 @@ void* __cdecl memcpy(WriteBarrierPtr<T> *dst, const void *src, size_t count)
539539
template <typename T>
540540
errno_t __cdecl memcpy_s(WriteBarrierPtr<T> *dst, size_t dstSize, const void *src, size_t srcSize)
541541
{
542-
CompileAssert(false);
542+
static_assert(false, "Use CopyArray instead");
543543
}
544544

545545
template <typename T>
546546
void __stdcall js_memcpy_s(__bcount(sizeInBytes) WriteBarrierPtr<T> *dst, size_t sizeInBytes, __bcount(count) const void *src, size_t count)
547547
{
548-
CompileAssert(false);
548+
static_assert(false, "Use CopyArray instead");
549549
}
550550

551551
template <typename T>

‎lib/Runtime/Base/FunctionBody.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9373,6 +9373,13 @@ namespace Js
93739373
}
93749374
#endif
93759375

9376+
#if DBG
9377+
// tag the jsMethod in case the native address is reused in recycler and create a false positive
9378+
this->jsMethod = (Js::JavascriptMethod)((intptr_t)this->jsMethod | 1);
9379+
#else
9380+
this->jsMethod = nullptr;
9381+
#endif
9382+
93769383
this->state = CleanedUp;
93779384
#if ENABLE_DEBUG_CONFIG_OPTIONS
93789385
#if !DBG

0 commit comments

Comments
 (0)