From 12b5b0285711cc67db82431576c14e5e0b8ed171 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Thu, 20 Mar 2014 14:38:50 -0700 Subject: [PATCH] Bug 984101 - Expand SpiderMonkey's use of poisoning for diagnostics; r=jonco --HG-- extra : rebase_source : 312db74b85c9b40db1ccfc98a96206d2e1381703 --- js/public/Utility.h | 14 ++++++-- .../assembler/MacroAssemblerCodeRef.h | 4 ++- js/src/gc/Heap.h | 1 + js/src/gc/Marking.cpp | 33 +++++++++++++++---- js/src/gc/Nursery.cpp | 17 ++++++---- js/src/gc/Nursery.h | 3 -- js/src/jit/Ion.cpp | 2 +- js/src/jsgc.cpp | 5 +-- js/src/jsiter.cpp | 4 +-- js/src/jsutil.h | 2 +- 10 files changed, 58 insertions(+), 27 deletions(-) diff --git a/js/public/Utility.h b/js/public/Utility.h index 5abfd16b2d78..90964a744111 100644 --- a/js/public/Utility.h +++ b/js/public/Utility.h @@ -35,10 +35,18 @@ namespace mozilla {} namespace js {} /* - * Pattern used to overwrite freed memory. If you are accessing an object with - * this pattern, you probably have a dangling pointer. + * Patterns used by SpiderMonkey to overwrite unused memory. If you are + * accessing an object with one of these pattern, you probably have a dangling + * pointer. */ -#define JS_FREE_PATTERN 0xDA +#define JS_FRESH_NURSERY_PATTERN 0x2F +#define JS_SWEPT_NURSERY_PATTERN 0x2B +#define JS_ALLOCATED_NURSERY_PATTERN 0x2D +#define JS_FRESH_TENURED_PATTERN 0x4F +#define JS_SWEPT_TENURED_PATTERN 0x4B +#define JS_ALLOCATED_TENURED_PATTERN 0x4D +#define JS_SWEPT_CODE_PATTERN 0x3b +#define JS_SWEPT_FRAME_PATTERN 0x5b #define JS_ASSERT(expr) MOZ_ASSERT(expr) #define JS_ASSERT_IF(cond, expr) MOZ_ASSERT_IF(cond, expr) diff --git a/js/src/assembler/assembler/MacroAssemblerCodeRef.h b/js/src/assembler/assembler/MacroAssemblerCodeRef.h index 6ed05bbe0abc..b6f17ebc68e5 100644 --- a/js/src/assembler/assembler/MacroAssemblerCodeRef.h +++ b/js/src/assembler/assembler/MacroAssemblerCodeRef.h @@ -36,6 +36,8 @@ #if ENABLE_ASSEMBLER +#include "jsutil.h" + // ASSERT_VALID_CODE_POINTER checks that ptr is a non-null pointer, and that it is a valid // instruction address on the platform (for example, check any alignment requirements). #if WTF_CPU_ARM_THUMB2 @@ -199,7 +201,7 @@ public: if (!m_executablePool) return; - memset(m_code.executableAddress(), JS_FREE_PATTERN, m_allocSize); + JS_POISON(m_code.executableAddress(), JS_SWEPT_CODE_PATTERN, m_allocSize); m_code = MacroAssemblerCodePtr(); diff --git a/js/src/gc/Heap.h b/js/src/gc/Heap.h index 3c7dff34d643..7aa4ed1dcb08 100644 --- a/js/src/gc/Heap.h +++ b/js/src/gc/Heap.h @@ -288,6 +288,7 @@ struct FreeSpan return nullptr; } checkSpan(); + JS_POISON(reinterpret_cast(thing), JS_ALLOCATED_TENURED_PATTERN, thingSize); return reinterpret_cast(thing); } diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index edb89e28cadb..bab045d4abfd 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -97,17 +97,36 @@ static void MarkChildren(JSTracer *trc, jit::JitCode *code); /*** Object Marking ***/ -#ifdef DEBUG +#if defined(DEBUG) template static inline bool IsThingPoisoned(T *thing) { - const uint8_t pb = JS_FREE_PATTERN; - const uint32_t pw = pb | (pb << 8) | (pb << 16) | (pb << 24); - JS_STATIC_ASSERT(sizeof(T) >= sizeof(FreeSpan) + sizeof(uint32_t)); - uint32_t *p = - reinterpret_cast(reinterpret_cast(thing) + 1); - return *p == pw; + static_assert(sizeof(T) >= sizeof(FreeSpan) + sizeof(uint32_t), + "Ensure it is well defined to look past any free span that " + "may be embedded in the thing's header when freed."); + const uint8_t poisonBytes[] = { + JS_FRESH_NURSERY_PATTERN, + JS_SWEPT_NURSERY_PATTERN, + JS_ALLOCATED_NURSERY_PATTERN, + JS_FRESH_TENURED_PATTERN, + JS_SWEPT_TENURED_PATTERN, + JS_ALLOCATED_TENURED_PATTERN, + JS_SWEPT_CODE_PATTERN, + JS_SWEPT_FRAME_PATTERN + }; + const int numPoisonBytes = sizeof(poisonBytes) / sizeof(poisonBytes[0]); + uint32_t *p = reinterpret_cast(reinterpret_cast(thing) + 1); + // Note: all free patterns are odd to make the common, not-poisoned case a single test. + if ((*p & 1) == 0) + return false; + for (int i = 0; i < numPoisonBytes; ++i) { + const uint8_t pb = poisonBytes[i]; + const uint32_t pw = pb | (pb << 8) | (pb << 16) | (pb << 24); + if (*p == pw) + return true; + } + return false; } #endif diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 26f369bc00f3..afc4097ca5b7 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -62,9 +62,7 @@ js::Nursery::init() currentStart_ = start(); rt->gcNurseryEnd_ = chunk(LastNurseryChunk).end(); numActiveChunks_ = 1; -#ifdef JS_GC_ZEAL - JS_POISON(heap, FreshNursery, NurserySize); -#endif + JS_POISON(heap, JS_FRESH_NURSERY_PATTERN, NurserySize); setCurrentChunk(0); updateDecommittedRegion(); @@ -170,9 +168,7 @@ js::Nursery::allocate(size_t size) void *thing = (void *)position(); position_ = position() + size; -#ifdef JS_GC_ZEAL - JS_POISON(thing, AllocatedThing, size); -#endif + JS_POISON(thing, JS_ALLOCATED_NURSERY_PATTERN, size); return thing; } @@ -877,7 +873,7 @@ js::Nursery::sweep(JSRuntime *rt) { #ifdef JS_GC_ZEAL /* Poison the nursery contents so touching a freed object will crash. */ - JS_POISON((void *)start(), SweptNursery, NurserySize - sizeof(JSRuntime *)); + JS_POISON((void *)start(), JS_SWEPT_NURSERY_PATTERN, NurserySize); for (int i = 0; i < NumNurseryChunks; ++i) chunk(i).trailer.runtime = runtime(); @@ -890,6 +886,11 @@ js::Nursery::sweep(JSRuntime *rt) } else #endif { +#ifdef JS_CRASH_DIAGNOSTICS + JS_POISON((void *)start(), JS_SWEPT_NURSERY_PATTERN, allocationEnd() - start()); + for (int i = 0; i < numActiveChunks_; ++i) + chunk(i).trailer.runtime = runtime(); +#endif setCurrentChunk(0); } @@ -900,7 +901,9 @@ js::Nursery::sweep(JSRuntime *rt) void js::Nursery::growAllocableSpace() { +#ifdef JS_GC_ZEAL MOZ_ASSERT_IF(runtime()->gcZeal_ == ZealGenerationalGCValue, numActiveChunks_ == NumNurseryChunks); +#endif numActiveChunks_ = Min(numActiveChunks_ * 2, NumNurseryChunks); } diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index ed090d4cd1a5..3f33ab7d0f0a 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -293,9 +293,6 @@ class Nursery * In debug and zeal builds, these bytes indicate the state of an unused * segment of nursery-allocated memory. */ - static const uint8_t FreshNursery = 0x2a; - static const uint8_t SweptNursery = 0x2b; - static const uint8_t AllocatedThing = 0x2c; void enterZealMode() { if (isEnabled()) numActiveChunks_ = NumNurseryChunks; diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 108dc1657ead..fda263d86900 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -742,7 +742,7 @@ JitCode::finalize(FreeOp *fop) // Don't do this if the Ion code is protected, as the signal handler will // deadlock trying to reacquire the interrupt lock. if (fop->runtime()->jitRuntime() && !fop->runtime()->jitRuntime()->ionCodeProtected()) - memset(code_, JS_FREE_PATTERN, bufferSize_); + memset(code_, JS_SWEPT_CODE_PATTERN, bufferSize_); code_ = nullptr; // Code buffers are stored inside JSC pools. diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index b2cc008f6bac..21af761c5440 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -484,7 +484,7 @@ Arena::finalize(FreeOp *fop, AllocKind thingKind, size_t thingSize) if (!newFreeSpanStart) newFreeSpanStart = thing; t->finalize(fop); - JS_POISON(t, JS_FREE_PATTERN, thingSize); + JS_POISON(t, JS_SWEPT_TENURED_PATTERN, thingSize); } } } @@ -493,6 +493,7 @@ Arena::finalize(FreeOp *fop, AllocKind thingKind, size_t thingSize) JS_ASSERT(newListTail == &newListHead); JS_ASSERT(!newFreeSpanStart || newFreeSpanStart == thingsStart(thingKind)); + JS_POISON(data, JS_SWEPT_TENURED_PATTERN, sizeof(data)); return true; } @@ -781,7 +782,7 @@ Chunk::prepareToBeFreed(JSRuntime *rt) void Chunk::init(JSRuntime *rt) { - JS_POISON(this, JS_FREE_PATTERN, ChunkSize); + JS_POISON(this, JS_FRESH_TENURED_PATTERN, ChunkSize); /* * We clear the bitmap to guard against xpc_IsGrayGCThing being called on diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 3c050d79667b..e3f227829369 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -1472,8 +1472,8 @@ FinalizeGenerator(FreeOp *fop, JSObject *obj) gen->state == JSGEN_OPEN); // If gen->state is JSGEN_CLOSED, gen->fp may be nullptr. if (gen->fp) - JS_POISON(gen->fp, JS_FREE_PATTERN, sizeof(InterpreterFrame)); - JS_POISON(gen, JS_FREE_PATTERN, sizeof(JSGenerator)); + JS_POISON(gen->fp, JS_SWEPT_FRAME_PATTERN, sizeof(InterpreterFrame)); + JS_POISON(gen, JS_SWEPT_FRAME_PATTERN, sizeof(JSGenerator)); fop->free_(gen); } diff --git a/js/src/jsutil.h b/js/src/jsutil.h index 98ae97d00a62..521cfd5a6263 100644 --- a/js/src/jsutil.h +++ b/js/src/jsutil.h @@ -278,7 +278,7 @@ ClearAllBitArrayElements(size_t *array, size_t length) #ifdef DEBUG # define JS_CRASH_DIAGNOSTICS 1 #endif -#ifdef JS_CRASH_DIAGNOSTICS +#if defined(JS_CRASH_DIAGNOSTICS) || defined(JS_GC_ZEAL) # define JS_POISON(p, val, size) memset((p), (val), (size)) #else # define JS_POISON(p, val, size) ((void) 0)