Why does there need to be specific support for MEMORY_SANITIZER at all in
V8?
There is no ADDRESS_SANITIZER support, and I think that this falls in the
same
category and I would argue it should be handled in the same way. Before
pursing
this patch any further, I'd like to understand why V8's handling of MSAN
should
be different that ASAN.
Also, if memory sanitizer is an absolute requirement, then please build V8
for
MSAN with the ARM simulator, which doesn't JIT code. This should work out
of the
box without any code changes, or at least with a much smaller, less fragile
CL.
Otherwise, my fundamental issue with the CL is how sloppily it handles
unpoisoning of the JITTed stack. I would much prefer a tighter approach that
unpoisoned precisely. I think that would involve some combination of the
following:
- Track the "top" sp per-thread precisely when entering JIT code. When
entering
the runtime, find the bottom precisely. Only unpoison the delta. Probably
involves some short assembly snippets. Even better: understand what gets
accessed by the C++ code (parameters, frame pointer stack crawl with some
info
earch frame, etc.) and only unpoison that.
- Don't unpoison all JIT stack space when entering the deoptimizer. Only
unpoision the copied frames in the input buffer.
- Don't explicitly unpoison parameters. They are simply part of the region
of
stack above the current C++ frame and the top of JITTED code frames.
https://codereview.chromium.org/26006004/diff/33001/src/deoptimizer.cc
File src/deoptimizer.cc (right):
https://codereview.chromium.org/26006004/diff/33001/src/deoptimizer.cc#newcode114
src/deoptimizer.cc:114: Deoptimizer* deoptimizer = new
Deoptimizer(isolate,
This big unpoison of the stack is total overkill. The deoptimizer copies
a specific number of frames from the stack to the input_ of buffer of
the Deoptimizer. This is done after returning from this routine in the
hand-written assembly code of the deoptimizer. After that, the
Deoptimizer::ComputeOutputFrames method inspect this buffer to build the
output frames. You should be able to precisely unpoison just the input
buffer for which you have the exact size at the beginning of
Deoptimizer::ComputeOutputFrames.
https://codereview.chromium.org/26006004/diff/33001/src/msan.h
File src/msan.h (right):
https://codereview.chromium.org/26006004/diff/33001/src/msan.h#newcode49
src/msan.h:49: __msan_unpoison(&dummy, 65536); \
This seems totally arbitrary and extremely fragile. Why 64K? You will
end up unpoisoning stuff in C++ frames above small stacks of JIT frames.
If you need to specifically unpoison parts of the stack, you should know
how much and be precise about it.
I don't feel comfortable at all adding this in this form, since it seems
to be a bug just waiting to happen and it feels likely to break through
unrelated to V8 in the future.
https://codereview.chromium.org/26006004/diff/33001/src/msan.h#newcode52
src/msan.h:52: __msan_unpoison(&(c), sizeof(c)); \
Why are these not covered by the 64K above? From the usage, it appears
the arguments are always parameters to the function this macro is
embedded in. If they are pushed before the call, then they lie at an
address above locals in the current frame?, so the &dummy call should
cover them.
https://codereview.chromium.org/26006004/diff/33001/src/platform-posix.cc
File src/platform-posix.cc (right):
https://codereview.chromium.org/26006004/diff/33001/src/platform-posix.cc#newcode71
src/platform-posix.cc:71: #include "msan.h"
Here and elsewhere: None of these includes should be hard wired (they
should be conditionally included based on whether MEMORY_SANITIZER is
active).
https://codereview.chromium.org/26006004/diff/33001/src/platform-posix.cc#newcode211
src/platform-posix.cc:211: #if defined(ADDRESS_SANITIZER) ||
defined(MEMORY_SANITIZER) || \
This change been landed separately, why is this in this CL?
https://codereview.chromium.org/26006004/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
