AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[profiler] Make ScrapeNativeContext check types and only run it where safe.

Browse Source 
Previously ScrapeNativeContext was written quite defensively which could result
in false positives and crashes.

This CL makes the function always bail out when we're running on non-ia32/x64
since only those 2 properly verify whether the program is setting up a frame.
If we are setting up a frame, the context will be garbage.

This CL also disables profiler tests when TSAN is running since TSAN makes
ScrapeNativeContext unsafe: it considers SIGPROF asynchronous and will run the
handler after the program has already run further than the context that's
passed into the handler.

Bug: v8:9860, v8:9869
Change-Id: I5a08374feba2e0e77ddd59e02dc2d7e9c90c2e04
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1866469
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64354}
This commit is contained in:
Toon Verwaest 2019-10-17 15:29:35 +02:00 committed by Commit Bot
parent 503917b16e
commit 2dbcdc028a
2 changed files with 25 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/profiler/tick-sample.cc
View File

@ -143,40 +143,20 @@ bool SimulatorHelper::FillRegisters(Isolate* isolate,
}
#endif // USE_SIMULATOR
// Attempts to safely dereference the address of a native context at a given
// context's address. Returns kNullAddress on failure, in the event that the
// context is in an inconsistent state.
// Returns the native context for a JavaScript frame. If the frame wasn't a
// JavaScript frame, it'll return kNullAddress.
Address ScrapeNativeContextAddress(Heap* heap, Address context_address) {
#if !defined(V8_TARGET_ARCH_IA32) && !defined(V8_TARGET_ARCH_X64)
return kNullAddress;
#else
DCHECK_EQ(heap->gc_state(), Heap::NOT_IN_GC);
// If the value is tagged, we're looking at a JavaScript frame.
if (!HAS_STRONG_HEAP_OBJECT_TAG(context_address)) return kNullAddress;
if (heap->memory_allocator()->IsOutsideAllocatedSpace(context_address))
return kNullAddress;
// Note that once a native context has been assigned to a context, the slot
// is no longer mutated except during pointer updates / evictions. Since
// pointer updates exclusively occur on the main thread, and we don't record
// TickSamples when the main thread's VM state is GC, the only other
// situation where the address here would be invalid is if it's being
// reassigned -- which isn't possible.
int native_context_offset =
i::Context::SlotOffset(i::Context::NATIVE_CONTEXT_INDEX);
i::Address native_context_slot_address =
context_address + native_context_offset;
// By the prior hypothesis, the indirect native context address should always
// be valid.
if (heap->memory_allocator()->IsOutsideAllocatedSpace(
native_context_slot_address)) {
DCHECK(false);
return kNullAddress;
}
i::ObjectSlot native_context_slot(native_context_slot_address);
i::Object native_context = native_context_slot.Relaxed_Load();
return native_context.ptr();
i::Object object(context_address);
return i::Context::cast(object).native_context().ptr();
#endif
}
} // namespace

16
test/cctest/cctest.status
View File

@ -122,6 +122,22 @@
'test-parsing/ObjectRestNegativeTestSlow': [PASS, ['mode == debug', SKIP]],
}], # ALWAYS
##############################################################################
['(arch != ia32 and arch != x64)', {
# BUG(v8:9860). We can only safely read the native context for a frame on
# ia32 and x64.
'test-cpu-profiler/ContextFilterMovedNativeContext': [FAIL],
'test-cpu-profiler/ContextIsolation': [FAIL],
}],
##############################################################################
['tsan == True', {
# BUG(v8:9869) TSAN considers SIGPROF an asynchronous signal, and will call
# into the handler after the stack has drifted from the context on the
# signal.
'test-cpu-profiler/*': [SKIP],
}],
##############################################################################
['arch == arm64', {
'test-api/Bug618': [PASS],